fix: preserve response continuity on physical WebSockets - #694
Merged
Conversation
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
This PR fixes response continuation handling for pooled WebSocket connections.
When
store=falseis used, aprevious_response_idis only valid on the physical WebSocket that created it. Previously, a continuation could be routed through another pooled connection or a newly established one, which could result in upstreamprevious_response_not_founderrors and break implicit conversation continuity.This change tracks response ownership at the physical WebSocket level and ensures continuations are routed only to the connection that owns the parent response. When that connection is no longer available, implicit continuations recover safely by replaying the full input instead of attempting an invalid continuation.
Along the way, this PR also improves WebSocket timeout handling, tightens per-account connection limits under concurrent load, and makes the container health check more robust in proxy environments.
What changed
Physical WebSocket response ownership
Responses are now associated with the physical WebSocket that produced them.
Requests containing
previous_response_idare routed only to the owning connection after verifying that it:Ownership information is removed whenever a connection is closed, replaced, evicted, or when the upstream rejects the response ID. Each physical WebSocket retains only its most recent response anchor.
Explicit continuations no longer fall back to another pooled connection or a newly created connection, since those continuations are not valid upstream.
Implicit continuation recovery
Implicit continuations are now recovered more safely when the original connection is no longer usable.
Generation-based chain advancement prevents concurrent sibling requests from advancing the implicit chain out of order.
If the owning connection has disappeared, recovery establishes a new continuation root by replaying the full input on an isolated pool key.
If the owning connection is merely busy, it remains the active parent so sibling requests can still complete without unnecessarily resetting the continuation chain.
turnStateis no longer restored from cross-turn affinity. Only the value explicitly supplied by the current request is forwarded.WebSocket lifecycle
Persistent and one-shot WebSocket requests now use bounded response-start waits.
Requests that connect successfully but never receive a response-start event now fail instead of waiting indefinitely.
Timeout errors are propagated correctly, and timers, streams, and connection resources are cleaned up before returning.
The existing one-shot fallback is preserved only for full-input requests that do not include
previous_response_id.Connection pool
Connection factories now count toward
maxPerAccountwhile they are being created.Capacity is reserved before asynchronous connection creation begins and released regardless of whether creation succeeds or fails, preventing concurrent acquisitions from temporarily exceeding the configured limit.
Container health check
The container health check now ignores inherited
HTTP_PROXY,HTTPS_PROXY, andALL_PROXYvariables when probing the local endpoint.It connects directly to
127.0.0.1, applies a bounded timeout, prefers the runtimePORTenvironment variable, and falls back to the configured port or8080.Tests now use an ephemeral port instead of reserving port
8080.Why
When response storage is disabled (
store=false), response continuations are tied to the physical WebSocket that created the parent response.Pooling by logical session alone is therefore insufficient. A continuation must be sent through the same physical connection; otherwise, the upstream may reject it with
previous_response_not_found, leaving implicit conversation state unrecoverable.This change makes that ownership explicit, prevents unsafe cross-connection continuations, and provides a safe recovery path when the original connection is no longer available.
Test coverage
Added or expanded coverage for:
Validation
Validated on Ubuntu 24.04 x86_64 with Node.js 22.23.1.
git diff --checkCompatibility
previous_response_idrequests now fail instead of being replayed across a different physical WebSocket.8080.Fixes #691
Thank you for taking the time to review this PR. :)