fix(sse-heartbeat): shape-aware keepalives keep streams alive through stricter proxies - #2233
Conversation
Bumps [mermaid](https://github.com/mermaid-js/mermaid) from 11.14.0 to 11.15.0. - [Release notes](https://github.com/mermaid-js/mermaid/releases) - [Commits](https://github.com/mermaid-js/mermaid/compare/mermaid@11.14.0...mermaid@11.15.0) --- updated-dependencies: - dependency-name: mermaid dependency-version: 11.15.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…RTBEAT_INTERVAL_MS
There was a problem hiding this comment.
Code Review
This pull request introduces configurable SSE heartbeat shapes (Anthropic ping, OpenAI chunk, and OpenAI responses) to prevent connection timeouts during long upstream processing. It also lowers the default stream idle timeout to 5 minutes and adds environment variable support for heartbeat intervals. Feedback identifies a redundant heartbeat transform in responsesHandler.ts and suggests optimizations in sseHeartbeat.ts, such as using the built-in identity transform and hoisting the TextEncoder to the module level to reduce allocations.
| .pipeThrough(createSseHeartbeatTransform({ | ||
| signal, | ||
| intervalMs: SSE_HEARTBEAT_INTERVAL_MS, | ||
| shape: HEARTBEAT_SHAPES.OPENAI_RESPONSES_IN_PROGRESS, | ||
| })); |
There was a problem hiding this comment.
This adds a second heartbeat transform to the stream. The handleChatCore function already applies an SSE heartbeat transform at the end of its pipeline (see chatCore.ts:4276). Since handleChatCore is called with convertedBody.stream = true, it will always attach a heartbeat if SSE_HEARTBEAT_INTERVAL_MS is enabled. This redundancy results in multiple keepalive events being sent to the client and may cause issues if the first heartbeat is processed by transformStream in an unexpected way.
There was a problem hiding this comment.
Thanks for catching this! You're right there's a redundancy. The first heartbeat wrap (in chatCore.ts:4276) emits shape comment when called from this path — because responsesHandler calls handleChatCore with clientRawRequest: null, so clientResponseFormat falls back to the default. The second wrap here (responsesHandler.ts) overrides with OPENAI_RESPONSES_IN_PROGRESS to match the post-transform shape that downstream clients actually parse.
Two ways to clean this up:
- Thread
clientResponseFormat: "openai-responses"throughconvertedBodysochatCore's heartbeat picks the right shape directly, then remove this second wrap. - Add an
omitHeartbeat: trueoption tohandleChatCoreand keep this wrap as the single authoritative emitter.
I have a slight lean toward option 1 since it keeps a single point of control, but it touches the convertedBody contract slightly. Happy to do either — which would you prefer?
| return new TransformStream<Uint8Array, Uint8Array>({ | ||
| transform(chunk, controller) { | ||
| controller.enqueue(chunk); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
| } | ||
|
|
||
| let intervalId: ReturnType<typeof setInterval> | undefined; | ||
| const encoder = new TextEncoder(); |
…h + module-level TextEncoder
…imeout default
b9db934
into
diegosouzapw:release/v3.8.0
|
Hey @NomenAK! Thank you for the shape-aware heartbeat work — exactly what was needed for strict proxies during long thinking phases. Integrated as b9db934. One small reviewer adjustment: I reverted the |
- antigravity: AntigravityCredentials.projectId widened to string|null to match base ProviderCredentials shape post-#2227 squash merge. - responses-handler: heartbeat assertion updated for #2233's new openai-responses-in-progress shape (was: keepalive comment). - search-registry: expected count is now 12 (ollama-search + zai-search both landed in this release).
Deep audit of all 320 commits since v3.7.9 found: - 18 merged PRs not documented in CHANGELOG (4 features, 10 bug fixes, 1 security, 2 chores, 1 debug improvement) - 3 contributors entirely missing from credits table (@NomenAK with 12 PRs, @kang-heewon, @one-vs) - 4 existing contributors with inaccurate PR counts (@oyi77 8→12, @ddarkr 2→3, @andrewmunsell 2→3, @nickwizard 2→3) New entries added: - feat: #2135 (1proxy settings), #2227 (antigravity project ID), #2238 (Z.AI Search), #2240 (CLI Suite) - fix: #2217, #2218, #2219, #2221, #2222, #2223, #2224, #2231, #2233, #2236, #2242, #2243 - security: #2209 (stack trace exposure) - chore: #2228, #2234 Total contributors updated from 50+ to 55+.
… stricter proxies (diegosouzapw#2233) Integrated into release/v3.8.0 with idle timeout default reverted to 600s
…pw#2233 diegosouzapw#2238 - antigravity: AntigravityCredentials.projectId widened to string|null to match base ProviderCredentials shape post-diegosouzapw#2227 squash merge. - responses-handler: heartbeat assertion updated for diegosouzapw#2233's new openai-responses-in-progress shape (was: keepalive comment). - search-registry: expected count is now 12 (ollama-search + zai-search both landed in this release).
Deep audit of all 320 commits since v3.7.9 found: - 18 merged PRs not documented in CHANGELOG (4 features, 10 bug fixes, 1 security, 2 chores, 1 debug improvement) - 3 contributors entirely missing from credits table (@NomenAK with 12 PRs, @kang-heewon, @one-vs) - 4 existing contributors with inaccurate PR counts (@oyi77 8→12, @ddarkr 2→3, @andrewmunsell 2→3, @nickwizard 2→3) New entries added: - feat: diegosouzapw#2135 (1proxy settings), diegosouzapw#2227 (antigravity project ID), diegosouzapw#2238 (Z.AI Search), diegosouzapw#2240 (CLI Suite) - fix: diegosouzapw#2217, diegosouzapw#2218, diegosouzapw#2219, diegosouzapw#2221, diegosouzapw#2222, diegosouzapw#2223, diegosouzapw#2224, diegosouzapw#2231, diegosouzapw#2233, diegosouzapw#2236, diegosouzapw#2242, diegosouzapw#2243 - security: diegosouzapw#2209 (stack trace exposure) - chore: diegosouzapw#2228, diegosouzapw#2234 Total contributors updated from 50+ to 55+.
… stricter proxies (diegosouzapw#2233) Integrated into release/v3.8.0 with idle timeout default reverted to 600s
…pw#2233 diegosouzapw#2238 - antigravity: AntigravityCredentials.projectId widened to string|null to match base ProviderCredentials shape post-diegosouzapw#2227 squash merge. - responses-handler: heartbeat assertion updated for diegosouzapw#2233's new openai-responses-in-progress shape (was: keepalive comment). - search-registry: expected count is now 12 (ollama-search + zai-search both landed in this release).
Deep audit of all 320 commits since v3.7.9 found: - 18 merged PRs not documented in CHANGELOG (4 features, 10 bug fixes, 1 security, 2 chores, 1 debug improvement) - 3 contributors entirely missing from credits table (@NomenAK with 12 PRs, @kang-heewon, @one-vs) - 4 existing contributors with inaccurate PR counts (@oyi77 8→12, @ddarkr 2→3, @andrewmunsell 2→3, @nickwizard 2→3) New entries added: - feat: diegosouzapw#2135 (1proxy settings), diegosouzapw#2227 (antigravity project ID), diegosouzapw#2238 (Z.AI Search), diegosouzapw#2240 (CLI Suite) - fix: diegosouzapw#2217, diegosouzapw#2218, diegosouzapw#2219, diegosouzapw#2221, diegosouzapw#2222, diegosouzapw#2223, diegosouzapw#2224, diegosouzapw#2231, diegosouzapw#2233, diegosouzapw#2236, diegosouzapw#2242, diegosouzapw#2243 - security: diegosouzapw#2209 (stack trace exposure) - chore: diegosouzapw#2228, diegosouzapw#2234 Total contributors updated from 50+ to 55+.
What
Extends the existing
sseHeartbeat.tsmodule with ashapeoption so synthetic keepalives are emitted in a format the downstream client (and any intermediate proxy) recognizes as activity:event: ping\ndata: {}\n\n(mirrors Anthropic native ping events)chat.completion.chunklinedata: {"type":"response.in_progress"}\n\nAlso bundles a separable change: lowers
STREAM_IDLE_TIMEOUT_MSdefault from 600_000 → 300_000 (still env-overridable viaSTREAM_IDLE_TIMEOUT_MS). Happy to split this into a follow-up PR if preferred.Why
Observed BYOK streaming sessions disconnecting mid-extended-thinking when the upstream provider (Anthropic Claude with thinking enabled on heavy contexts) goes silent for 60-120s. Some reverse proxies in front of the consumer count only
data:SSE lines as keepalive activity — not SSE comments (: keepalive). The current comment-style heartbeat fires correctly but isn't visible to those proxies, so the connection drops before the upstream resumes streaming.The 10-minute
STREAM_IDLE_TIMEOUT_MSdefault also turned out to be dead weight in practice: downstream proxies abandon well before we hit it, so we never get a useful failure signal. 5 minutes is still a safety ceiling but actionable.How
open-sse/utils/sseHeartbeat.ts: addsHEARTBEAT_SHAPESenum,shapeForClientFormat()helper mapping client format → shape, and an internalbuildHeartbeatPayload()builder. ExistingintervalMs/signalparameters preserved; newintervalMs <= 0returns a passthrough TransformStream (disables heartbeat cleanly).open-sse/handlers/chatCore.ts:4276: passesintervalMs: SSE_HEARTBEAT_INTERVAL_MSandshape: shapeForClientFormat(clientResponseFormat)to the existing heartbeat call site.open-sse/handlers/responsesHandler.ts:74: passesshape: HEARTBEAT_SHAPES.OPENAI_RESPONSES_IN_PROGRESS.open-sse/config/constants.ts: exports newSSE_HEARTBEAT_INTERVAL_MS(default 15s, env-overridable; set to 0 to disable).src/shared/utils/runtimeTimeouts.ts: surfacessseHeartbeatIntervalMsinUpstreamTimeoutConfigand tightensDEFAULT_STREAM_IDLE_TIMEOUT_MSto 300_000.open-sse/utils/stream.ts:903-905,1515matches/^event:\s*keepalive\b/i. None of the new heartbeat shapes produce that literal — a regression test intests/unit/sse-heartbeat.test.tscovers every shape against this regex.Tests
tests/unit/sse-heartbeat.test.ts: extended from 2 to 9 tests. Existing 2 still byte-identical (back-compat).tests/unit/sse-heartbeat-integration.test.ts: new file, 3 time-based integration tests piping a fake upstream through the heartbeat and asserting downstream sees correct-shape output within 200ms, plus regression vs the strip regex.tests/unit/runtime-timeouts.test.ts: extended for the new config field + the lowered idle default.npm run typecheck:core: clean.Notes
Happy to split the
STREAM_IDLE_TIMEOUT_MSdefault change into a separate PR if you'd prefer keeping this PR strictly about the heartbeat shape — the env var remains the authoritative override either way. Same for the shape default: kept as"comment"(current behavior) so no surprise for existing consumers; new shapes are opt-in via the wiring sites.Happy to revise the API shape, the format strings, or the wiring approach if you'd prefer a different layout.