Skip to content

fix(ai-gateway): log partial response body when client disconnects mid-stream - #4890

Merged
chrarnoldus merged 2 commits into
mainfrom
petal-bellusaurus
Jul 30, 2026
Merged

fix(ai-gateway): log partial response body when client disconnects mid-stream#4890
chrarnoldus merged 2 commits into
mainfrom
petal-bellusaurus

Conversation

@chrarnoldus

Copy link
Copy Markdown
Contributor

Problem

When a client disconnects mid-stream (e.g. it gives up the moment an error event arrives), the response pipeline's cancel() callback settled the request-log capture with 'response stream was cancelled' and no response body — even though rewriteSseStream had already accumulated the raw upstream chunks (including the error event) in capturedChunks. The accumulated text lived in a closure the cancel() callback couldn't reach, and settleOnce made the empty settlement win, so the api_request_log row ended up with response = NULL and only a response_body_read_error.

This mostly hits non-terminal error events ({"error": ...} chunks on chat_completions, type: 'error' events on messages), where the upstream keeps the stream open briefly after the error and the client disconnects first. Terminal events ([DONE], message_stop, response.completed/incomplete/failed) were already captured because settleBody() runs synchronously in the same macrotask as the final enqueue, before cancel() can interleave.

Fix

Hoist the capturedChunks buffer out of rewriteSseStream into the scope shared by the stream's start() and cancel() callbacks (one line of plumbing per API kind), so cancel() can settle with the partial body received so far — same as the existing upstream timeout/disconnect paths already do via settleReadError. The buffer stays null when request logging is disabled, so non-logged traffic still accumulates nothing.

Not covered (intentionally)

If the client disconnects before any bytes arrive (e.g. client-side timeout while the upstream is still rate-limiting), there is still nothing to log, and an error event that arrives after the disconnect is dropped — capturing it would require keeping the upstream stream alive purely for logging after the client is gone.

Tests

  • New: each API kind records the chunks received before the response stream is cancelled.
  • Tightened the existing cancel test to assert a undefined partial body when nothing was streamed.

Note: pnpm format:check/lint/typecheck --changes-only passed via the pre-push hook; the jest run for this file was not completed locally.

@chrarnoldus chrarnoldus self-assigned this Jul 30, 2026
@kilo-code-bot

kilo-code-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Hoisting capturedChunks into the scope shared by start() and cancel() in all three SSE rewriters correctly lets a client disconnect settle the request-log capture with the partial body, and settleOnce still prevents the terminal-event setBody path from being clobbered; verified with high confidence.

Notes

  • partialCapturedBody() intentionally returns undefined for an empty buffer, which keeps setReadError's existing { readError }-only settlement shape and matches the tightened test at apps/web/src/lib/rewriteModelResponse.test.ts:917.
  • No new memory retention: the buffer's lifetime is still bound to the response stream's underlying source, and accumulation after a client disconnect is unchanged from the previous closure-scoped buffer. It remains null when request logging is disabled.
  • Minor, pre-existing and not introduced here: the streaming TextDecoder is only flushed in settleBody(), so a partial body captured via cancel() can omit a trailing incomplete multi-byte sequence.
Files Reviewed (2 files)
  • apps/web/src/lib/rewriteModelResponse.ts
  • apps/web/src/lib/rewriteModelResponse.test.ts

Reviewed by claude-opus-5 · Input: 42 · Output: 8.6K · Cached: 902.7K

Review guidance: REVIEW.md from base branch main

@chrarnoldus
chrarnoldus merged commit 43b4607 into main Jul 30, 2026
15 checks passed
@chrarnoldus
chrarnoldus deleted the petal-bellusaurus branch July 30, 2026 16:07
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