Skip to content

fix(proxy): 首包前 response.failed 返回真实错误码(含 #318 + 时序修复)#332

Merged
james-6-23 merged 2 commits into
mainfrom
fix/first-token-response-failed-http-error
Jul 8, 2026
Merged

fix(proxy): 首包前 response.failed 返回真实错误码(含 #318 + 时序修复)#332
james-6-23 merged 2 commits into
mainfrom
fix/first-token-response-failed-http-error

Conversation

@james-6-23

@james-6-23 james-6-23 commented Jul 8, 2026

Copy link
Copy Markdown
Owner

说明

本 PR 包含 #318 的原始提交(cherry-pick,保留 @JrCx7scC 署名),并在其上修复了一个使原方案在 HTTP 层不生效的时序 bug。合并后关闭 #318

Closes #318

#318 的问题

原方案在回调里中止 SSE 转发后,循环外的 c.JSON(4xx) 实际不生效:

  1. ReadSSEStream 返回后紧跟的 streamWriter.Flush() 在零写入时也会调用 flusher.Flush();
  2. gin 的 responseWriter.Flush() 第一行是 WriteHeaderNow() —— 200 + text/event-stream header 就此定型;
  3. 后续 c.JSON(400) 只能把 JSON 追加进一个 200 的 event-stream 响应体。

用 gin v1.12.0 + httptest 复刻该调用顺序实测:STATUS=200 CT="text/event-stream",下游中转层照样按 200 计费,且直连 SSE 客户端连原来能收到的 response.failed 事件都没了。

修复(第二个 commit)

  • 收尾 flush 加 wroteAnyBody 守卫:零写入时不触碰 transport flush,header 留到循环外真正响应时再提交(Responses / ChatCompletions 两处);
  • 回调命中拦截时置位 abortedForHTTPError,外层错误分支改用该标记,避免客户端已断开等零写入场景对死连接误写 JSON;
  • c.JSON 前覆盖 Content-Type: application/json; charset=utf-8(gin 的 JSON render 不覆盖已设置的 Content-Type);
  • 新增 wire 级测试:真实 HTTP 连接上断言首包前失败下游收到 4xx JSON、已产出首 token 时维持 200 SSE 收尾。

未覆盖(与 #318 保持一致的范围)

IsOpenAIResponsesAPI() 直连分支、responses_ws.go 入站 WS、image-generation 强制 HTTP 分支仍保持旧行为,后续单独处理。

测试

go build ./...go test ./... 全部通过。

Summary by CodeRabbit

  • Bug Fixes
    • Improved streaming error handling so early upstream failures now return a proper JSON error instead of ending with a misleading successful stream response.
    • Prevented incomplete SSE responses from being sent when no data has reached the client yet, reducing incorrect success outcomes.
    • Preserved normal streaming behavior once output has already started, so completed streams still finish as expected.

JrCx7scC and others added 2 commits July 8, 2026 20:55
流式 /v1/responses 与 /v1/chat/completions 在首 token 之前收到上游
response.failed(如 context_length_exceeded)时,现有代码把失败事件写进下游
SSE 并补 [DONE],HTTP 状态码停在 200。但此时 pending 尚未 flush、下游 200
header 其实还没发出(见 stream_flush_writer.go),完全可以返回真实的 4xx/5xx。

停在 200 + [DONE] 会被上游中转层误判为一次正常完成:例如 New API 的
pass-through Responses 渠道拿不到 usage,便按其本地预估的 input token 计费,
造成"上游 0 输出却按 input 收费"。#310 已让 context_length_exceeded 等确定性
客户端错误不再换号重试,但流式下游返回仍是 200 + [DONE];本 commit 补上这一半:
首 token 前、未向下游写任何内容、客户端未断开时,中止 SSE 转发,由循环外按
outcome.logStatusCode 返回 JSON 错误(与非流式路径一致)。

- 新增可单测的 shouldReturnHTTPErrorForResponseFailed
- Responses / ChatCompletions 两条主流式路径接入 + 循环后错误返回
- 加单元测试

注:image-generation 强制 HTTP 分支与 WebSocket 路径(responses_ws.go)存在
同一模式,本 PR 未改,留待维护者确认是否一并处理。
PR #318 在回调里中止 SSE 转发后,循环外的 c.JSON(4xx) 实际不生效:
ReadSSEStream 返回后紧跟的 streamWriter.Flush() 在零写入时也会调用
flusher.Flush(),而 gin 的 Flush 第一行就是 WriteHeaderNow(),200 +
text/event-stream header 就此定型,后续 c.JSON 只能把 JSON 追加进
一个 200 的 event-stream 响应体(实测复现:STATUS=200)。

修复:
- 收尾 flush 加 wroteAnyBody 守卫,零写入时不触碰 transport flush,
  header 留到循环外真正响应时再提交(Responses / ChatCompletions 两处)
- 回调命中拦截时置位 abortedForHTTPError,外层错误分支改用该标记,
  避免客户端已断开、写零 body 等场景误触发对死连接的 JSON 写入
- c.JSON 前覆盖 Content-Type 为 application/json(gin 的 JSON render
  不会覆盖已设置的 text/event-stream)
- 新增 wire 级测试:真实 HTTP 连接上断言首包前失败下游收到 4xx JSON、
  已产出首 token 时维持 200 SSE 收尾
@james-6-23 james-6-23 merged commit c653146 into main Jul 8, 2026
8 of 9 checks passed
@james-6-23 james-6-23 deleted the fix/first-token-response-failed-http-error branch July 8, 2026 14:05
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5d685a3e-bec1-41cc-a732-101d961b06ec

📥 Commits

Reviewing files that changed from the base of the PR and between 526db3c and 46b1af7.

📒 Files selected for processing (2)
  • proxy/handler.go
  • proxy/response_failed_billing_test.go

📝 Walkthrough

Walkthrough

This change adds a helper to detect when an upstream response.failed event arrives before the first token and before any downstream body is written, then wires it into both the /v1/responses and /v1/chat/completions streaming handlers to abort SSE forwarding and return a real JSON error status instead of HTTP 200 + [DONE]. Tests cover the helper and end-to-end wire behavior.

Changes

Early response.failed error handling

Layer / File(s) Summary
Shared detection helper
proxy/handler.go
Adds shouldReturnHTTPErrorForResponseFailed to check event type, first-token state, written-body state, and client-disconnect state.
/v1/responses streaming abort path
proxy/handler.go
Tracks abortedForHTTPError, resets pending SSE events and stops processing on early failure, conditionally flushes only when body was written, and returns JSON upstream error using the real status code.
/v1/chat/completions streaming abort path
proxy/handler.go
Applies the same abortedForHTTPError tracking, event-loop short-circuit, conditional flush, and JSON error response for the chat completions streaming flow.
Unit and wire-level tests
proxy/response_failed_billing_test.go
Adds table-driven tests for the helper and an httptest-based streamEpilogue simulation verifying 400 JSON errors before first token and unaffected 200 SSE responses after first token.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Handler as Streaming Handler
  participant Upstream

  Client->>Handler: Start streaming request
  Handler->>Upstream: Forward request
  alt response.failed before first token, no body written
    Upstream-->>Handler: response.failed event
    Handler->>Handler: shouldReturnHTTPErrorForResponseFailed() -> true
    Handler->>Handler: pending.Reset(), abortedForHTTPError=true
    Handler-->>Client: HTTP JSON error (real status code, message)
  else first token already produced
    Upstream-->>Handler: first token event
    Handler-->>Client: SSE chunk
    Upstream-->>Handler: response.failed event
    Handler->>Handler: shouldReturnHTTPErrorForResponseFailed() -> false
    Handler-->>Client: SSE continues, ends 200 + [DONE]
  end
Loading

Possibly related PRs

  • james-6-23/codex2api#252: Both PRs modify the streaming SSE forwarding/error handling in proxy/handler.go around early upstream response.failed events before the first token, including deferred/buffered SSE writing.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/first-token-response-failed-http-error

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

james-6-23 added a commit that referenced this pull request Jul 8, 2026
#318/#332 只覆盖了 Responses/ChatCompletions 主流式路径,本次补齐:

OpenAI Responses API 直连分支(IsOpenAIResponsesAPI):
- 回调拦截首 token 前的 response.failed,收尾 flush 加 wroteAnyBody 守卫,
  循环外覆盖 Content-Type 后按 outcome.logStatusCode 返回 JSON 错误,
  与主路径行为一致(此前该分支把失败事件写进 200 流)

入站 WebSocket(responses_ws.go):
- WS 升级后没有 HTTP 状态码可用,等价语义为:首 token 前不可重试的
  response.failed 不透传原始失败帧,改写结构化 error 帧后按错误类别
  用非正常 close code 关闭(429→1013,其余 4xx→1008,5xx→1011),
  下游不再把失败会话当正常收尾
- 可重试失败不拦截:silent retry 开启时仍换号重试,关闭时按既有约定
  原样透传失败帧(TestResponsesWebSocketSilentRetryDisabledRelaysRetryableFailure)

测试:
- 新增 close code 映射单测
- 新增端到端 WS 测试:断言 error 帧 + 1008 close + 不换号重试
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants