refactor: standardize request body handling and error messages across providers - #730
Closed
Pratham-Mishra04 wants to merge 1 commit into
Conversation
18 tasks
Collaborator
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Nov 2, 2025
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR refactors request and response handling across multiple AI provider implementations. Key changes include: migrating from generic interface{} to pre-serialized JSON []byte for request bodies, conditionally setting authentication headers only when credentials are present, implementing centralized error handling using schema-based constants, adding context-based request body caching and decoding utilities, and introducing raw response passthrough for specific providers. Changes
Sequence DiagramssequenceDiagram
participant Caller
participant Provider
participant checkContextAndGetRequestBody
participant Converter
participant JSON_Marshal
participant Handler
Caller->>Provider: Call TextCompletion(ctx, request)
Provider->>checkContextAndGetRequestBody: checkContextAndGetRequestBody(ctx, converter)
checkContextAndGetRequestBody->>checkContextAndGetRequestBody: Check cached body in context
alt Body cached
checkContextAndGetRequestBody-->>Provider: Return cached jsonData
else Body not cached
checkContextAndGetRequestBody->>Converter: Call converter function
Converter-->>checkContextAndGetRequestBody: Return (converted body, error)
alt Conversion error
checkContextAndGetRequestBody-->>Provider: Return BifrostError
else Conversion OK
checkContextAndGetRequestBody->>JSON_Marshal: Marshal to JSON
alt Marshal error
checkContextAndGetRequestBody-->>Provider: Return BifrostError
else Marshal OK
checkContextAndGetRequestBody-->>Provider: Return jsonData []byte
end
end
end
Provider->>Handler: Call completeRequest(ctx, jsonData, url, key)
Handler->>Handler: Set conditional auth header (if key non-empty)
Handler->>Handler: Execute HTTP request with jsonData as body
Handler->>Handler: Call checkAndDecodeBody(response)
Handler->>Handler: Handle gzip decompression if needed
Handler-->>Provider: Return decoded body + latency + error
Provider-->>Caller: Return result
sequenceDiagram
participant Handler
participant HTTP_Response
participant checkAndDecodeBody
participant Decompressor
participant JSON_Unmarshal
Handler->>HTTP_Response: Receive response
Handler->>checkAndDecodeBody: checkAndDecodeBody(response)
checkAndDecodeBody->>checkAndDecodeBody: Check Content-Encoding header
alt Encoding == gzip
checkAndDecodeBody->>Decompressor: Create gzip.NewReader
Decompressor-->>checkAndDecodeBody: Return reader
checkAndDecodeBody->>checkAndDecodeBody: Read and decompress
alt Decompression error
checkAndDecodeBody-->>Handler: Return error
else Decompression OK
checkAndDecodeBody-->>Handler: Return decompressed body
end
else No gzip encoding
checkAndDecodeBody->>HTTP_Response: Read body directly
checkAndDecodeBody-->>Handler: Return raw body
end
Handler->>JSON_Unmarshal: Unmarshal decoded body to schema
alt Unmarshal error
Handler->>Handler: Create BifrostError with provider-specific context
Handler-->>Caller: Return error
else Unmarshal OK
Handler-->>Caller: Return parsed response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
core/schemas/providers/gemini/speech.go (1)
20-20: Consider whether hardcoding ModalityAudio is always correct.The response modality is now hardcoded to
ModalityAudio. If speech requests might need other modalities (e.g., text + audio), this removes that flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
core/providers/anthropic.go(13 hunks)core/providers/azure.go(13 hunks)core/providers/bedrock.go(11 hunks)core/providers/cerebras.go(6 hunks)core/providers/cohere.go(8 hunks)core/providers/gemini.go(14 hunks)core/providers/groq.go(2 hunks)core/providers/mistral.go(4 hunks)core/providers/ollama.go(1 hunks)core/providers/openai.go(23 hunks)core/providers/openrouter.go(5 hunks)core/providers/parasail.go(3 hunks)core/providers/sgl.go(1 hunks)core/providers/utils.go(3 hunks)core/providers/vertex.go(8 hunks)core/schemas/providers/anthropic/responses.go(5 hunks)core/schemas/providers/anthropic/types.go(3 hunks)core/schemas/providers/gemini/speech.go(2 hunks)core/schemas/providers/vertex/types.go(1 hunks)core/schemas/responses.go(1 hunks)core/utils.go(1 hunks)framework/modelcatalog/main.go(1 hunks)transports/bifrost-http/handlers/server.go(1 hunks)transports/bifrost-http/integrations/anthropic.go(1 hunks)transports/bifrost-http/integrations/genai.go(1 hunks)transports/bifrost-http/integrations/openai.go(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (22)
transports/bifrost-http/handlers/server.go (1)
transports/bifrost-http/lib/config.go (1)
Config(135-165)
core/providers/sgl.go (3)
core/schemas/bifrost.go (4)
SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/speech.go (1)
BifrostSpeechRequest(9-15)core/schemas/transcriptions.go (2)
BifrostTranscriptionRequest(3-9)BifrostTranscriptionResponse(11-21)
core/providers/groq.go (2)
core/schemas/bifrost.go (5)
EmbeddingRequest(89-89)SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/account.go (1)
Key(8-17)
core/utils.go (1)
core/schemas/bifrost.go (4)
ModelProvider(32-32)Azure(36-36)Bedrock(38-38)Vertex(40-40)
core/providers/cohere.go (3)
core/schemas/provider.go (1)
ErrProviderResponseUnmarshal(25-25)core/schemas/bifrost.go (7)
TextCompletionRequest(83-83)BifrostError(326-335)TextCompletionStreamRequest(84-84)SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/account.go (1)
Key(8-17)
core/providers/openrouter.go (2)
core/schemas/bifrost.go (5)
EmbeddingRequest(89-89)SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/account.go (1)
Key(8-17)
core/schemas/providers/anthropic/responses.go (3)
core/schemas/providers/anthropic/types.go (2)
AnthropicMetaData(52-54)AnthropicContentBlockTypeThinking(130-130)core/schemas/responses.go (3)
ResponsesReasoning(704-707)ResponsesReasoningContent(718-721)ResponsesReasoningContentBlockTypeSummaryText(714-714)ui/lib/types/logs.ts (2)
ResponsesReasoning(401-404)ResponsesReasoningContent(396-399)
core/providers/vertex.go (7)
core/schemas/bifrost.go (7)
TextCompletionRequest(83-83)Vertex(40-40)TextCompletionStreamRequest(84-84)SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/account.go (2)
Key(8-17)VertexKeyConfig(29-33)core/schemas/providers/anthropic/chat.go (1)
ToAnthropicChatCompletionRequest(374-605)core/schemas/providers/openai/chat.go (1)
ToOpenAIChatRequest(20-61)core/schemas/providers/vertex/types.go (1)
DefaultVertexAnthropicVersion(8-8)core/utils.go (1)
Ptr(46-48)core/schemas/providers/vertex/embedding.go (1)
ToVertexEmbeddingRequest(8-67)
core/providers/anthropic.go (5)
core/schemas/bifrost.go (9)
BifrostError(326-335)TextCompletionStreamRequest(84-84)ModelProvider(32-32)BifrostStream(291-298)EmbeddingRequest(89-89)SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/provider.go (2)
ErrProviderResponseUnmarshal(25-25)PostHookRunner(194-194)core/schemas/providers/anthropic/text.go (1)
ToAnthropicTextCompletionRequest(11-46)core/schemas/providers/anthropic/chat.go (1)
ToAnthropicChatCompletionRequest(374-605)core/schemas/providers/anthropic/responses.go (1)
ToAnthropicResponsesRequest(117-199)
transports/bifrost-http/integrations/openai.go (1)
core/schemas/bifrost.go (1)
OpenAI(35-35)
core/providers/cerebras.go (3)
core/schemas/bifrost.go (7)
EmbeddingRequest(89-89)BifrostError(326-335)SpeechRequest(90-90)BifrostStream(291-298)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/account.go (1)
Key(8-17)core/schemas/provider.go (1)
PostHookRunner(194-194)
core/providers/gemini.go (6)
core/schemas/account.go (1)
Key(8-17)core/schemas/providers/gemini/types.go (1)
GenerateContentResponse(1173-1189)core/schemas/provider.go (4)
ErrProviderResponseUnmarshal(25-25)Provider(197-224)ErrProviderRequestTimedOut(22-22)ErrProviderRequest(24-24)core/schemas/providers/openai/chat.go (1)
ToOpenAIChatRequest(20-61)core/schemas/providers/gemini/speech.go (1)
ToGeminiSpeechRequest(9-41)core/schemas/providers/gemini/transcription.go (1)
ToGeminiTranscriptionRequest(5-73)
transports/bifrost-http/integrations/genai.go (4)
core/schemas/bifrost.go (1)
Gemini(47-47)core/schemas/providers/gemini/embedding.go (1)
ToGeminiEmbeddingResponse(57-100)transports/bifrost-http/integrations/router.go (1)
ChatResponseConverter(92-92)core/schemas/chatcompletions.go (1)
BifrostChatResponse(20-30)
core/providers/azure.go (7)
core/schemas/account.go (2)
Key(8-17)AzureKeyConfig(21-25)core/schemas/bifrost.go (9)
RequestType(79-79)BifrostError(326-335)TextCompletionRequest(83-83)ChatCompletionRequest(85-85)EmbeddingRequest(89-89)SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/provider.go (1)
ErrProviderResponseUnmarshal(25-25)core/schemas/providers/openai/text.go (1)
ToOpenAITextCompletionRequest(8-25)core/schemas/providers/openai/chat.go (1)
ToOpenAIChatRequest(20-61)core/schemas/providers/openai/responses.go (1)
ToOpenAIResponsesRequest(32-53)core/schemas/providers/openai/embedding.go (1)
ToOpenAIEmbeddingRequest(22-40)
core/providers/ollama.go (3)
core/schemas/bifrost.go (4)
SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/speech.go (1)
BifrostSpeechRequest(9-15)core/schemas/transcriptions.go (1)
BifrostTranscriptionRequest(3-9)
core/providers/utils.go (2)
core/schemas/bifrost.go (6)
BifrostContextKeyRequestBody(110-110)ModelProvider(32-32)BifrostError(326-335)RequestType(79-79)ErrorField(344-351)BifrostErrorExtraFields(393-397)core/schemas/provider.go (2)
ErrProviderJSONMarshaling(26-26)Provider(197-224)
transports/bifrost-http/integrations/anthropic.go (2)
core/schemas/provider.go (1)
Provider(197-224)core/schemas/bifrost.go (1)
Anthropic(37-37)
core/providers/mistral.go (3)
core/schemas/bifrost.go (7)
TextCompletionRequest(83-83)BifrostError(326-335)TextCompletionStreamRequest(84-84)SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/provider.go (1)
PostHookRunner(194-194)core/schemas/account.go (1)
Key(8-17)
core/providers/bedrock.go (7)
core/schemas/account.go (2)
Key(8-17)BedrockKeyConfig(39-46)core/schemas/bifrost.go (7)
BifrostError(326-335)TextCompletionStreamRequest(84-84)Bedrock(38-38)SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/providers/bedrock/types.go (1)
DefaultBedrockRegion(4-4)core/schemas/providers/bedrock/text.go (1)
ToBedrockTextCompletionRequest(11-58)core/schemas/providers/bedrock/chat.go (1)
ToBedrockChatCompletionRequest(12-42)core/schemas/providers/bedrock/responses.go (1)
ToBedrockResponsesRequest(12-118)core/schemas/providers/bedrock/embedding.go (1)
ToBedrockTitanEmbeddingRequest(12-41)
core/providers/parasail.go (2)
core/schemas/bifrost.go (7)
TextCompletionRequest(83-83)TextCompletionStreamRequest(84-84)EmbeddingRequest(89-89)SpeechRequest(90-90)SpeechStreamRequest(91-91)TranscriptionRequest(92-92)TranscriptionStreamRequest(93-93)core/schemas/account.go (1)
Key(8-17)
core/schemas/providers/gemini/speech.go (2)
core/schemas/speech.go (1)
BifrostSpeechRequest(9-15)core/schemas/providers/gemini/types.go (4)
GeminiGenerationRequest(51-64)GenerationConfig(622-688)Modality(697-697)ModalityAudio(707-707)
core/providers/openai.go (8)
core/schemas/providers/openai/text.go (1)
ToOpenAITextCompletionRequest(8-25)core/schemas/provider.go (2)
ErrProviderResponseUnmarshal(25-25)ErrProviderDecodeRaw(28-28)core/utils.go (1)
Ptr(46-48)core/schemas/chatcompletions.go (1)
ChatStreamOptions(176-179)core/schemas/providers/openai/chat.go (1)
ToOpenAIChatRequest(20-61)core/schemas/providers/openai/responses.go (1)
ToOpenAIResponsesRequest(32-53)core/schemas/providers/openai/embedding.go (1)
ToOpenAIEmbeddingRequest(22-40)core/schemas/providers/openai/speech.go (1)
ToOpenAISpeechRequest(20-38)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (9)
framework/modelcatalog/main.go (1)
221-236: LGTM: Good defensive programming.The nil check prevents potential panic when dereferencing
modelData.Data. This aligns with the PR's goal of improving error handling.transports/bifrost-http/integrations/genai.go (2)
48-55: LGTM: Consistent raw response passthrough pattern.The conditional RawResponse passthrough for Gemini provider aligns with the PR's standardization objectives and maintains backward compatibility.
56-63: LGTM: Mirrors embedding converter logic.Consistent implementation of raw response passthrough for chat completions.
core/schemas/providers/vertex/types.go (1)
7-9: LGTM: Clean constant addition.The exported constant provides a sensible default version for Vertex Anthropic API requests.
transports/bifrost-http/integrations/anthropic.go (1)
70-77: LGTM: Consistent with GenAI integration pattern.The RawResponse passthrough for Anthropic follows the same pattern implemented for Gemini, providing consistency across providers.
core/schemas/responses.go (1)
97-97: LGTM: Clean field addition for user context.The optional
Userfield extendsResponsesParametersto support user identification in requests, aligning with provider-specific metadata requirements.core/schemas/providers/gemini/speech.go (1)
9-9: No breaking changes detected.The grep results show both callers in
core/providers/gemini.go(lines 378 and 416) are already callingToGeminiSpeechRequest(request)with a single argument, matching the current signature. TheresponseModalitiesparameter removal does not break any callers in the codebase.core/utils.go (1)
62-64: Disregard this review comment—the function name is already clear and appropriate.The function name
isKeySkippingAllowedaccurately describes its purpose in context. The logic checks whether a given provider allows skipping key selection (returningtruefor all providers except Azure, Bedrock, and Vertex). The usage atcore/bifrost.go:2338shows it's called with a variable namedskipKeySelection, which reinforces the semantics. The boolean naming patternis...Allowedis idiomatic Go and follows standard conventions for predicate functions.The suggested alternatives (
canSkipKeyValidation,allowsEmptyKey) are less precise—canSkipKeyValidationconflates validation with key selection, andallowsEmptyKeyobscures the specific intent. Since the function has only one call site and the name aligns with its context, no refactoring is necessary.Likely an incorrect or invalid review comment.
transports/bifrost-http/handlers/server.go (1)
578-589: Graceful degradation is intentional and properly implemented.Verification confirms that all downstream operations handle missing model catalog data gracefully:
- OTEL plugin checks
if pricingManager != nilbefore cost calculation (lines 458, 503)- Telemetry plugin checks
if p.pricingManager != nilbefore cost calculation (lines 317-319)- Logging plugin checks
if p.pricingManager != nilbefore cost calculation (lines 460-462)- Governance plugin checks
if p.modelCatalog != nilbefore cost calculation (lines 483-485)When
ListAllModelsfails,AddModelDataToPoolis skipped, leaving PricingManager empty but functional. Cost calculations safely degrade to 0.0, and model lookups return empty slices. Server startup succeeds without catalog data, and request handling proceeds normally.
Comment on lines
+181
to
191
| respBody, err := checkAndDecodeBody(resp) | ||
| if err != nil { | ||
| return nil, latency, newBifrostOperationError(schemas.ErrProviderResponseUnmarshal, err, provider.GetProviderKey()) | ||
| } | ||
|
|
||
| // Read the response body and copy it before releasing the response | ||
| // to avoid use-after-free since resp.Body() references fasthttp's internal buffer | ||
| bodyCopy := append([]byte(nil), resp.Body()...) | ||
| // bodyCopy := append([]byte(nil), resp.Body()...) | ||
|
|
||
| return bodyCopy, latency, nil | ||
| return respBody, latency, nil | ||
| } |
Contributor
There was a problem hiding this comment.
Restore the defensive body copy before releasing the fasthttp response.
checkAndDecodeBody returns the slice from fasthttp.Response.Body() for non-gzipped payloads. Once the deferred fasthttp.ReleaseResponse(resp) runs, that buffer is recycled, so propagating respBody upward causes use-after-free corruption in every caller (TextCompletion, ChatCompletion, Responses, etc.). Bring back the copy that existed here previously.
respBody, err := checkAndDecodeBody(resp)
if err != nil {
return nil, latency, newBifrostOperationError(schemas.ErrProviderResponseUnmarshal, err, provider.GetProviderKey())
}
- // Read the response body and copy it before releasing the response
- // to avoid use-after-free since resp.Body() references fasthttp's internal buffer
-
- return respBody, latency, nil
+ // Read the response body and copy it before releasing the response
+ // to avoid use-after-free since resp.Body() references fasthttp's internal buffer
+ bodyCopy := append([]byte(nil), respBody...)
+ return bodyCopy, latency, nil📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| respBody, err := checkAndDecodeBody(resp) | |
| if err != nil { | |
| return nil, latency, newBifrostOperationError(schemas.ErrProviderResponseUnmarshal, err, provider.GetProviderKey()) | |
| } | |
| // Read the response body and copy it before releasing the response | |
| // to avoid use-after-free since resp.Body() references fasthttp's internal buffer | |
| bodyCopy := append([]byte(nil), resp.Body()...) | |
| // bodyCopy := append([]byte(nil), resp.Body()...) | |
| return bodyCopy, latency, nil | |
| return respBody, latency, nil | |
| } | |
| respBody, err := checkAndDecodeBody(resp) | |
| if err != nil { | |
| return nil, latency, newBifrostOperationError(schemas.ErrProviderResponseUnmarshal, err, provider.GetProviderKey()) | |
| } | |
| // Read the response body and copy it before releasing the response | |
| // to avoid use-after-free since resp.Body() references fasthttp's internal buffer | |
| bodyCopy := append([]byte(nil), respBody...) | |
| return bodyCopy, latency, nil | |
| } |
🤖 Prompt for AI Agents
In core/providers/anthropic.go around lines 181 to 191, restore a defensive copy
of the response body returned by checkAndDecodeBody before releasing the
fasthttp response to avoid use-after-free: after getting respBody and before
calling fasthttp.ReleaseResponse(resp) (or allowing its deferred call to run),
create a copy (e.g. append([]byte(nil), respBody...)) and return that copy (and
not the slice that points into resp's internal buffer); ensure this is done for
the non-gzipped path so callers receive a safe, independently allocated byte
slice.
Comment on lines
+299
to
+308
| func() (any, error) { | ||
| reqBody := openai.ToOpenAITextCompletionRequest(request) | ||
| if reqBody != nil { | ||
| reqBody.Stream = schemas.Ptr(true) | ||
| reqBody.StreamOptions = &schemas.ChatStreamOptions{ | ||
| IncludeUsage: schemas.Ptr(true), | ||
| } | ||
| } | ||
| return reqBody, nil | ||
| }, |
Contributor
There was a problem hiding this comment.
Preserve existing stream options when enabling usage data
Both helpers overwrite StreamOptions with a fresh struct, so any caller-supplied flags (e.g., IncludeObfuscation) disappear the moment we enable usage streaming. That’s a regression for providers relying on additional stream metadata. Instead, reuse the existing struct (creating it only when nil) and set IncludeUsage in place.
- if reqBody != nil {
- reqBody.Stream = schemas.Ptr(true)
- reqBody.StreamOptions = &schemas.ChatStreamOptions{
- IncludeUsage: schemas.Ptr(true),
- }
- }
+ if reqBody != nil {
+ reqBody.Stream = schemas.Ptr(true)
+ if reqBody.StreamOptions == nil {
+ reqBody.StreamOptions = &schemas.ChatStreamOptions{}
+ }
+ reqBody.StreamOptions.IncludeUsage = schemas.Ptr(true)
+ }
@@
- if reqBody != nil {
- reqBody.Stream = schemas.Ptr(true)
- reqBody.StreamOptions = &schemas.ChatStreamOptions{
- IncludeUsage: schemas.Ptr(true),
- }
- }
+ if reqBody != nil {
+ reqBody.Stream = schemas.Ptr(true)
+ if reqBody.StreamOptions == nil {
+ reqBody.StreamOptions = &schemas.ChatStreamOptions{}
+ }
+ reqBody.StreamOptions.IncludeUsage = schemas.Ptr(true)
+ }Also applies to: 651-659
🤖 Prompt for AI Agents
In core/providers/openai.go around lines 299 to 308 (and likewise at 651-659),
the current code unconditionally assigns a new StreamOptions struct which
discards any caller-supplied stream flags; instead, if reqBody.StreamOptions is
nil allocate a new schemas.ChatStreamOptions and assign it back, otherwise reuse
the existing struct and set reqBody.StreamOptions.IncludeUsage =
schemas.Ptr(true) in place (do not replace the StreamOptions pointer), and
ensure reqBody.Stream remains true — repeat the same in the other location to
preserve other flags like IncludeObfuscation.
Comment on lines
+552
to
+572
| func() (any, error) { | ||
| reqBody := anthropic.ToAnthropicChatCompletionRequest(request) | ||
| if reqBody == nil { | ||
| return nil, fmt.Errorf("chat completion input is not provided") | ||
| } | ||
|
|
||
| reqBody.Stream = schemas.Ptr(true) | ||
| reqBody.Stream = schemas.Ptr(true) | ||
|
|
||
| // Convert struct to map for Vertex API | ||
| reqBytes, _ := sonic.Marshal(reqBody) | ||
| var requestBody map[string]interface{} | ||
| sonic.Unmarshal(reqBytes, &requestBody) | ||
| // Convert struct to map for Vertex API | ||
| reqBytes, _ := sonic.Marshal(reqBody) | ||
| var requestBody map[string]interface{} | ||
| sonic.Unmarshal(reqBytes, &requestBody) | ||
|
|
||
| if _, exists := requestBody["anthropic_version"]; !exists { | ||
| requestBody["anthropic_version"] = "vertex-2023-10-16" | ||
| } | ||
| if _, exists := requestBody["anthropic_version"]; !exists { | ||
| requestBody["anthropic_version"] = vertex.DefaultVertexAnthropicVersion | ||
| } | ||
|
|
||
| delete(requestBody, "model") | ||
| delete(requestBody, "region") | ||
| delete(requestBody, "model") | ||
| delete(requestBody, "region") | ||
| return reqBody, nil | ||
| }, |
Contributor
There was a problem hiding this comment.
Restore Vertex Claude streaming payload mutations
When we return reqBody here, the anthropic_version default we just injected—and the removals of model/region—are dropped. As a result, Claude streaming calls go out without the required anthropic_version and with fields Vertex rejects, so the endpoint fails. Return the mutated requestBody map instead so those adjustments persist.
- delete(requestBody, "model")
- delete(requestBody, "region")
- return reqBody, nil
+ delete(requestBody, "model")
+ delete(requestBody, "region")
+ return requestBody, nil📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func() (any, error) { | |
| reqBody := anthropic.ToAnthropicChatCompletionRequest(request) | |
| if reqBody == nil { | |
| return nil, fmt.Errorf("chat completion input is not provided") | |
| } | |
| reqBody.Stream = schemas.Ptr(true) | |
| reqBody.Stream = schemas.Ptr(true) | |
| // Convert struct to map for Vertex API | |
| reqBytes, _ := sonic.Marshal(reqBody) | |
| var requestBody map[string]interface{} | |
| sonic.Unmarshal(reqBytes, &requestBody) | |
| // Convert struct to map for Vertex API | |
| reqBytes, _ := sonic.Marshal(reqBody) | |
| var requestBody map[string]interface{} | |
| sonic.Unmarshal(reqBytes, &requestBody) | |
| if _, exists := requestBody["anthropic_version"]; !exists { | |
| requestBody["anthropic_version"] = "vertex-2023-10-16" | |
| } | |
| if _, exists := requestBody["anthropic_version"]; !exists { | |
| requestBody["anthropic_version"] = vertex.DefaultVertexAnthropicVersion | |
| } | |
| delete(requestBody, "model") | |
| delete(requestBody, "region") | |
| delete(requestBody, "model") | |
| delete(requestBody, "region") | |
| return reqBody, nil | |
| }, | |
| func() (any, error) { | |
| reqBody := anthropic.ToAnthropicChatCompletionRequest(request) | |
| if reqBody == nil { | |
| return nil, fmt.Errorf("chat completion input is not provided") | |
| } | |
| reqBody.Stream = schemas.Ptr(true) | |
| // Convert struct to map for Vertex API | |
| reqBytes, _ := sonic.Marshal(reqBody) | |
| var requestBody map[string]interface{} | |
| sonic.Unmarshal(reqBytes, &requestBody) | |
| if _, exists := requestBody["anthropic_version"]; !exists { | |
| requestBody["anthropic_version"] = vertex.DefaultVertexAnthropicVersion | |
| } | |
| delete(requestBody, "model") | |
| delete(requestBody, "region") | |
| return requestBody, nil | |
| }, |
🤖 Prompt for AI Agents
In core/providers/vertex.go around lines 552 to 572, the function builds a
requestBody map and mutates it (injects anthropic_version and deletes
model/region) but then returns the original reqBody struct so those changes are
lost; change the return value to return the mutated requestBody map (not
reqBody) so the anthropic_version default and removals persist when sending the
Vertex Claude streaming payload.
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
Refactored provider request handling to improve error handling, support gzip responses, and standardize unsupported operation errors across providers.
Changes
checkContextAndGetRequestBodyutility to centralize request body preparation and error handlingcheckAndDecodeBodyto properly handle gzipped responsesType of change
Affected areas
How to test
Test various provider API calls with and without API keys:
Breaking changes
Related issues
Improves error handling and response processing across all providers.
Security considerations
Improved handling of API keys and authentication headers.
Checklist