-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add error handling to the Connection state machine #110
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HerbCaudill this implementation seems to be enough to catch errors from the state machine and prevent crashing the sync server. Maybe you have some suggestions at how to do more with the actual error here or how to manage the connection going forward?
this.emit('change', summary) | ||
this.#log(`⏩ ${summary} `) | ||
this.#machine.subscribe({ | ||
next: state => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two signatures for the subscribe
method, one accepts Observer<T>
object and the other accepts listener functions as separate arguments. I've opted for switching to the object signature here because I find it easier to reason about but we could switch to passing multiple functions if we choose.
Docs: https://stately.ai/docs/actors#error-handling
Source: https://github.com/statelyai/xstate/blob/main/packages/core/src/createActor.ts#L322-L350
this.#log(`⏩ ${summary} `) | ||
}, | ||
error: error => { | ||
console.error('Connection encountered an error', error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm only logging the error to the console here. I'm not sure if it's helpful to also/instead call this.#log
.
I also experimented with calling this.stop()
here after logging the error to try to force disconnecting this Connection
. I don't know if that's appropriate or if there's some better means of disconnecting. Below is a listing of the output around the exception with and without calling stop()
for comparison:
Without calling this.stop()
:
localfirst:auth:connection:localhost:brent emit change 'connected' +1ms
localfirst:auth:connection:localhost:brent ⏩ connected +0ms
localfirst:auth:connection:localhost:brent emit updated undefined +0ms
localfirst:auth:connection:localhost admitted device +4ms
Connection encountered an error Error: Device k1ut2gm7y1kqf4was69jm6w3 not found
at assert (file:///C:/dev/xdev/local-first-auth/packages/shared/dist/index.js:4:19)
at device (file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:699:3)
at memberByDeviceId (file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:891:22)
at Team.memberByDeviceId (file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:1812:12)
at admit (file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:2474:39)
at file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:2478:24
at Function.resolveAssign [as resolve] (C:\dev\xdev\local-first-auth\node_modules\.pnpm\[email protected]\node_modules\xstate\dist\log-a312ebfc.cjs.js:77:21)
at resolveAndExecuteActionsWithContext (C:\dev\xdev\local-first-auth\node_modules\.pnpm\[email protected]\node_modules\xstate\dist\raise-182bb5c9.cjs.js:2240:56)
at resolveActionsAndContext (C:\dev\xdev\local-first-auth\node_modules\.pnpm\[email protected]\node_modules\xstate\dist\raise-182bb5c9.cjs.js:2262:21)
at microstep (C:\dev\xdev\local-first-auth\node_modules\.pnpm\[email protected]\node_modules\xstate\dist\raise-182bb5c9.cjs.js:1987:15)
localfirst:auth:connection:localhost:brent <-brent #23 SYNC {"head":["3qnkp5bwi8bhv6AH8AMuP77PR2EGvQC8e6ZXB45StY7H"]} +27ms
localfirst:auth:connection:localhost:brent sending sync message {"head":["3qnkp5bwi8bhv6AH8AMuP77PR2EGvQC8e6ZXB45StY7H"]} +1ms
localfirst:auth:connection:localhost:brent ->brent #12 SYNC {"head":["3qnkp5bwi8bhv6AH8AMuP77PR2EGvQC8e6ZXB45StY7H"]} +0ms
localfirst:auth:connection:localhost:brent emit change 'connected' +1ms
localfirst:auth:connection:localhost:brent ⏩ connected +0ms
localfirst:auth:connection:localhost <-? #2 DISCONNECT +7s
With calling this.stop()
:
localfirst:auth:connection:localhost:brent emit change 'connected' +0ms
localfirst:auth:connection:localhost:brent ⏩ connected +0ms
localfirst:auth:connection:localhost:brent emit updated undefined +1ms
localfirst:auth:connection:localhost admitted device +6ms
Connection encountered an error Error: Device hfppeln7nthulp6nlb1u1jbo not found
at assert (file:///C:/dev/xdev/local-first-auth/packages/shared/dist/index.js:4:19)
at device (file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:699:3)
at memberByDeviceId (file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:891:22)
at Team.memberByDeviceId (file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:1812:12)
at admit (file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:2474:39)
at file:///C:/dev/xdev/local-first-auth/packages/auth/dist/index.js:2478:24
at Function.resolveAssign [as resolve] (C:\dev\xdev\local-first-auth\node_modules\.pnpm\[email protected]\node_modules\xstate\dist\log-a312ebfc.cjs.js:77:21)
at resolveAndExecuteActionsWithContext (C:\dev\xdev\local-first-auth\node_modules\.pnpm\[email protected]\node_modules\xstate\dist\raise-182bb5c9.cjs.js:2240:56)
at resolveActionsAndContext (C:\dev\xdev\local-first-auth\node_modules\.pnpm\[email protected]\node_modules\xstate\dist\raise-182bb5c9.cjs.js:2262:21)
at microstep (C:\dev\xdev\local-first-auth\node_modules\.pnpm\[email protected]\node_modules\xstate\dist\raise-182bb5c9.cjs.js:1987:15)
localfirst:auth:connection:localhost ->? #2 DISCONNECT +2ms
localfirst:auth:connection:localhost connection stopped +1ms
localfirst:auth:connection:localhost:brent <-brent #23 SYNC {"head":["E33dZ6zvXdsA7bfpT3Ebu7jK7wZMBTTsEA7qrnoxtiVD"]} +24ms
localfirst:auth:connection:localhost:brent sending sync message {"head":["E33dZ6zvXdsA7bfpT3Ebu7jK7wZMBTTsEA7qrnoxtiVD"]} +1ms
localfirst:auth:connection:localhost:brent ->brent #12 SYNC {"head":["E33dZ6zvXdsA7bfpT3Ebu7jK7wZMBTTsEA7qrnoxtiVD"]} +0ms
localfirst:auth:connection:localhost:brent emit change 'connected' +1ms
localfirst:auth:connection:localhost:brent ⏩ connected +0ms
We can add an error listener when we subscribe to the state machine actor. In this case, we just log the error to the host console.
bde3d2d
to
ff32ee2
Compare
Solves
Unexpected errors that occur in the state machine within
Connection.ts
are not handled and can completely crash a sync server when they're encountered.Description
This adds a simple error listener when we subscribe to the state machine Actor so that we can at least log the error the host console. This also seems to prevent the hard crash that brings down the running sync server process.