fix(net): reconnect when the session closes, not just when it errors#2183
Open
fperex wants to merge 1 commit into
Open
fix(net): reconnect when the session closes, not just when it errors#2183fperex wants to merge 1 commit into
fperex wants to merge 1 commit into
Conversation
Established.closed RESOLVES on every session end: qmux never rejects it and WebTransport only rejects on an abnormal close. Reload only retried from its catch block, so a session that simply dropped left a dead established, a status stuck on connected, and no reconnect attempt. Firefox and Safari ride the qmux WebSocket, so this was their only path to drop recovery and a page reload was the only way back. Race the session close against the effect teardown with a CANCELLED sentinel (both resolve to undefined otherwise), then clear established, report disconnected and retry on the existing backoff ladder. The ladder only resets once a session survives STABLE_CONNECTION_MS, so a relay that accepts and immediately drops us is not hammered once a second forever. Also pin the per-run cancel/abort handles (a rerun swaps both before awaiting the spawn) and swallow the closed rejection in the bandwidth poll, which only uses it as a stop signal.
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.
Split out of #2163.
A dropped session never reconnected.
Established.closedresolves on every session end: qmux never rejects it, and native WebTransport rejects only on an abnormal close. ButReloadonly retried from itscatchblock, so a session that simply closed fell through without scheduling a retry, leaving a deadestablished, a status still reading"connected", throughput at zero, and no reconnect attempt ever. A page reload was the only way back.This disproportionately hit Safari and Firefox, which ride the qmux WebSocket (where
closedalways resolves), so for them it was the only path to drop recovery.The fix treats a resolved
closedas a disconnect: clearestablished, report"disconnected", and retry on the existing backoff ladder.Two subtleties, both deliberate:
undefined, so racing them is ambiguous. ACANCELLEDsentinel tells "the effect was torn down" apart from "the session ended", and only the latter reconnects.STABLE_CONNECTION_MS. A relay that accepts us and immediately drops us (bad auth, shedding load, a redirect it cannot serve) would otherwise reset the ladder on every attempt and get hammered once a second forever.Also pins the per-run
cancel/aborthandles, since a rerun swaps both before awaiting the spawn, and swallows theclosedrejection in the bandwidth poll, which only uses it as a stop signal (that rejection is what drives the reconnect and must not surface as an unhandled rejection).Public API changes: none.
Test plan:
just js checkgreen. Verified in the demo by bouncing the relay: the viewer reconnects on the backoff ladder instead of sitting on a permanently "connected" dead session.