Skip to content

fix(streaming): bound translated SSE inspection (#1112) - #1133

Open
lidge-jun wants to merge 1 commit into
devfrom
codex/260806-stack01-bounded-sse
Open

fix(streaming): bound translated SSE inspection (#1112)#1133
lidge-jun wants to merge 1 commit into
devfrom
codex/260806-stack01-bounded-sse

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Summary

Bounds the translated-path SSE inspection so a delimiterless upstream stream can no longer pin memory or triple parse cost.

trackSseForRequestLog (src/server/relay.ts:353) accumulated decoded bytes into an unbounded buffer inside inspectChunk and re-parsed every complete payload through three separate string helpers; relaySseWithHeartbeat repeated the same unbounded append. 64 MiB of delimiterless upstream bytes therefore stayed resident and were scanned three times.

Both scanners now use the existing byte-bounded createSseInspector: an over-cap frame is discarded from inspection only, the parser resynchronizes at the next SSE delimiter, and the payload is parsed once and shared across the request-log, first-output, and terminal observers. Bytes relayed downstream are unchanged — the regression test asserts byte-for-byte preservation.

Closes #1112.

Attribution

This is @Ingwannu's work from #1114, cherry-picked with their authorship intact (git log shows them as the commit author, not me). #1114 is left open for them; I have not closed it. Thank you for the clean, correctly-layered fix — reusing the existing inspector rather than writing a third scanner is exactly right.

This is stack 1 of a stacked campaign landing the current bug backlog with contributor attribution preserved. Planning unit: devlog/_plan/260806_stacked_bug_campaign/.

Verification

  • bun test tests/sse-inspector-bounds.test.ts — 23 pass, 0 fail
  • bun run typecheck — exit 0
  • bun run privacy:scan — passed
  • Full bun run test via the repository prepush gate — passed

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability when processing server-sent event streams containing oversized or fragmented messages.
    • Preserved valid event data while preventing unbounded buffering.
    • Ensured stream completion, errors, and heartbeat relay behavior are handled consistently.
  • Documentation

    • Clarified how passthrough event streams handle response logging and heartbeat forwarding.
  • Tests

    • Added coverage for oversized frames, stream recovery, byte preservation, completion reporting, and heartbeat behavior.

@github-actions github-actions Bot added the bug Something isn't working label Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 91aabc3e-d94d-4599-9530-d88eb9d53564

📥 Commits

Reviewing files that changed from the base of the PR and between e9d957b and b2d1bb8.

📒 Files selected for processing (3)
  • src/server/relay.ts
  • structure/04_transports-and-sidecars.md
  • tests/sse-inspector-bounds.test.ts

📝 Walkthrough

Walkthrough

The translated request-log tracker and heartbeat relay now use createSseInspector. The inspector provides bounded frame handling, resynchronization, single-payload parsing, and explicit lifecycle cleanup. Tests and transport documentation cover the updated behavior.

Changes

SSE inspection integration

Layer / File(s) Summary
Shared inspector wiring
src/server/relay.ts
trackSseForRequestLog and relaySseWithHeartbeat now use createSseInspector, including EOF finalization and disposal on completion, errors, and cancellation.
Behavior validation and documentation
tests/sse-inspector-bounds.test.ts, structure/04_transports-and-sidecars.md
Tests cover frame limits, resynchronization, byte preservation, terminal reporting, heartbeat relay, and single JSON parsing. Documentation describes shared inspector use.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant UpstreamStream
  participant SSEInspector
  participant RequestLogTracking
  participant HeartbeatRelay
  participant Observers

  UpstreamStream->>RequestLogTracking: SSE chunks
  RequestLogTracking->>SSEInspector: feed chunks
  SSEInspector->>Observers: bounded parsed payload
  RequestLogTracking->>SSEInspector: finish and dispose

  UpstreamStream->>HeartbeatRelay: SSE chunks
  HeartbeatRelay->>SSEInspector: feed chunks
  SSEInspector->>Observers: heartbeat and terminal observations
  HeartbeatRelay->>SSEInspector: finish and dispose
Loading

Possibly related PRs

Suggested reviewers: ingwannu, wibias

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the translated SSE inspection fix and matches the primary change.
Linked Issues check ✅ Passed The changes bound translated SSE inspection, reuse single-payload parsing, preserve downstream bytes, and add regression tests for issue #1112.
Out of Scope Changes check ✅ Passed The code, documentation, and tests are directly related to the linked issue and stated SSE inspection objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/260806-stack01-bounded-sse

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants