fix(relay): re-dial once with a fresh token before treating a 4401 as revocation - #102602
Merged
Merged
Conversation
… revocation A 4401 close after a successful handshake was read unconditionally as the connector having revoked this gateway's per-gateway secret (opt-out), so the transport latched auth_revoked, the adapter went relay_disabled, and all messaging stopped until a manual restart. But the connector sends the same plain 4401 'unauthorized' for an EXPIRED upgrade token (make_upgrade_token TTL is 300s). Scale-to-zero makes that routine: the instance is suspended while a re-dial is in flight, the token was minted before the freeze, the dial completes on resume with a token past its TTL, the connector refuses it with 4401, and the gateway misreads an expired token as a revoked credential. Production incident 2026-09-02 - the connector DB secret was never revoked. Fix, in gateway/relay/ws_transport.py: - The first post-handshake 4401 is provisional. The reader schedules ONE immediate re-dial (_redial_with_fresh_token) that bypasses the reconnect backoff; _dial_and_start mints a fresh token on every call. The backoff supervisor design is untouched. - Only a 4401 against that fresh token (either refused at the upgrade or closed after a descriptor on that connection, tracked by dial generation) latches auth_revoked. Terminal behaviour is otherwise unchanged. - If the fresh dial fails for a non-auth reason, hand off to the normal backoff supervisor. - Read the Close frame reason (_close_reason_of, sibling of _close_code_of). A 4401 whose reason is exactly 'expired' never latches revocation and takes the normal reconnect path - forward-compatible hook for the connector change landing separately. - disconnect() cancels the one-shot retry task alongside the supervisor. Tests (tests/gateway/relay/test_ws_transport.py, real websockets server): - 4401 once, next dial accepted -> reconnected, auth_revoked False, exactly 2 dials. - 4401 on every dial -> auth_revoked True after exactly one retry (2 dials), no supervisor, no further dials. - 4401 reason 'expired' (once and repeated) -> never latched, reconnects via the normal supervisor. - Existing 7d-B tests keep passing (4401 before any handshake stays retryable; the revoking-every-dial stub still latches).
૮ >ﻌ< ა ci reviewran on 5f0d500 — fix(relay): a dial whose reader died mid-hello is a failed d
|
…down Three teardown tests construct WebSocketRelayTransport via object.__new__ without __init__, so the new _auth_retry attribute was absent and disconnect() raised AttributeError. Read it with getattr like the other optional teardown handles.
alt-glitch
added
type/bug
…next dial, not a second dialer Review (round 1) found a race: a reader that dies with a provisional 4401 while the backoff supervisor is already mid-dial started _redial_with_fresh_token as a SECOND concurrent dialer; both installed sockets/readers and the supervisor could overwrite the accepted retry socket (3 dials, wrong socket). Now a provisional 4401 sets _auth_retry_pending; _dial_and_start consumes it and stamps _auth_retry_generation on whichever dialer performs the next dial. The reader arms a dialer only when none is live (_dialer_running), and an upgrade-time 4401 on that generation latches revocation from either dialer (_latch_if_fresh_token_refused). The reader also iterates its captured ws handle, not self._ws. Regression test reproduces the race with a deterministic fake connect; it fails with the _dialer_running guard removed.
…retry marker survives network failures Review round 2: - BLOCKER: with one-live-dialer, a reader that dies while its own dialer is still inside _dial_and_start (hello in flight) arms nothing — that is the dialer's job — but the dialer then returned 'connected', leaving no socket, no reader, no dialer. _dial_and_start now raises ConnectionError when the reader it installed has already finished, so both dialers take their normal failure path (retry -> supervisor; supervisor -> backoff). - MAJOR: _auth_retry_pending was consumed before websockets.connect, so a connect-time network failure un-marked the retry and the NEXT dial's real fresh-token 4401 read as another first strike (revocation never latched). The marker is now consumed only when the token reaches an auth outcome: upgrade accepted (stamp the generation) or upgrade 4401'd (judge it). Two regression tests, each mutation-checked red against its own guard.
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
2 participants
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.
Problem
A 4401 WebSocket close after a successful handshake was read unconditionally as "the connector revoked this gateway's secret" (opt-out). The transport latched
auth_revoked, the adapter wentrelay_disabled, and messaging stopped until a manual restart.The connector sends the same plain
4401 unauthorizedfor an expired upgrade token (make_upgrade_token, TTL 300s). With scale-to-zero that is routine:4401 and _handshake_succeededand latches revocation.Production incident 2026-09-02. The connector DB secret was never revoked.
Fix (
gateway/relay/ws_transport.py)_redial_with_fresh_token), bypassing the reconnect backoff. The backoff supervisor design is unchanged.auth_revokedlatches only if that fresh-token dial is also rejected with 4401 (refused at the upgrade, or closed after a descriptor on that connection — tracked by dial generation). Terminal behaviour is otherwise as before._close_reason_of(exc)(sibling of_close_code_of). A 4401 whose close reason is exactlyexpirednever latches revocation and takes the normal reconnect path. Forward-compatible hook for the connector-side change landing separately;docs/relay-connector-contract.mdis intentionally not touched here.disconnect()cancels the one-shot retry task alongside the supervisor.The adapter's revocation monitor only polls
auth_revoked, so the retry window is invisible to it.Tests
tests/gateway/relay/test_ws_transport.py(real in-processwebsocketsserver,reconnect_backoff_s=5.0so a fast recovery proves the retry bypasses backoff):auth_revokedFalse, exactly 2 dials.auth_revokedTrue after exactly one retry (dial count == 2), no supervisor, no further dials.expired(once, and twice in a row) → never latched, reconnects via the normal supervisor.Mutation check: with the retry removed (latch immediately, as before), the "4401 once then accepted" test fails; restored, all pass.
Review ledger (sol,
reviewerprofile = gpt-5.6-sol; each finding parent-reproduced before fixing)05ec6c1origin/main(5 hunks, main's comment-trim refactor of this file);_auth_retryfolded into main's teardown loop.05ec6c1_redial_with_fresh_tokenwhile the supervisor was already mid-dial → two concurrent dialers (probe: 3 dials, supervisor overwrote the retry's socket). Fixed14c4991: retry is a marker consumed by the next dial, reader arms a dialer only when none is live, reader iterates its capturedws.14c4991connect(), so a network blip un-marked the retry and the real fresh-token 4401 never latched (probe: 6 dials, revoked=False). Both fixed5f0d500:_dial_and_startraises if its reader already finished; marker consumed only at an auth outcome.5f0d500ConnectionErrorjudged acceptable (transient startup failure, gateway already handles). No new findings.Every regression test added in rounds 1–3 was mutation-checked: removing the guard it protects turns it red.