Fix connect tunnel stall on non-auth handshake rejection - #1689
Open
grrowl wants to merge 1 commit into
Open
Conversation
When the gate rejects the tunnel upgrade with a status other than 401/403 (e.g. a 502/503 Cloudflare returns mid-deploy), the `unexpected-response` handler only recorded `lastError` and warned. Because that listener is registered, `ws` skips its own `abortHandshake`, so neither `error` nor `close` ever fires. The reconnect backoff is scheduled in the `close` handler, so it was never set and the socket sat in CONNECTING forever. To a user this presents as `bb connect` silently going offline and never coming back until they reload the plugin or re-pair — a single transient 502 from the getbb.app gate is enough to trigger it. Call `tunnel.terminate()` in that branch. During CONNECTING it routes to `abortHandshake`, which aborts the request and emits `error` then `close`, so the existing backoff/reconnect path runs unchanged. The 401/403 branch already tears the socket down via `credentialRejected` -> `teardown()`. Reproduced with a test that stands up a real `ws` server rejecting the upgrade with a 502 and asserts a reconnect is scheduled; it fails before this change (nextRetryAt stays null) and passes after. Co-Authored-By: Claude Opus 4.8 <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.
The bug
In
plugins/connect/src/tunnel.ts, theunexpected-responsehandler for theconnect tunnel handles a gate handshake rejection. For statuses other than
401/403 it only recorded
lastErrorand logged a warning:The subtle part is why that leaves the socket wedged. Registering an
unexpected-responselistener makeswsskip its own internalabortHandshake— it assumes the app now owns teardown. But this handler nevertore the socket down, so neither
errornorcloseever fired. Thereconnect backoff (
ReconnectBackoff+ thesetTimeoutthat re-dials) livesentirely in the
closehandler, so it was never scheduled and the socket sat inCONNECTINGindefinitely.User impact
A single non-auth rejection from the getbb.app gate — exactly the transient
502/503Cloudflare serves mid-deploy — took a user'sbb connecttunnelpermanently offline. It presented silently as "connect randomly stops working
and never comes back," recoverable only by reloading the plugin or re-pairing.
The fix
Call
tunnel.terminate()in that branch. DuringCONNECTING,terminate()routes to
abortHandshake, which aborts the request and emitserrorthenclose, so the existing backoff/reconnect path runs unchanged — no handlerrestructuring needed.
The 401/403 branch was already fine:
credentialRejected()callsteardown(),which invokes
this.tunnel?.terminate(), so that socket is torn down rather thanstranded in
CONNECTING.Reproduction / test
Added
tunnel-handshake.test.ts, which stands up a realwsserver thatrejects the upgrade with an HTTP
502, points aConnectTunnelat it, andasserts a reconnect is scheduled (
nextRetryAtbecomes non-null, state isreconnecting). It runs against a real handshake because a mockedwscannotreproduce the
abortHandshake-skip behavior.nextRetryAtstaysnulland the waittimes out after 3s.
HOST_DAEMON_PROTOCOL_VERSIONis unaffected: this changes only the plugin'sclient-side reaction to a gate handshake rejection, no server↔host-daemon wire
message, session payload, or RPC.