Skip to content

fix(web): surface transport failures to pending stream consumers - #563

Open
Caldalis wants to merge 1 commit into
OpenBMB:mainfrom
Caldalis:fix/web-stream-transport-failures
Open

fix(web): surface transport failures to pending stream consumers#563
Caldalis wants to merge 1 commit into
OpenBMB:mainfrom
Caldalis:fix/web-stream-transport-failures

Conversation

@Caldalis

@Caldalis Caldalis commented Sep 8, 2026

Copy link
Copy Markdown

Not a re-run of #152 — this PR does not change the error-clearing in next(). Per the #128 resolution that behaviour is intentional, and it stays: a failure is delivered exactly once and the iterator never becomes a zombie that re-throws forever. What changes is that the failure reaches the consumer at all.

#505AsyncEventQueue.fail() resolved already-parked waiters with { done: true }, so a consumer awaiting next() when the transport dies sees a clean end of turn instead of the error.

#506 — an error response frame carrying a stream's id was dropped, because the handler only looked in pending. The stream never settles and the consumer hangs.

Fix

  • waiters now hold { resolve, reject }, and fail() rejects parked waiters instead of resolving them { done: true }. This is what AsyncEventQueue in src/gateway/client/GatewayWsClient.ts already does — the browser copy had drifted.
  • In handleMessage, a response that misses pending falls back to streams and calls stream.fail() when ok is false. Checked against GatewayWsConnection.ts:89-118 that a successful submit_turn only ever sends a final event and never an ok: true response, so the branch is gated on !frame.ok.

Why one PR

Same chain. With only the #506 half applied, the routed error still hits the unfixed fail() and gets swallowed, so the reported symptom is unchanged:

build #505 case #506 case
main Missing expected rejection hangs, never settles
#506 half only Missing expected rejection stream completed cleanly instead of surfacing the failure
this PR pass pass

Known limitation

next() checks this.error before draining this.values, so buffered events are discarded when a stream fails — close() keeps them, fail() doesn't. Pre-existing and shared with the Node client; this PR only makes that path reachable where it previously hung. Worth a separate fix.

GatewayWsClient has the same !pending gap. Left out to keep this off the path the Web UI actually uses.

Tests

tests/web/gateway-browser-client-stream-failures.spec.ts, 6 cases: both bugs, plus guards for normal completion, single delivery on the parked and unparked paths, and a malformed error envelope. Both bug cases fail on main.

Root suite 510 pass / 0 fail — the 7 cancelled are the pre-existing tests/network/fetch.spec.ts ones (#441). Web Regression green: 156 files / 1299 tests.

@Caldalis Caldalis changed the title fix(web): deliver stream failures to browser gateway consumers fix(web): surface transport failures to pending stream consumers Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant