Stop recording a client hang-up as a provider error - #219
Open
anilguleroglu wants to merge 1 commit into
Open
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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'withusage: {}. 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'sAbortSignal, and only a closed response socket reachescancel()— soabortController.signal.abortedis an exact test for "the client walked away." That case is split out from the provider-error path:cancelled. Both the SQLite and MongoDB aggregations matchsuccess/errorby exact value, so this is additive — a cancelled call still counts towardtotalCallsand the token sums, without landing in either the success or error bucket. No CHECK constraint on the SQLite column andStatusBadgealready recognized the value, so no migration.output_tokens_estimatedso a measured count and a derived one stay distinguishable in cost reporting.[DONE]into a stream with no reader left, throwing a second exception.auditStreamedOutput, taggedchat.completions:stream:cancelledwhen 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
next lint: cleanfalse)🤖 Generated with Claude Code
https://claude.ai/code/session_016vhAPUV6foqg5ap3x6j2K5