Skip to content

fix: handle remaining stream and tool-call edge cases after #3 - #10

Merged
thaolaptrinh merged 1 commit into
thaolaptrinh:mainfrom
yibudak:fix/stream-reliability-follow-up
Sep 9, 2026
Merged

thaolaptrinh merged 1 commit into
thaolaptrinh:mainfrom
yibudak:fix/stream-reliability-follow-up

Conversation

@yibudak

@yibudak yibudak commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #3: this addresses reproducible edge cases that remain on main at a97eb903932f2253d8ebeca2bcfabdbb47908889, which already includes that PR. It does not replace the earlier streaming/backpressure work or change authentication, model routing, deployment configuration, or supported API routes.

Changes

  • Downstream cancellation: observe ServerResponse.close rather than the already-completed request upload. Cancel upstream reads for disconnected SSE and JSON clients, settle backpressured writes on close/error, and avoid retrying an already-aborted caller.
  • Native stream idle timeout: ReadableStreamDefaultReader.cancel() resolves a pending read as EOF; it does not reject that read. Explicitly destroy the Node stream with the idle error so the encoder's existing error/terminal-record path runs instead of an indistinguishable successful EOF. Normal successful streams retain their existing behavior.
  • Bounded HTTP error bodies: keep non-2xx body consumption within the per-attempt deadline, cap it at 16 KiB, cancel unread data, and redact the supplied upstream key from HTTP diagnostics. Do not apply the header deadline to an ongoing successful generation.
  • Tool-call reconstruction: avoid appending canonical arguments after the same JSON was already streamed. Emit complete OpenAI ID/name metadata only once per index; preserve final-only calls and interleaved Anthropic tool blocks. Reconcile insignificant whitespace in partial JSON prefixes without changing emitted bytes, and reject conflicting values or whitespace that splits JSON tokens.
  • Non-streaming failures: do not convert an upstream error event into an empty successful response.
  • Token-count input validation: return a protocol-shaped 400 for null/primitive/malformed bodies instead of a 500 or silently accepting invalid top-level input. The token count remains the existing character-based estimate.

Reproduction examples on unmodified main

  • POST /v1/messages/count_tokens with JSON null returns 500.
  • Canceling a real local HTTP response client leaves the synthetic upstream signal/read uncanceled.
  • A tool-call-delta with {"x":1} followed by the matching canonical tool-call produces concatenated arguments {"x":1}{"x":1} in both protocols.
  • A native upstream stream that stalls is treated as clean EOF after cancellation.
  • A 400 response whose body never finishes outlives the configured header deadline.
  • A non-streaming upstream error event produces an empty success response.

The automated reproductions use synthetic upstream events/credentials, native Web streams, and real loopback HTTP connections. They demonstrate specific edge cases, not that every model/request encounters these failures. In particular, this is not a claim to fix the provider-side model availability problem discussed in #7.

Verification

  • Node.js 24 TypeScript build passed.
  • Full suite: 265 passed, 0 failed across 17 test files.
  • Regression tests cover disconnects, normal completion, backpressure, native idle cancellation, stalled/oversized error bodies, both tool protocols, partial whitespace/escapes, and invalid input.
  • Compared the current unmodified upstream build with the patched build using the same local reproductions.
  • Separate authorized live smoke checks passed for OpenAI JSON, OpenAI SSE, Anthropic JSON, and a streamed tool-call/tool-result round trip using deepseek/deepseek-v4-flash.

No real credentials, host-specific paths, private network configuration, or generated build artifacts are included. Happy to split the validation change or other independent pieces if smaller follow-up PRs would be preferred.

@thaolaptrinh
thaolaptrinh merged commit 0c788ec into thaolaptrinh:main Sep 9, 2026
2 checks passed
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.

2 participants