Skip to content

fix(proxy): 首包前不可重试失败补齐 OpenAI 直连与入站 WS 两条路径#333

Merged
james-6-23 merged 1 commit into
mainfrom
fix/first-token-failed-remaining-paths
Jul 8, 2026
Merged

fix(proxy): 首包前不可重试失败补齐 OpenAI 直连与入站 WS 两条路径#333
james-6-23 merged 1 commit into
mainfrom
fix/first-token-failed-remaining-paths

Conversation

@james-6-23

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

Copy link
Copy Markdown
Owner

说明

#332(含 #318)只覆盖了 Responses/ChatCompletions 主流式路径,本 PR 补齐剩余两条:

OpenAI Responses API 直连分支(IsOpenAIResponsesAPI)

与主路径同构的三处修改:回调拦截首 token 前的 response.failed、收尾 flush 加 wroteAnyBody 守卫(避免空 flush 提前提交 200 header)、循环外覆盖 Content-Type 后按 outcome.logStatusCode 返回 JSON 错误。此前该分支把失败事件原样写进 200 流。

入站 WebSocket(responses_ws.go)

WS 升级完成后没有 HTTP 状态码可用,等价语义为:

  • 首 token 前不可重试response.failed(如 context_length_exceeded)不透传原始失败帧,改写结构化 {"type":"error"} 帧后按错误类别用非正常 close code 关闭:429→1013(TryAgainLater),其余 4xx→1008(PolicyViolation),5xx→1011(InternalServerErr)。下游中转/计费方不再把失败会话当正常收尾。
  • 可重试失败不拦截:silent retry 开启时仍走既有换号重试;关闭时按既有约定原样透传失败帧(受 TestResponsesWebSocketSilentRetryDisabledRelaysRetryableFailure 保护)。

测试

  • 新增 responsesWSCloseCodeForStatus 映射单测
  • 新增端到端 WS 测试:mock 上游首包前返回 context_length_exceeded,断言客户端收到 error 帧(携带上游 message)+ 1008 close + 不换号重试
  • go build ./...go test ./... 全部通过

Summary by CodeRabbit

  • Bug Fixes
    • Improved how early upstream failures are reported so users now get a proper error response instead of an apparent success.
    • Fixed streaming behavior to avoid sending incomplete or misleading responses when a request fails before any content is produced.
    • WebSocket streams now close with clearer status-specific codes, making retry and policy failures easier to interpret.

#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 + 不换号重试
@james-6-23 james-6-23 merged commit 53c6408 into main Jul 8, 2026
8 checks passed
@james-6-23 james-6-23 deleted the fix/first-token-failed-remaining-paths branch July 8, 2026 14:16
@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: ede10453-f1e5-45ff-90f7-883b5b44cc8e

📥 Commits

Reviewing files that changed from the base of the PR and between c653146 and c9a2dfa.

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

📝 Walkthrough

Walkthrough

This change updates HTTP SSE and WebSocket streaming handlers for /v1/responses to detect non-retryable response.failed events occurring before the first token and, when no body has been written, abort streaming and return a proper HTTP-style error (JSON for HTTP, structured error frame plus mapped close code for WebSocket) instead of forwarding the failure as a successful stream. Tests validate close-code mapping and the end-to-end WebSocket error flow.

Changes

Response.failed abort handling

Layer / File(s) Summary
SSE HTTP streaming abort and JSON error response
proxy/handler.go
Adds abortedForHTTPError flag, intercepts pre-first-token non-retryable response.failed to stop SSE forwarding, skips Flush() when nothing was written, and returns a JSON upstream_error response with the correct status code.
WebSocket abort, structured error frame, and close code mapping
proxy/responses_ws.go
Adds abortedForErrorClose flag, discards pending pre-first-token frames on non-retryable failure, writes a structured error payload, and adds responsesWSCloseCodeForStatus to map HTTP status codes to WebSocket close codes (429, 4xx, 5xx).
Tests for close-code mapping and non-retryable failure flow
proxy/response_failed_billing_test.go
Adds imports and two tests validating status-to-close-code mapping and the end-to-end WebSocket error/close behavior with no retry.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Handler as HTTP Handler
  participant WSHandler as WebSocket Handler
  participant Upstream

  Note over Client,Upstream: HTTP SSE stream, pre-first-token failure
  Client->>Handler: POST /v1/responses (stream)
  Handler->>Upstream: forward request
  Upstream-->>Handler: response.failed (non-retryable)
  Handler->>Handler: abortedForHTTPError=true, skip flush
  Handler-->>Client: JSON upstream_error (logStatusCode)

  Note over Client,Upstream: WebSocket stream, pre-first-token failure
  Client->>WSHandler: open WebSocket
  WSHandler->>Upstream: forward request
  Upstream-->>WSHandler: response.failed (non-retryable)
  WSHandler->>WSHandler: abortedForErrorClose=true, discard pending frames
  WSHandler->>WSHandler: map status to close code
  WSHandler-->>Client: structured error frame + WebSocket close
Loading

Possibly related PRs

  • james-6-23/codex2api#252: Both PRs modify /v1/responses streaming WebSocket/SSE handling around response.failed events occurring before the first token, altering buffered output management and error responses at the same handler code paths.
✨ 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-failed-remaining-paths

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.

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.

1 participant