Skip to content

fix(ai-gateway): continue usage processing after stream errors - #4816

Merged
chrarnoldus merged 4 commits into
mainfrom
heavy-comet
Jul 28, 2026
Merged

fix(ai-gateway): continue usage processing after stream errors#4816
chrarnoldus merged 4 commits into
mainfrom
heavy-comet

Conversation

@chrarnoldus

@chrarnoldus chrarnoldus commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • treat every usage stream-processing failure, including read and parser errors, as an aborted stream
  • log each failure through errorExceptInTest without generating Sentry noise
  • preserve partial usage data so processTokenData still runs inside after()

Verification

  • pnpm --filter web test --runInBand src/lib/ai-gateway/processUsage.test.ts -t "handles .* gracefully and returns partial data|handles SSE parser errors as aborted streams"
  • scripts/typecheck-all.sh --changes-only
  • pnpm --filter web lint -- apps/web/src/lib/ai-gateway/processUsage.shared.ts apps/web/src/lib/ai-gateway/processUsage.test.ts
  • pnpm exec oxfmt --list-different apps/web/src/lib/ai-gateway/processUsage.shared.ts apps/web/src/lib/ai-gateway/processUsage.test.ts

@chrarnoldus chrarnoldus self-assigned this Jul 28, 2026
Comment thread apps/web/src/lib/ai-gateway/processUsage.shared.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The latest commit removes Sentry reporting for stream-processing failures in drainSseStream, so genuine SSE parser errors are now completely invisible to Sentry, contradicting the PR's own goal of keeping parser errors visible.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/ai-gateway/processUsage.shared.ts 115 Stream-processing failures now only go through errorExceptInTest (console-only, no-op in tests) instead of captureException; every read/parse error is still unconditionally swallowed as an abort, so no failure of this kind reaches Sentry anymore
Files Reviewed (2 files)
  • apps/web/src/lib/ai-gateway/processUsage.shared.ts - 1 issue
  • apps/web/src/lib/ai-gateway/processUsage.test.ts - 0 issues

Fix these issues in Kilo Cloud

Previous Review Summaries (3 snapshots, latest commit ea2fdc7)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit ea2fdc7)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The final commit on this PR ("log all usage stream aborts") regresses drainSseStream in processUsage.shared.ts: it stops consulting isResponseInterruptedError and now unconditionally reports every stream read/parse failure to Sentry while swallowing genuine SSE parser errors, contradicting this PR's own stated goal of keeping parser errors visible.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/ai-gateway/processUsage.shared.ts 114 drainSseStream's catch block no longer checks isResponseInterruptedError; every stream failure (including expected client aborts/timeouts) is now reported via captureException, while SSE parser errors are swallowed instead of propagating
Files Reviewed (2 files)
  • apps/web/src/lib/ai-gateway/processUsage.shared.ts - 1 issue
  • apps/web/src/lib/ai-gateway/processUsage.test.ts - 0 issues

Fix these issues in Kilo Cloud

Previous review (commit 0f28374)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The previously flagged issue is resolved: unexpected reader.read() failures are now captured via Sentry (captureException with usage_stream_read tag) instead of being silently swallowed, while isResponseInterruptedError continues to recognize expected interruptions (including Undici UND_ERR_BODY_TIMEOUT).

Files Reviewed (2 files)
  • apps/web/src/lib/ai-gateway/processUsage.shared.ts
  • apps/web/src/lib/ai-gateway/processUsage.test.ts

Previous review (commit 1fe42f0)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

drainSseStream now silently swallows every reader.read() failure as a stream interruption, dropping the previous distinction (isResponseInterruptedError) that re-threw unexpected errors for visibility.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/ai-gateway/processUsage.shared.ts 112 All reader.read() errors are now treated as an aborted stream with no logging/re-throw for non-interruption errors, potentially masking real bugs
Files Reviewed (2 files)
  • apps/web/src/lib/ai-gateway/processUsage.shared.ts - 1 issue
  • apps/web/src/lib/ai-gateway/processUsage.test.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-5 · Input: 52 · Output: 11.1K · Cached: 1.5M

Review guidance: REVIEW.md from base branch main

Comment thread apps/web/src/lib/ai-gateway/processUsage.shared.ts Outdated
Comment thread apps/web/src/lib/ai-gateway/processUsage.shared.ts
@chrarnoldus
chrarnoldus merged commit ca7031b into main Jul 28, 2026
15 checks passed
@chrarnoldus
chrarnoldus deleted the heavy-comet branch July 28, 2026 11:04
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