fix(sdk): recover mid-stream provider 5xx/429 like severed connections - #1174
Open
nordicnode wants to merge 1 commit into
Open
fix(sdk): recover mid-stream provider 5xx/429 like severed connections#1174nordicnode wants to merge 1 commit into
nordicnode wants to merge 1 commit into
Conversation
A provider-reported 500/429 arriving mid-stream — the openai-compatible shim enqueues it as an error part with finishReason='error' — was thrown straight out of the stream and ended the entire run with an error. The same underlying transient event surfacing as a severed body instead took the capped recovery path (note injected into the conversation, retry step forced, capped at MAX_CONSECUTIVE_STREAM_RECOVERIES). The recoverable class was 'the connection failed to speak' and the fatal class was 'the provider reported a failure', which is backwards for flaky endpoints, where both are the same transient event. Route retryable APICallErrors (429, any 5xx) through the same capped recovery path with a message naming the HTTP status. Client-error statuses (400/401/402/403) are deterministic — retrying cannot help — so they stay fatal and still propagate to the run's error handling. Refs CodebuffAI#1155
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
1 participant
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.
Summary
errorpart withfinishReason='error'— no longer ends the entire run with an errorclassifyThrownStreamRecovery(already consulted for both the thrown-iterator and error-chunk shapes) so retryableAPICallErrors (429, any 5xx) take the same capped recovery path as a severed body: note injected into the conversation, retry step forced, capped atMAX_CONSECUTIVE_STREAM_RECOVERIESRetryErrorcause chain recovers, client-error statuses and post-cancel 5xx stay fatalThis addresses the run-killing part of #1155: the recoverable class was "the connection failed to speak" (
isTransientNetworkError, a socket-level allowlist) and the fatal class was "the provider reported a failure", which is backwards for flaky endpoints, where both are the same transient event.Validation
Re-run on the new main after the history rewrite:
bun test sdk/src/__tests__/stream-interruption.test.ts— 19 pass, 0 failbunx tsc --noEmit -p sdk/tsconfig.jsoncleanRefs #1155
Replaces #1162 — auto-closed when the repo history was force-pushed (the original commits no longer exist in the new history). Rebased onto the new main per the maintainer's request; picking the review up here.