docs(ui): pin the scope of the socket's server-disconnect retry bound - #9582
Open
lstein wants to merge 1 commit into
Open
docs(ui): pin the scope of the socket's server-disconnect retry bound#9582lstein wants to merge 1 commit into
lstein wants to merge 1 commit into
Conversation
…invoke-ai#9542) The five-attempt bound added in invoke-ai#9540 governs server-initiated disconnects only. A retry that fails at the transport level surfaces as `connect_error`, not `disconnect`, and `socket.connect()` re-arms the manager's reconnection, so the manager's default unbounded backoff takes over from there. That handoff is deliberate: the server drops sockets for authorization changes and expects the client back, so `reconnection: false` or a `reconnectionAttempts` cap would strand the tab exactly when the server is briefly unreachable after such a change, and would hand the same dead end to every ordinary outage the manager rides out today. Record that in the hook's rationale and pin it with two tests: a `connect_error` during a retry is left to the manager (no second driver), and the socket is built with manager reconnection unbounded. Closes invoke-ai#9542 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Z8LWHPW4TfpNCtwUo5yr9
lstein
requested review from
JPPhoto,
Pfannkuchensack,
blessedcoolant and
dunkeroni
as code owners
September 11, 2026 01:53
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.
Summary
Resolves #9542 by option 1 from the issue: keep socket.io's own unbounded reconnection as the app's recovery path, and make the scope of the #9540 retry bound explicit in code and tests.
The five-attempt bound in
useSocketIOgoverns server-initiated disconnects only. When one of those retries fails at the transport level (server restarting, proxy down), socket.io reports it asconnect_errorrather thandisconnect, andsocket.connect()has already re-armed the manager's reconnection, so the manager's default backoff (infinite attempts, 1s doubling to a 5s ceiling, ±50% jitter) takes over. Our counter never sees it.That handoff is deliberate, and this PR says so where the next reader will look:
MAX_SERVER_DISCONNECT_RECONNECTS: the server drops sockets for authorization changes and expects the client back, soreconnection: falseor areconnectionAttemptscap would strand the tab exactly when the server is briefly unreachable after such a change, and would hand the same dead end to every ordinary outage the manager rides out today. Also notes why the two drivers cannot race: a retry here fires once per server disconnect (which needs a connected socket), andSocket.connect()does not open the manager while it is between its own attempts.connect_errorduring a retry is left to the manager (the hook adds no second driver), and the socket is built with manager reconnection unbounded.No behaviour changes. The thread's other conclusion stands too: raising only
reconnectionDelayMaxwould not slow the manager's 1s/2s/4s opening steps, so a rate-limit, if ever wanted, needs its own specified policy and reproduction (the four-attempts-in-180ms burst has not reproduced with the lockfile's client versions).QA Instructions
Frontend only.
pnpm vitest run src/services/events/useSocketIO.test.tsx(12 pass). Each new test was checked against the mutation it guards: aconnect_errorhandler that retries fails the first, and eitherreconnection: falseorreconnectionAttempts: 5fails the second, with nothing else affected. All fivepnpm lint:*scripts pass.Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_014Z8LWHPW4TfpNCtwUo5yr9