fix(aws): cancel Bedrock requests on client disconnect - #6589
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
WalkthroughChangesAWS invocation lifecycle
Billing path normalization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AWSHandler
participant BedrockSDK
participant EventStream
Client->>AWSHandler: Send request with context
AWSHandler->>BedrockSDK: Invoke using derived context
BedrockSDK-->>AWSHandler: Return response or event stream
EventStream-->>AWSHandler: Deliver chunks and usage
Client-->>AWSHandler: Cancel request
AWSHandler->>EventStream: Close upstream stream
AWSHandler-->>Client: Stop output and finalize response
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/aws/relay_aws_test.go`:
- Line 448: Update the final body-length assertion in the cancellation test to
verify that responseWriter.Body.Len() is at least bodyLengthBeforeCancel, rather
than requiring exact equality. Keep the existing partial-usage and "[DONE]"
assertions unchanged.
🪄 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 Plus
Run ID: abbeff12-d2e9-42bc-ae14-35b8e32597c6
📒 Files selected for processing (4)
relay/channel/aws/relay-aws.gorelay/channel/aws/relay_aws_test.goservice/billing_usage.goservice/text_quota_test.go
| assert.Equal(t, dto.BillingUsageSemanticAnthropic, result.usage.BillingUsage.Semantic) | ||
| assert.Equal(t, 100, result.usage.BillingUsage.ClaudeUsage.InputTokens) | ||
| assert.Equal(t, 1, result.usage.BillingUsage.ClaudeUsage.OutputTokens) | ||
| assert.Equal(t, bodyLengthBeforeCancel, responseWriter.Body.Len()) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Relax the exact body-length assertion.
bodyLengthBeforeCancel is captured inside Flush at the moment the body first contains partial. The producer has already written all three initial events to the pipe at that point. The handler can therefore write further SSE frames before it observes ctx.Done(), so the final body length is not deterministic. This assertion can fail intermittently.
The invariants that matter are already covered by the partial-usage assertions and by assert.NotContains(..., "[DONE]"). Assert a lower bound instead of exact equality.
💚 Proposed change
- assert.Equal(t, bodyLengthBeforeCancel, responseWriter.Body.Len())
+ assert.GreaterOrEqual(t, responseWriter.Body.Len(), bodyLengthBeforeCancel)As per coding guidelines: "prefer deterministic table tests with explicit inputs and exact outputs, and avoid coverage-only, fake stress, timing, or implementation-detail tests".
📝 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.
| assert.Equal(t, bodyLengthBeforeCancel, responseWriter.Body.Len()) | |
| assert.GreaterOrEqual(t, responseWriter.Body.Len(), bodyLengthBeforeCancel) |
🤖 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/aws/relay_aws_test.go` at line 448, Update the final
body-length assertion in the cancellation test to verify that
responseWriter.Body.Len() is at least bodyLengthBeforeCancel, rather than
requiring exact equality. Keep the existing partial-usage and "[DONE]"
assertions unchanged.
Source: Coding guidelines
合并上游 16 个提交,主要是 token Auto 分组 (QuantumNous#6590)、deepseek responses API (QuantumNous#6562)、Bedrock 客户端断开取消 (QuantumNous#6589)、分层重试计费加固 (QuantumNous#6518/QuantumNous#6570)、 zstd 请求解压 (QuantumNous#6545)、OIDC 自定义显示名 (QuantumNous#6012)、日志暴露 stream_status (QuantumNous#6558)。 33 处冲突的处理: - 11 个 legacy channel adaptor:上游删除 panic 之后的死代码以配合新增的 go vet CI,本仓早已把整段 panic 换成返回 ErrLegacyAdaptorNotImplemented, 已达成同一目的且不会让中继链路 panic,保留本仓实现。 - relay-aws.go 流式循环:两侧改动正交,合并保留 —— 上游的 ctx.Done() 取消 分支(客户端断开时不再空转上游),加本仓的 finalizeClaudeOnError(提前 返回时补发流终止符,否则 Claude 格式调用方会一直挂着)。 - 7 个 locale:两侧各自插入相邻 key,按字母序归并;上游 24 个新 key 与本仓 1278 个 fork key 全部保留,逐一核对无丢失。 - keys / oauth / profile 前端:上游 Auto 分组为主体,叠加本仓改动; account-bindings-tab 与 oauth/$provider 保留本仓实现,理由见下。 测试取舍: - 不引入 model/token_auto_groups_cache_test.go:它依赖上游的 truncateTables, 该助手全局清表,与本仓行级清理原则冲突(共享库里有开发数据)。 - 不携带上游 controller/token_test.go:其迁移兼容测试会替换 model.DB 并在 cleanup 里关闭,后续用 harness 连接的测试会拿到已关闭的句柄。按既有约定, token_auto_groups_test.go 需要的四个助手放进 zz_upstream_test_shims_test.go, 其中 openTokenControllerTestDB 增加了 model.DB/LOG_DB 的成对保存还原。 本仓原 token_test.go 的 controller 行为测试迁到 gen_ctrl_token_handlers_test.go,与上游文件名脱钩以免再冲突。 - 三处断言随上游行为更新:stream_status 现对日志所有者可见;deepseek 的 ConvertOpenAIResponsesRequest 已实现不再返回错误;计费路径判定收紧为 「标记之外还需带对应 usage payload」,并补了三个反向用例锁住该语义。 遗留:oauth 绑定回调仍用 window.opener 判定 bind/login,上游 QuantumNous#6425 已改为 sessionStorage 标记 + state 比对。该修复要求 popup 先以 about:blank 打开再打 标记,本仓四个内置 provider 走 window.open(url) 直开,直接套用会让绑定永远 判成 login,故本次未采纳,需单独改造。
合并官方上游 Calcium-Ion/new-api main 分支的 10 个新 commit: - Feat/auto group (QuantumNous#6590) - fix(aws): cancel Bedrock requests on client disconnect (QuantumNous#6589) - fix(billing): harden tiered retry group-switch billing (QuantumNous#6570) - fix(billing): settle tiered retries with final group (QuantumNous#6518) - feat: deepseek responses api (QuantumNous#6562) - fix(oauth): stop treating a foreign window.opener as a bind flow (QuantumNous#6425) - fix(relay): preserve multipart image edits for New API channels (QuantumNous#6559) - feat(logs): expose stream status to log owners (QuantumNous#6558) - feat: support zstd request decompression (QuantumNous#6545) - style: use text-sm for public header nav links (QuantumNous#6557) 冲突解决: model/option.go 的 switch case 区域,双方各自新增了选项注册 (我方 GroupPassThrough + 官方 MaxTokenAutoGroups),取并集保留两者。 其余文件均自动合并成功。 验证: go build + go test 全过(relay/helper、middleware、controller)
…s / 84 files) 上游本次内容: - Feat/auto group (QuantumNous#6590):令牌自动分组(含前端 auto-group 编排 UI 与后端组选择) - fix(aws): Bedrock 请求在客户端断连时取消 (QuantumNous#6589) - fix(relay): 设置 Request.GetBody,让 HTTP/2 传输在上游 stream reset 后可透明重试 (QuantumNous#6249) - refactor(relay): 把 replay 元数据移到请求体上(RelayInfo.UpstreamRequestBodySize 移除) 冲突 1 处,纯新增函数相邻,两侧保留: - relay/channel/api_request.go:我方 upstreamRequestIDFromHeaders 与上游 keepUpstreamRedirectResponse
Brings in from QuantumNous/new-api: - fix: 修复兑换码额度精度损失 (QuantumNous#6685) - feat(rate-limit): user critical rate limit for access token and aff transfer routes - fix: test Claude/Gemini endpoints with native request format (QuantumNous#6698) - feat(channels): refine fetched model categorization (QuantumNous#6632) - security: atomic access-token rotation and aff updates (merge commit from fork) - refactor(relay): move replay metadata onto request bodies - fix(relay): set Request.GetBody for transparent HTTP/2 retry (QuantumNous#6249) - Feat/auto group (QuantumNous#6590) - fix(aws): cancel Bedrock requests on client disconnect (QuantumNous#6589) Conflicts resolved: - controller/model.go: adopt upstream's unified group-model loop with FormatMatchingModelName token-limit matching; keep fork's hiddenMappedModels filter inside the loop - model/user.go: keep fork's BatchDeleteUsers; adopt upstream's atomic gorm.Expr-based inviteUser - relay/channel/task/sora/adaptor_test.go: keep both fork's New API Video channel tests and upstream's replayable-body test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZteapnbTWdP52n7wZ6NsN
* v1.0.0-rc.24: (117 commits) CI: enhance release synchronization workflow with optional file syncing fix: 修复兑换码额度精度损失 (QuantumNous#6685) feat(rate-limit): add user critical rate limit middleware for access token and aff transfer routes fix: test Claude/Gemini endpoints with native request format (QuantumNous#6698) feat(channels): refine fetched model categorization (QuantumNous#6632) Merge commit from fork refactor(relay): move replay metadata onto request bodies fix(relay): set Request.GetBody so the HTTP/2 transport can transparently retry after an upstream stream reset (QuantumNous#6249) Feat/auto group (QuantumNous#6590) fix(aws): cancel Bedrock requests on client disconnect (QuantumNous#6589) fix(billing): harden tiered retry group-switch billing (QuantumNous#6570) fix(billing): settle tiered retries with final group (QuantumNous#6518) feat: deepseek responses api (QuantumNous#6562) style: use text-sm for public header nav links to match other nav components (QuantumNous#6557) fix(oauth): stop treating a foreign window.opener as a bind flow (QuantumNous#6425) fix(relay): preserve multipart image edits for New API channels (QuantumNous#6559) feat(logs): expose stream status to log owners (QuantumNous#6558) feat: support zstd request decompression (QuantumNous#6545) fix: preserve Qwen thinking_budget passthrough (QuantumNous#5836) feat(oidc): 支持自定义 OIDC 登录显示名称 (QuantumNous#6012) ... # Conflicts: # service/text_quota.go # web/src/features/models/components/drawers/model-mutate-drawer.tsx # web/src/features/pricing/components/model-details.tsx # web/src/features/pricing/lib/price.ts
Important
📝 变更描述 / Description
AWS Bedrock 请求原先使用独立的后台 Context,客户端断开后,上游生成仍会继续,最终可能按照完整 Token 用量结算。
本次修改让 Bedrock SDK 请求继承客户端请求 Context,同时保留
RelayTimeout:RelayTimeout保持原有重试行为。usage_billing_path的日志判定,使审计标签与实际采用的BillingUsage一致,不改变计费数值。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
验证范围包括 Context 继承、客户端取消、重试控制、流关闭、部分 usage 保留及日志路径判定。