- 
        Couldn't load subscription status. 
- Fork 2.4k
fix: preserve context by retrying with full conversation on invalid previous_response_id #7714
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|  | @@ -217,8 +217,8 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||
| metadata, | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| // Make the request | ||||||||||||||||||||||||||
| yield* this.executeRequest(requestBody, model, metadata) | ||||||||||||||||||||||||||
| // Make the request (pass systemPrompt and messages for potential retry) | ||||||||||||||||||||||||||
| yield* this.executeRequest(requestBody, model, metadata, systemPrompt, messages) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| private buildRequestBody( | ||||||||||||||||||||||||||
|  | @@ -297,6 +297,8 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||
| requestBody: any, | ||||||||||||||||||||||||||
| model: OpenAiNativeModel, | ||||||||||||||||||||||||||
| metadata?: ApiHandlerCreateMessageMetadata, | ||||||||||||||||||||||||||
| systemPrompt?: string, | ||||||||||||||||||||||||||
| messages?: Anthropic.Messages.MessageParam[], | ||||||||||||||||||||||||||
| ): ApiStream { | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| // Use the official SDK | ||||||||||||||||||||||||||
|  | @@ -323,13 +325,19 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||
| if (is400Error && requestBody.previous_response_id && isPreviousResponseError) { | ||||||||||||||||||||||||||
| // Log the error and retry without the previous_response_id | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment mentions logging but no actual logging is implemented. Consider adding proper error logging here for debugging purposes: 
        Suggested change
       
 | ||||||||||||||||||||||||||
| // Remove the problematic previous_response_id and retry | ||||||||||||||||||||||||||
| const retryRequestBody = { ...requestBody } | ||||||||||||||||||||||||||
| delete retryRequestBody.previous_response_id | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| // Clear the stored lastResponseId to prevent using it again | ||||||||||||||||||||||||||
| this.lastResponseId = undefined | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| // Re-prepare the full conversation without previous_response_id | ||||||||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good fix: when a 400 error occurs due to an invalid previous_response_id, the code now clears it and re-prepares the full conversation. Consider extracting the duplicated logic for re-preparing the conversation (lines ~327–339 and ~517–521) into a helper method to reduce repetition and ease future maintenance. | ||||||||||||||||||||||||||
| let retryRequestBody = { ...requestBody } | ||||||||||||||||||||||||||
| delete retryRequestBody.previous_response_id | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| // If we have the original messages, re-prepare the full conversation | ||||||||||||||||||||||||||
| if (systemPrompt && messages) { | ||||||||||||||||||||||||||
| const { formattedInput } = this.prepareStructuredInput(systemPrompt, messages, undefined) | ||||||||||||||||||||||||||
| retryRequestBody.input = formattedInput | ||||||||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with the existing suggestion - this duplicated logic for re-preparing the conversation (here and around line 517-521) should be extracted into a helper method. Consider something like: 
        Suggested change
       
 | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| // Retry with the SDK | ||||||||||||||||||||||||||
| const retryStream = (await (this.client as any).responses.create( | ||||||||||||||||||||||||||
|  | @@ -338,7 +346,13 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| if (typeof (retryStream as any)[Symbol.asyncIterator] !== "function") { | ||||||||||||||||||||||||||
| // If SDK fails, fall back to SSE | ||||||||||||||||||||||||||
| yield* this.makeGpt5ResponsesAPIRequest(retryRequestBody, model, metadata) | ||||||||||||||||||||||||||
| yield* this.makeGpt5ResponsesAPIRequest( | ||||||||||||||||||||||||||
| retryRequestBody, | ||||||||||||||||||||||||||
| model, | ||||||||||||||||||||||||||
| metadata, | ||||||||||||||||||||||||||
| systemPrompt, | ||||||||||||||||||||||||||
| messages, | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
|  | @@ -350,13 +364,13 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||
| } catch (retryErr) { | ||||||||||||||||||||||||||
| // If retry also fails, fall back to SSE | ||||||||||||||||||||||||||
| yield* this.makeGpt5ResponsesAPIRequest(retryRequestBody, model, metadata) | ||||||||||||||||||||||||||
| yield* this.makeGpt5ResponsesAPIRequest(retryRequestBody, model, metadata, systemPrompt, messages) | ||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| // For other errors, fallback to manual SSE via fetch | ||||||||||||||||||||||||||
| yield* this.makeGpt5ResponsesAPIRequest(requestBody, model, metadata) | ||||||||||||||||||||||||||
| yield* this.makeGpt5ResponsesAPIRequest(requestBody, model, metadata, systemPrompt, messages) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
|  | @@ -445,6 +459,8 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||
| requestBody: any, | ||||||||||||||||||||||||||
| model: OpenAiNativeModel, | ||||||||||||||||||||||||||
| metadata?: ApiHandlerCreateMessageMetadata, | ||||||||||||||||||||||||||
| systemPrompt?: string, | ||||||||||||||||||||||||||
| messages?: Anthropic.Messages.MessageParam[], | ||||||||||||||||||||||||||
| ): ApiStream { | ||||||||||||||||||||||||||
| const apiKey = this.options.openAiNativeApiKey ?? "not-provided" | ||||||||||||||||||||||||||
| const baseUrl = this.options.openAiNativeBaseUrl || "https://api.openai.com" | ||||||||||||||||||||||||||
|  | @@ -489,16 +505,22 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||
| if (response.status === 400 && requestBody.previous_response_id && isPreviousResponseError) { | ||||||||||||||||||||||||||
| // Log the error and retry without the previous_response_id | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same logging issue here - consider adding actual error logging: 
        Suggested change
       
 | ||||||||||||||||||||||||||
| // Remove the problematic previous_response_id and retry | ||||||||||||||||||||||||||
| const retryRequestBody = { ...requestBody } | ||||||||||||||||||||||||||
| delete retryRequestBody.previous_response_id | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| // Clear the stored lastResponseId to prevent using it again | ||||||||||||||||||||||||||
| this.lastResponseId = undefined | ||||||||||||||||||||||||||
| // Resolve the promise once to unblock any waiting requests | ||||||||||||||||||||||||||
| this.resolveResponseId(undefined) | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| // Retry the request without the previous_response_id | ||||||||||||||||||||||||||
| // Re-prepare the full conversation without previous_response_id | ||||||||||||||||||||||||||
| let retryRequestBody = { ...requestBody } | ||||||||||||||||||||||||||
| delete retryRequestBody.previous_response_id | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| // If we have the original messages, re-prepare the full conversation | ||||||||||||||||||||||||||
| if (systemPrompt && messages) { | ||||||||||||||||||||||||||
| const { formattedInput } = this.prepareStructuredInput(systemPrompt, messages, undefined) | ||||||||||||||||||||||||||
| retryRequestBody.input = formattedInput | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
| // Retry the request with full conversation context | ||||||||||||||||||||||||||
| const retryResponse = await fetch(url, { | ||||||||||||||||||||||||||
| method: "POST", | ||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great test coverage! Consider adding one more test case for when both the initial request AND the retry fail, to ensure proper error propagation in that scenario.