perf: optimize Responses relay body and billing parsing - #5714
Conversation
WalkthroughAdds ChangesReplayableBody Infrastructure and Handler Migration
Responses API Billing DTOs and Handler Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
relay/channel/openai/relay_responses.go (1)
60-67: 📐 Maintainability & Code Quality | 🟡 MinorMap response tool types before flagging missing built-ins.
ResponsesBillingMeta.Toolsuses response-side types likeweb_search_call, whileBuiltInToolsis keyed by the request-sideweb_search_preview, so this path will log an error on normal web-search responses. Translate the type first or treat the miss as debug.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/openai/relay_responses.go` around lines 60 - 67, The tool lookup in the billing loop is using the response-side type directly, so `BuiltInTools` misses normal web-search calls and logs an error in `relayResponses`/`ResponsesBillingMeta` processing. Update the logic around `billingMeta.Tools` and `info.ResponsesUsageInfo.BuiltInTools` to translate response tool types like `web_search_call` to the request-side key used in the map (or otherwise handle the miss as a non-error/debug case) before incrementing `CallCount`.
🧹 Nitpick comments (1)
relay/claude_handler.go (1)
187-202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplayable-body lifecycle is correct.
body.Size()is captured forContentLength,jsonDatais released for GC, and the deferredcloseReplayableOutboundBodyruns afterDoRequestreturns (request body is fully sent by then, including streaming).The same
outboundBody/inline-closure cleanup pattern now appears inclaude_handler.go,compatible_handler.go,responses_handler.go, andchat_completions_via_responses.go. Consider extracting a small helper (e.g.doRequestWithReplayableBody(c, info, adaptor, requestBody, outboundBody, label)) to reduce duplication, but this is optional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/claude_handler.go` around lines 187 - 202, This replayable-body cleanup pattern is already correct; no functional fix is needed. If you want to address the remaining duplication, extract the repeated `defer closeReplayableOutboundBody(...)` plus `adaptor.DoRequest(...)` flow into a small shared helper such as `doRequestWithReplayableBody` and reuse it from `claude_handler`, `compatible_handler`, `responses_handler`, and `chat_completions_via_responses` to keep the lifecycle handling consistent and easier to maintain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relay/channel/api_request.go`:
- Around line 307-322: The newUpstreamRequest helper handles
common.ReplayableBody but does not make the request reusable for redirects or
retries. Update the replayableBody branch in newUpstreamRequest so the returned
http.Request has GetBody wired to replayableBody.Open(), and keep ContentLength
set from replayableBody.Size() to preserve resendability.
---
Outside diff comments:
In `@relay/channel/openai/relay_responses.go`:
- Around line 60-67: The tool lookup in the billing loop is using the
response-side type directly, so `BuiltInTools` misses normal web-search calls
and logs an error in `relayResponses`/`ResponsesBillingMeta` processing. Update
the logic around `billingMeta.Tools` and `info.ResponsesUsageInfo.BuiltInTools`
to translate response tool types like `web_search_call` to the request-side key
used in the map (or otherwise handle the miss as a non-error/debug case) before
incrementing `CallCount`.
---
Nitpick comments:
In `@relay/claude_handler.go`:
- Around line 187-202: This replayable-body cleanup pattern is already correct;
no functional fix is needed. If you want to address the remaining duplication,
extract the repeated `defer closeReplayableOutboundBody(...)` plus
`adaptor.DoRequest(...)` flow into a small shared helper such as
`doRequestWithReplayableBody` and reuse it from `claude_handler`,
`compatible_handler`, `responses_handler`, and `chat_completions_via_responses`
to keep the lifecycle handling consistent and easier to maintain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a624243-18b0-4382-b76d-fe88fb9f882f
📒 Files selected for processing (12)
common/body_storage.godto/openai_response.gorelay/channel/api_request.gorelay/channel/openai/chat_via_responses.gorelay/channel/openai/helper.gorelay/channel/openai/relay_responses.gorelay/chat_completions_via_responses.gorelay/claude_handler.gorelay/common/outbound_body.gorelay/compatible_handler.gorelay/outbound_body.gorelay/responses_handler.go
| func newUpstreamRequest(method string, url string, requestBody io.Reader) (*http.Request, error) { | ||
| if replayableBody, ok := requestBody.(common.ReplayableBody); ok { | ||
| body, err := replayableBody.Open() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| req, err := http.NewRequest(method, url, body) | ||
| if err != nil { | ||
| _ = body.Close() | ||
| return nil, err | ||
| } | ||
| req.ContentLength = replayableBody.Size() | ||
| return req, nil | ||
| } | ||
| return http.NewRequest(method, url, requestBody) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
Go net/http Client when does it use Request.GetBody to retry or redirect a request body
💡 Result:
In Go's net/http package, the Request.GetBody function is used by the http.Client to re-read a request body when it needs to be sent again [1][2]. Because the standard Request.Body is an io.ReadCloser (which is consumed after being read), the client cannot automatically retry a request or follow certain redirects if the body has already been read [3][4]. The client uses GetBody in the following scenarios: 1. Redirects (307 and 308): When the server returns a 307 (Temporary Redirect) or 308 (Permanent Redirect) status code, the client is permitted to follow the redirect while preserving the original HTTP method and body [1]. To send the body again to the new location, the client invokes the GetBody function to obtain a fresh, unread copy of the request body [1][2]. 2. Request Retries: The http.Transport may automatically retry an idempotent request (such as GET, HEAD, OPTIONS, or TRACE) if it encounters a transient network error [2]. If a request has a body, the transport will only perform this retry if GetBody is defined, allowing it to re-open and send the request body again [2]. Important Considerations: - Population: The standard library's http.NewRequest function automatically populates GetBody for common types like strings, byte slices, and bytes.Buffers [1][2]. If you manually create a request and set the Body field using a custom io.ReadCloser (e.g., from an os.File), GetBody will be nil by default, and redirects or retries that require reading the body again will fail [3][4][5]. - Implementation: When creating custom requests, you should define GetBody if you want your request to support these automatic redirects and retries [1][2]. It should be a function that returns a new io.ReadCloser that reads from the start of the data each time it is called [1][2]. - Server-side: GetBody is strictly for client-side requests and is unused when processing requests on the server side [2].
Citations:
- 1: https://pkg.go.dev/net/http
- 2: https://duyanghao.github.io/http-transport/
- 3: https://forum.golangbridge.org/t/understanding-the-limitation-of-reusing-file-based-request-bodies-in-gos-http-client/34168
- 4: https://stackoverflow.com/questions/52429036/getting-error-on-put-body-length-0-using-net-http
- 5: 307/308 Redirects Fail for POST Requests with Non-Empty Body (Missing req.GetBody) projectdiscovery/retryablehttp-go#493
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and ReplayableBody implementation.
git ls-files | rg '^relay/channel/api_request\.go$|common/.*\.go$'
printf '\n--- api_request.go outline ---\n'
ast-grep outline relay/channel/api_request.go --view expanded || true
printf '\n--- ReplayableBody search ---\n'
rg -n "type ReplayableBody|func \(.*\) Open\(|func \(.*\) Size\(|ReplayableBody" relay common -g '*.go'Repository: QuantumNous/new-api
Length of output: 4917
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant slices around newUpstreamRequest and ReplayableBody.
sed -n '280,340p' relay/channel/api_request.go
printf '\n---\n'
sed -n '1,220p' common/*.go 2>/dev/null | rg -n "type ReplayableBody|Open\(|Size\(|GetBody|Replayable" -n -C 2 || trueRepository: QuantumNous/new-api
Length of output: 1764
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and ReplayableBody implementation.
git ls-files | rg '^relay/channel/api_request\.go$|common/.*\.go$'
printf '\n--- api_request.go outline ---\n'
ast-grep outline relay/channel/api_request.go --view expanded || true
printf '\n--- ReplayableBody search ---\n'
rg -n "type ReplayableBody|func \(.*\) Open\(|func \(.*\) Size\(|ReplayableBody" relay common -g '*.go'Repository: QuantumNous/new-api
Length of output: 4917
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant slices around newUpstreamRequest and ReplayableBody.
sed -n '280,340p' relay/channel/api_request.go
printf '\n---\n'
sed -n '1,220p' common/*.go 2>/dev/null | rg -n "type ReplayableBody|Open\(|Size\(|GetBody|Replayable" -n -C 2 || trueRepository: QuantumNous/new-api
Length of output: 1764
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect ReplayableBody and storage implementations.
sed -n '1,140p' relay/common/outbound_body.go
printf '\n--- body_storage.go memory/disk implementations ---\n'
sed -n '1,320p' common/body_storage.go
printf '\n--- request execution path around DoRequest ---\n'
sed -n '330,560p' relay/channel/api_request.goRepository: QuantumNous/new-api
Length of output: 15348
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect ReplayableBody and storage implementations.
sed -n '1,140p' relay/common/outbound_body.go
printf '\n--- body_storage.go memory/disk implementations ---\n'
sed -n '1,320p' common/body_storage.go
printf '\n--- request execution path around DoRequest ---\n'
sed -n '330,560p' relay/channel/api_request.goRepository: QuantumNous/new-api
Length of output: 15348
Set req.GetBody for replayable bodies
http.NewRequest only auto-populates GetBody for a few concrete reader types, so this ReplayableBody still can’t be resent on 307/308 redirects or transport retries. Wire it to replayableBody.Open() here.
♻️ Proposed change
req.ContentLength = replayableBody.Size()
+ req.GetBody = func() (io.ReadCloser, error) {
+ return replayableBody.Open()
+ }
return req, 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.
| func newUpstreamRequest(method string, url string, requestBody io.Reader) (*http.Request, error) { | |
| if replayableBody, ok := requestBody.(common.ReplayableBody); ok { | |
| body, err := replayableBody.Open() | |
| if err != nil { | |
| return nil, err | |
| } | |
| req, err := http.NewRequest(method, url, body) | |
| if err != nil { | |
| _ = body.Close() | |
| return nil, err | |
| } | |
| req.ContentLength = replayableBody.Size() | |
| return req, nil | |
| } | |
| return http.NewRequest(method, url, requestBody) | |
| } | |
| func newUpstreamRequest(method string, url string, requestBody io.Reader) (*http.Request, error) { | |
| if replayableBody, ok := requestBody.(common.ReplayableBody); ok { | |
| body, err := replayableBody.Open() | |
| if err != nil { | |
| return nil, err | |
| } | |
| req, err := http.NewRequest(method, url, body) | |
| if err != nil { | |
| _ = body.Close() | |
| return nil, err | |
| } | |
| req.ContentLength = replayableBody.Size() | |
| req.GetBody = func() (io.ReadCloser, error) { | |
| return replayableBody.Open() | |
| } | |
| return req, nil | |
| } | |
| return http.NewRequest(method, url, requestBody) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@relay/channel/api_request.go` around lines 307 - 322, The newUpstreamRequest
helper handles common.ReplayableBody but does not make the request reusable for
redirects or retries. Update the replayableBody branch in newUpstreamRequest so
the returned http.Request has GetBody wired to replayableBody.Open(), and keep
ContentLength set from replayableBody.Size() to preserve resendability.
51fdfc5 to
2b6f1df
Compare
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
New Features
Bug Fixes