Repository navigation
fix(websocket): report errors as Error objects and recover from auth failures - #198
Open
thorbenziemek wants to merge 1 commit into
Open
thorbenziemek wants to merge 1 commit into
thorbenziemek wants to merge 1 commit into
Conversation
…failures
The websocket client only ever reported server-sent error messages, as plain
`{ error: { message } }` objects, and permanently bricked itself on an
authentication failure. Connection-level failures reached nobody at all and did
not count towards the backoff.
- All errors are now `BrokerizeWebSocketError extends BrokerizeError`, with a
stable `code` and a `terminal` flag.
- Errors passed to a subscription callback are always terminal; errors the
client recovers from are reported via the new `subscribeConnectionState`.
- A rejected token is retried once with a forced refresh before the client goes
fatal, and `reconnect()` revives a fatal client.
- `onerror`, a rejected `getToken()` and unparsable messages are handled
instead of escaping as uncaught exceptions, and count towards the backoff.
- Subscriber callbacks are contained, so one throwing subscriber no longer
starves the others.
- `AuthorizedApiContext.destroy()` destroys the websocket client it created,
and reports requests on a destroyed context as `AbortError`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The websocket client reported almost nothing to applications, and what it did report was not usable as an error.
{ error: { message } }).err instanceof Errorwas false,err.messagewasundefined(the message sits aterr.error.message), and there was noname,codeor stack. Consumers had to reconstruct an error from the payload and decide severity by matching the literal string"authentication failed"— the same string the client itself matches on.onerrorwas never assigned, a rejectedAuth.getToken()was onlyconsole.error-ed, andJSON.parseon inbound frames was unguarded. These escaped as uncaught exceptions (with thewspackage, an unhandlederrorevent takes down the process) and reached no subscriber._errorCountwas only incremented fromonmessage, real connection failures never escalated the backoff: a client retried every ~0.6 s indefinitely against a down server.getToken()left an open, unauthenticated socket.this._socketstayed non-null, so the reconnect interval's!this._socketguard never fired and the client hung permanently with no signal.AuthorizedApiContextowns the client internally.forEachand silently starved every subscriber after it.This is the cause of the crashes seen downstream in bg-trading during the 2026-10-05 outage (
"Object captured as exception with keys: error"), which that repo currently works around by converting the payload into an error itself.Change
Errors are errors.
BrokerizeWebSocketError extends BrokerizeErrorcarries a stablecode(AUTH,WEBSOCKET_CONNECTION_FAILED,WEBSOCKET_PROTOCOL_ERROR,WEBSOCKET_ERROR), the matchinghttpStatusCode, an optionalcauseand aterminalflag. An authentication failure maps toAUTH/ 401, so it can be handled like a rejected token refresh on the REST API.isBrokerizeWebSocketError()is exported for name-based checks, which survive duplicate copies of the package in a dependency tree.Two channels, with a clear contract. Errors delivered to a subscription callback are now always terminal: the subscription is over. Everything the client recovers from on its own goes to the new
subscribeConnectionState(cb), which reportsclosed | connecting | open | reconnecting | fatalplus the causing error, and emits the current state on subscribe. Applications can show that live updates are unavailable instead of tearing down subscriptions on every hiccup — and, importantly, mapping the callback'serrstraight onto an RxJSsubscriber.error()stays correct.Authentication failures are recoverable. The usual cause is an id token that expired in flight, so the client now retries once with
getToken({ forceRefresh: true })and only escalates tofatalif that is rejected as well. A successful authentication restores the retry budget. A client that did go fatal can be revived withreconnect(); its subscriptions were terminated, so callers subscribe again.The silent failure paths now report.
onerror, a rejectedgetToken()(which also closes the socket so the reconnect interval takes over, instead of hanging on an unauthenticated connection) and unparsable frames all produce aBrokerizeWebSocketErrorand count towards the backoff — exactly once per failed attempt, whether it surfaces as an error event, a close event or both.Lifecycle fixes.
_invokeCallbackcontains every subscriber call, so a throwing subscriber cannot starve the others.oncloseno longer nullsthis._socketunconditionally. It closed overthis, not over its own socket, so the late close of a replaced socket discarded a freshly created connection and leaked the old one. Handlers are now detached on close and every handler ignores events from an outdated socket.else if (message as WebSocketError)was a cast, not a check, and matched every unrecognised message. It now checks.errorand logs genuinely unknown messages.Subscriptionbefore the callback runs._endSubscriptionno longer opens a connection just to send an unsubscribe, and tolerates an entry that is already gone.destroy()closes the socket, drops subscriptions and clears the reconnect and ping timers.AuthorizedApiContext.destroy()calls it for the client it created itself — previously that client kept reconnecting forever with the tokens of a context that was gone. A client passed in by the application is left alone.AuthorizedApiContextnow throwsBrokerizeAbortError, whosenameisAbortError.destroy()aborts the context'sAbortController, so a request already in flight rejects with aDOMExceptionnamedAbortError; both cases can now be handled with one check, the wayfetchbehaves with an already-aborted signal.API
Additive, except that
BrokerizeWebSocketClientgainssubscribeConnectionState,getConnectionStateandreconnect. Applications that wrap a client (the normal case) are unaffected; an application that implements the interface itself has to add them.destroy()is deliberately not on the interface, so that a client obtained fromcreateWebSocketClient()cannot be used to tear down the context's shared client.Auth.getTokenandCognitoFacade's session take an optionalGetTokenOptions, which existing implementations may ignore.Newly exported:
BrokerizeWebSocketError,BrokerizeWebSocketErrorCode,BrokerizeWebSocketErrorParams,isBrokerizeWebSocketError,BrokerizeAbortError,GetTokenOptions,WebSocketConnectionState,WebSocketConnectionStatus,ConnectionStateCallback.Server-side follow-up
WebSocketTypes.BrokerizeWebSocketError(the wire payload) gained an optionalcode, whichisAuthenticationErrorprefers over matchingmessage. The backend does not send it yet, so the string match remains as a fallback.src/websocketTypes.tsis a manual copy of the backend file and needs this field added there.Tests
npm test(new,node:test, no new dependencies, wired into the Build workflow) covers the error conversion, the auth retry and the escalation to fatal, the restored retry budget,reconnect(), the asynchronous report on a dead client, callback containment, unparsable messages, thegetToken()rejection, backoff accounting, the outdated-socket close event anddestroy(). 11 passing.tsc,eslintandprettierclean; API report regenerated.🤖 Generated with Claude Code