Skip to content

fix(sdk): harden the stream transport against a non-conforming peer #484

Description

@EricAndrechek

Area: sdk — streaming transport. Both surfaced reviewing #470 and deliberately not fixed there; neither is reachable with a conforming peer.

Two independent gaps in how the transport behaves against a peer that misbehaves. Filed together because both are "the SDK trusts the other side to be well-formed", and either is cheap now.

1. A buffer overflow re-dials forever with no escalation

clients/ts/src/stream/sse.ts ends the connection when eventsource-parser reports max-buffer-size-exceeded, returning terminal: false so the stream reconnects. That is right — an unterminated frame is a per-connection fault, not a permanent one.

But the backoff reset undoes it. _run resets attempt to 0 once a connection has held for STABLE_CONNECTION_MS, and accumulating 16 MiB usually takes longer than that. So the cycle is: connect, download up to 16 MiB, overflow, disconnect, reset the schedule, re-dial in ~0.5–1s, repeat — indefinitely, at a flat rate, never backing off.

WaveHouse cannot produce this: wireFrame always terminates a frame. It needs a broken or hostile intermediary. It is at least loud — each cycle emits SSE_PARSE_ERROR — but the client will keep pulling 16 MiB a second forever.

Fix sketch: don't treat an overflow-terminated connection as "held", or count consecutive overflows and escalate to terminal after N. The first is a one-line change to where liveMs is computed; the second is a better experience but needs a counter reset rule.

Acceptance

  • Repeated overflows back off rather than re-dialing at a flat rate
  • A single overflow followed by a healthy connection still resets the schedule
  • Pinned by a test that overflows twice and asserts the second delay exceeds the first

2. FetchLike never says the implementation must honor init.signal

_attempt re-checks this._closed after the auth call and after a fetch rejection, but not after a fetch resolves. With any implementation that honors init.signal that is fine — an abort errors the body, _pump's catch returns, and _emitStatus/_dispatch are _closed-guarded, so nothing reaches the consumer either way.

An options.fetch that ignores the signal is a different story: _pump keeps reading a socket the consumer already closed, after the connection counter has been decremented. The contract in clients/ts/src/types.ts enumerates what the SDK reads off the response and requires a streaming body, but never states that the signal must be honored — so an implementer has not been told.

Fix sketch: document the signal requirement on FetchLike (cheap, and probably the whole fix), and optionally re-check _closed on the success path as defence in depth.

Acceptance

  • FetchLike's documented contract states that init.signal must be honored
  • Decide explicitly whether the success-path _closed re-check is worth adding

Related: #470 (introduced both paths), #476, #477.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/sdkTypeScript SDK (clients/ts/)bugSomething isn't working

    Type

    No type

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions