Repository navigation
feat: reconnect and re-attach automatically after a lost connection (DB-760) - #82
Merged
kcheng486 merged 2 commits intoOct 19, 2026
Conversation
…DB-760) Review on #80: exposing ConnectionLostError, reconnect() and attach() as the normal recovery path leaves every consumer to write its own reconnect, backoff and cancellation loop for a transient socket failure. The driver already has what that needs (the session endpoint, the pending execution IDs, their handlers and stores), so it now recovers by itself. When the reader thread loses the transport, it reconnects to the same session WebSocket instead of failing pending queries: - connect_direct() hands the Connection a transport factory: one WebSocket attempt to the same URI with the headers snapshot, bounded by the time left. 401/403/404 and other non-retryable 4xx, or a set cancel_event, raise InterfaceError and end the recovery at once; any other failure is retried. - The reader retries with jittered exponential backoff (0.5s doubling to 5s) for up to reconnect_timeout_seconds, a new connect() and connect_direct() parameter (default 60s, None disables). The default fits, after up to ~40s of keepalive detection, in sql-session's 120s grace period for orphaned executions. A transport lost again within 10s of reconnecting continues the same recovery, deadline and backoff included, so a session that accepts and drops at once can't be reconnected to forever. - It reconnects eagerly, with or without pending queries. - On success it re-registers every pending query as Cursor.attach() registers one (fresh watchdog, _Attach(resumed=True), handler and store kept) and sends retrieve_results with attach: true for each, so the existing attach reply handling applies: owner again on a new session, polled on an old one, ExecutionCancelledError if the orphan lease ran out, ExecutionNotFoundError (with a reconnect-specific message) if the statement never reached the session. Cancels this connection already sent are sent again after the attaches. - The reader holds __send_lock for the whole recovery, so execute() and cancels wait for it: they then go out over the new transport after the re-attaches, or fail with ConnectionLostError once the connection is closed. close() cuts a backoff short. - A send that fails on a lost transport no longer closes the connection when reconnecting is on: it aborts the transport so the reader notices, and its query is re-attached with the rest. - Only when reconnecting fails, or is off, do pending queries get ConnectionLostError. A Connection built without a factory behaves as before. cancel_requested is now set in __send, once the cancel is admitted and just before it goes out, rather than before the cancel waits for the send lock. Otherwise a cancel waiting out a reconnect would take the blame for a cancellation the session made itself (the orphan lease) and report it as an empty success. This also resolves the review finding on #80 about that window. Connection.reconnect() and Cursor.attach() stay as the manual path for when automatic recovery is off or has given up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ursor.attach() (DB-760) With the connection reconnecting and re-attaching by itself, the manual path is redundant: after automatic recovery gives up, the session's orphan lease has all but run out too. Keeping it would leave public API to support for little gain, and it is what the review on #80 objected to as the normal recovery path. - Remove Connection.reconnect() and the reconnect_factory connect_direct() passed for it; the transport factory stays. - Remove Cursor.attach() and its attach_fn plumbing; Cursor is back to main's apart from the execution_id property, kept for correlating an execution with its errors and logs. - Remove Connection.__attach_execution, and the "already tracking execution" check in __send, which only an attach could trigger. - Every attach is now a re-attach after reconnecting, so _Attach loses resumed and ExecutionNotFoundError has a single message. - ConnectionLostError, ExecutionNotFoundError and ExecutionCancelledError stay public: fetches still raise them. Their docs no longer point at the manual path. - The README drops the manual-recovery example. tests/test_attach.py drives the same reply handling through a reconnect instead of Cursor.attach(): reattach() executes query EID, loses the transport, and hands the connection the fake session to reconnect to. Tests of the removed API itself (argument checks, rejecting an in-flight or tracked ID, reconnect()'s factory) are gone. This also makes the review finding on #80 about attach(Y) cancelling the cursor's own query before rejecting Y moot. Co-Authored-By: Claude Opus 5.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.
Summary
Stacked on #80, in response to Simon's review: consumers shouldn't each need their own reconnect, backoff and cancellation loop around
reconnect()andattach(). The driver already has the session endpoint, the pending execution IDs, and their handlers and stores. So when the WebSocket drops, the connection now reconnects to the same session by itself and re-attaches every pending query. Callers just get their results. The second commit then removes the manual path,Connection.reconnect()andCursor.attach(), so none of it becomes public API.Merging: #80's own diff still adds
reconnect()andattach(). To keep main from ever shipping them, merge this PR into #80's branch first, then merge #80.reconnect_timeout_seconds. That's a newconnect()/connect_direct()parameter: default 60s,Noneturns reconnecting off.cancel_event, ends recovery at once.retrieve_resultswithattach: true, keeping its handler and store. So feat: re-attach to a running execution after a lost connection (DB-760) #80's reply handling applies unchanged:ExecutionCancelledError.ExecutionNotFoundError, with a message specific to reconnecting.execute()and cancels wait. They then go out over the new transport, after the re-attaches, or fail withConnectionLostErroronce the connection closes.close()cuts a backoff short.ConnectionLostError. AConnectionbuilt without a transport factory behaves exactly as before.cancel_requestedis now set when the cancel is actually sent, not before it waits for the send lock. Otherwise a cancel waiting out a reconnect would be blamed for the session's own orphan-lease cancellation, which would read as an empty success. This also fixes Hambot's second finding on feat: re-attach to a running execution after a lost connection (DB-760) #80.Connection.reconnect(),Cursor.attach(),Connection.__attach_executionand the reconnect factory are gone. Once automatic recovery gives up, the session's orphan lease has nearly run out anyway.Cursoris back to main's version apart from theexecution_idproperty.ConnectionLostError,ExecutionNotFoundErrorandExecutionCancelledError, since fetches still raise them.attach(Y)cancels the cursor's own query before rejecting Y) no longer applies.Why 60s: keepalive takes up to about 40s (20s ping interval + 20s ping timeout) to notice a dead path, and sql-session cancels orphaned executions after 120s by default (wherobots/sql-session#219). 60s fits inside that.
Related Issues
Relates to DB-760. Base: #80. Server side: wherobots/sql-session#219. With this, the MCP server's reconnect loop in studio-backend#2894 should be unnecessary.
Requester Checklist
Complete these before marking Ready for Review
Visual Proof
tests/test_attach.pynow runs #80's reply-handling tests through a reconnect instead ofCursor.attach():reattach()executes queryEID, drops the transport, and hands the connection the fake session to reconnect to. Tests of the removed API itself are gone.tests/test_reconnect.py(31 new tests) drives fake transports and a fake factory through:execute()and a cancel waiting for the reconnect and following the re-attaches; a cancel sent before the loss being sent again;close()during backoff, during a connection attempt, and with anexecute()waiting;connect_direct()'s factory: same URI, a snapshot of the headers,open_timeoutbounded by the time left, 401/403/404 refused, 429/502/503 retried,cancel_eventhonoured.Mutation checks: each of these regressions makes a test fail:
close();InterfaceError;Not tested against a real SQL session or proxy:
execute()orcursor.close()can block for up toreconnect_timeout_seconds. Afterclose(), a waiting sender can stay blocked until the connection attempt in progress times out (at most 30s).ExecutionNotFoundError; the driver doesn't send it again. That's deliberate: it can't tell "never arrived" from "finished and evicted".Size Justification (if L/XL)
Against #80: +385/−243 in library code and docs, and +823/−328 in tests. The two commits are separate steps: automatic recovery first, then removing the manual path. Walkthrough:
driver.py:open_transport, the single-attempt factory, and the new parameter.connection.py:__main_loop→__reconnect: the recovery window and abort.__open_transport: backoff.__resume: the swap, re-attaches and cancels.__send: the failed-send hand-off and wherecancel_requestedis set.README.md: "Recovering from a lost connection".tests/test_attach.py'sreattach()helper.Reviewer Checklist
If these are not met, close the tab — this PR is not ready for review
Model: Claude Opus 5.5 (claude-opus-5-5) · Harness: Claude Code
🤖 Generated with Claude Code