Skip to content

Retry websocket connections closed with code 1001 - #140

Open
patrickswedish wants to merge 1 commit into
coinbase:masterfrom
patrickswedish:codex/retry-websocket-going-away
Open

Retry websocket connections closed with code 1001#140
patrickswedish wants to merge 1 commit into
coinbase:masterfrom
patrickswedish:codex/retry-websocket-going-away

Conversation

@patrickswedish

Copy link
Copy Markdown

Closes #63.

What changed

  • treat a remote WebSocket close with status 1001 (Going Away) as a recoverable disconnect
  • invoke on_close once for that remote lifecycle event
  • reuse the existing reconnect and resubscribe path when retries are enabled
  • preserve deliberate client shutdown as terminal behavior
  • extend the mock WebSocket server with a real 1001 restart scenario

Root cause

websockets classifies both 1000 and 1001 as ConnectionClosedOK. The message handler previously exited for every ConnectionClosedOK, so a Coinbase service restart silently stopped the receive loop without notifying the caller or attempting to restore subscriptions.

Validation

  • python -m unittest tests.websocket.test_websocket_base — 20 tests passed
  • python -m unittest discover -v — 174 tests passed
  • black --check passes for the three changed files after formatting
  • git diff --check passes

The new regression test uses the local WebSocket server to send status 1001, restart, and verify both the close callback and subscription restoration.

@cb-heimdall

Copy link
Copy Markdown

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

@patrickswedish
patrickswedish marked this pull request as ready for review August 1, 2026 19:26
@patrickswedish

Copy link
Copy Markdown
Author

Checking in on this — happy to address any review feedback or rebase if needed. All 174 tests pass and the change is scoped to the 1001 close-code handling path with no impact on normal shutdown behavior.

Treat remote Going Away closures as recoverable while preserving deliberate client shutdown, with local-server regression coverage for reconnect and resubscribe.
@patrickswedish
patrickswedish force-pushed the codex/retry-websocket-going-away branch from 0f4e849 to 8a5edfe Compare August 11, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1001 Going Away does not trigger on_close function

2 participants