Skip to content

Stop recording a client hang-up as a provider error - #219

Open
anilguleroglu wants to merge 1 commit into
mainfrom
fix/stream-client-disconnect-not-an-error
Open

Stop recording a client hang-up as a provider error#219
anilguleroglu wants to merge 1 commit into
mainfrom
fix/stream-client-disconnect-not-an-error

Conversation

@anilguleroglu

Copy link
Copy Markdown
Collaborator

Why

Follow-up to #218. A streamed call whose client disconnects mid-stream lands in the same catch block as a genuine provider failure, so every user who pressed stop was written to the usage log as status: 'error' with usage: {}. Two consequences followed: the model error rate — and alerting built on it — tracked user behavior instead of provider health, and the output tokens the provider had already generated (and billed us for) were recorded as zero.

What changed

Only cancel() aborts the stream's AbortSignal, and only a closed response socket reaches cancel() — so abortController.signal.aborted is an exact test for "the client walked away." That case is split out from the provider-error path:

  • A third usage status: cancelled. Both the SQLite and MongoDB aggregations match success/error by exact value, so this is additive — a cancelled call still counts toward totalCalls and the token sums, without landing in either the success or error bucket. No CHECK constraint on the SQLite column and StatusBadge already recognized the value, so no migration.
  • The generated output is recorded, not discarded. Provider usage rides on the terminal chunk, which is exactly the chunk a cancelling client didn't stay for. When it never arrives, output is estimated from the streamed text with the same chars/4 rule the quota pre-flight already uses, and the entry is marked output_tokens_estimated so a measured count and a derived one stay distinguishable in cost reporting.
  • Stop writing to an already-cancelled controller. The old error path enqueued an error frame and [DONE] into a stream with no reader left, throwing a second exception.
  • The output guardrail now runs on cancelled streams too. Those tokens reached the caller before the disconnect; only auditing completions that ran to the end meant hanging up was a way to skip the audit. Both paths go through one auditStreamedOutput, tagged chat.completions:stream:cancelled when the caller left early so the two stay separable in the evaluation log.

UI / reporting impact

Model Hub's Logs and Overview tabs can now show a neutral Cancelled badge. The model error rate will drop for any tenant with meaningful stream-cancellation traffic, since those calls are no longer miscounted as errors — this is the fix, not a regression, but worth knowing before someone looks at a dashboard and asks why the error rate moved.

Verification

  • TypeScript: clean
  • next lint: clean
  • Full suite: 3108 passed, 4 skipped (215 files) — includes 3 new mutation-tested cases (each fails when its guarding condition is flipped to false)
  • Docs build: clean
  • Production build: successful

🤖 Generated with Claude Code

https://claude.ai/code/session_016vhAPUV6foqg5ap3x6j2K5

A streamed call whose client disconnects lands in the same catch as a provider
failure, so every user who pressed stop was written to the usage log as
`status: 'error'` with `usage: {}`. Two things followed from that. The model
error rate — and the alerting on top of it — tracked user behaviour rather than
provider health, and the output the provider had already generated, and bills
us for, was recorded as zero tokens.

Only `cancel()` aborts the stream's signal, and only a closed response socket
reaches `cancel()`, so an aborted signal is an exact test for "the client went
away". Split that case out:

- Log it as `cancelled`, a third usage status. Both the SQLite and Mongo
  aggregations match `success` and `error` by exact value, so the new state is
  additive: it stays in `totalCalls` and in the token sums without landing in
  either bucket. The status column has no CHECK constraint and the Model Hub
  badge already knows the value, so nothing needs migrating.
- Record the output that was produced. Provider usage rides on the terminal
  chunk, which is exactly the chunk a cancelling client does not wait for, so
  when it never arrived the output is estimated from the streamed text with the
  same chars/4 rule the quota pre-flight uses. The entry carries
  `output_tokens_estimated` so a measured count and a derived one stay
  distinguishable in cost reporting — an estimate is defensible, quietly
  passing one off as measured is not.
- Stop writing to the controller. It is already cancelled; the error frame and
  `[DONE]` the old path emitted threw a second exception into a stream with no
  reader left.

Also run the streaming output guardrail on a cancelled stream. Those tokens
reached the caller, and auditing only the completions that ran to the end made
hanging up a way to skip the audit. Both paths now go through one
`auditStreamedOutput`, tagged `chat.completions:stream:cancelled` when the
caller left early so the two are separable in the evaluation log.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant