fix(browser): don't paint an error over a live stream - #1200
Merged
Merged
Conversation
The SDK closes the auth WebSocket as soon as it is finished with it, and
reports that close through the `error` callback EVEN THOUGH authentication
succeeded. Measured on dev, with a single mint so nothing else is in play:
23:32:58.148 [connection] WARN ... <- connect() ALREADY running
WebSocket .../auth failed: Close received after close
dcv.authenticate failed {code: 10}
23:32:58.456 [connectionmanager] Added connection 1
23:32:58.467 [dcv::display:1] ...
23:32:59.177 [dcv::input:1] ...
`connect` is running BEFORE the error arrives, so `success` fired first and
the stream establishes normally afterwards. In Node the same close is a
clean code 1000; Chrome surfaces it as a socket error.
`#status` is an absolutely-positioned overlay covering the whole display, so
the error handler was painting "Could not start the session. It may have
ended." across a working stream — and clearing `starting`, so a later post
could open a second one.
Ignore an auth error once a session exists; a real pre-session failure still
reports, and a disconnect resets the flag so a later attempt can fail loudly
again.
This also retires the `code: 10` I chased for several rounds as a service
fault. It was never a failure at all.
Mutation-checked: removing the guard fails the new assertion.
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.
What
The DCV SDK closes the auth WebSocket as soon as it is finished with it, and reports that close through the
errorcallback even though authentication succeeded.Measured on dev, with a single mint so nothing else is in play:
connectis running before the error arrives — sosuccessfired first, and the stream establishes normally afterwards. In Node the identical close is a cleancode 1000; Chrome surfaces it as a socket error.#statusis an absolutely-positioned overlay covering the whole display, so the handler was painting "Could not start the session. It may have ended." across a working stream. It also clearedstarting, so a later post could open a second one.Fix
Ignore an auth error once a session exists. A genuine pre-session failure still reports, and a disconnect resets the flag so a later attempt can fail loudly again.
This retires the code 10
I treated
dcv.authenticate failed {code: 10}as a service fault for several rounds. It was never a failure — it is the SDK reporting its own cleanup. Two things made it convincing: it is logged at error level with "Failed to communicate with server", and the stream it belonged to was genuinely broken at the time for four unrelated reasons (#1190, #1192, #1194, #1196). Once those were fixed the message stayed, because it had never been a symptom of any of them.The tell was always in the ordering —
connectrunning before the error — and I only saw it after #1199 removed the second mint that was muddying the sequence.Tests
Asserts an error after success leaves the status clear, and that a pre-session failure still surfaces.
Mutation-checked: removing the guard fails the new assertion. Suite 880 pass,
tsc --noEmitclean.🤖 Generated with Claude Code