Skip to content

fix(proxy): stop empty 503 bodies becoming Codex Unknown error (#452)#463

Merged
Wibias merged 3 commits into
lidge-jun:devfrom
Wibias:fix/issue-452-opaque-503
Jul 25, 2026
Merged

fix(proxy): stop empty 503 bodies becoming Codex Unknown error (#452)#463
Wibias merged 3 commits into
lidge-jun:devfrom
Wibias:fix/issue-452-opaque-503

Conversation

@Wibias

@Wibias Wibias commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Test plan

Summary by CodeRabbit

  • Bug Fixes
    • Improved error responses when upstream services return empty or unsuccessful responses.
    • Preserved relevant status details and validated retry timing information.
    • Standardized shutdown responses as JSON and included retry guidance.
    • Improved handling of streaming errors and truncated streams.
  • Diagnostics
    • Added optional debug logging for outbound requests and stream failures while redacting sensitive credentials.
  • Tests
    • Added coverage for upstream errors, shutdown behavior, retry headers, and diagnostic redaction.

Wrap empty/non-JSON ChatGPT passthrough failures and drain responses as JSON so Codex always gets error.message. Also add openai-chat provider debug diagnostics for lidge-jun#452.
@github-actions github-actions Bot added the bug Something isn't working label Jul 25, 2026
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR standardizes empty-body upstream failures and server-draining responses as JSON 503/429-compatible envelopes, preserves validated retry headers, and adds redacted OpenAI chat request and stream diagnostics with unit and integration coverage.

Changes

Error handling and diagnostics

Layer / File(s) Summary
Upstream passthrough error handling
src/server/responses/passthrough-error.ts, src/server/responses/core.ts, tests/issue-452-empty-503.test.ts
Non-2xx upstream responses are handled before normal passthrough processing. Empty bodies become JSON errors, valid Retry-After values are preserved, combo failures use typed envelopes, and passthrough behavior is tested across response and chat-completion routes.
Draining response centralization
src/server/index.ts, tests/issue-452-empty-503.test.ts
Draining exits use a shared CORS-enabled JSON 503 response with Retry-After: 5 across HTTP and WebSocket routes, with integration coverage.
OpenAI chat request and stream diagnostics
src/adapters/openai-chat.ts, tests/issue-452-empty-503.test.ts
Debug logging records redacted request metadata, inline upstream stream errors, and truncated streams while preserving terminal error events. Tests verify host logging and sensitive-path redaction.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ResponsesCore
  participant Upstream
  participant ErrorFormatter
  Client->>ResponsesCore: Send passthrough request
  ResponsesCore->>Upstream: Fetch upstream response
  Upstream-->>ResponsesCore: Return non-2xx status and body
  ResponsesCore->>ErrorFormatter: Format status, body, and retry headers
  ErrorFormatter-->>Client: Return structured JSON error
Loading

Possibly related PRs

Suggested reviewers: lidge-jun, ingwannu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main fix: empty 503 error bodies are normalized so Codex no longer surfaces Unknown error.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Wibias added 2 commits July 26, 2026 00:02
Preserve non-empty upstream 400/5xx bytes and headers so Codex pool-retry activation and Claude Retry-After fidelity stay intact. Empty bodies still become JSON for lidge-jun#452.
Normalized empty-body passthrough errors keep a validated Retry-After with application/json. openai-chat diagnostics log only the host so Cloudflare account paths never appear.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/server/responses/passthrough-error.ts`:
- Around line 4-8: Extract the duplicated Retry-After validation into a shared
helper, using the existing implementation in core.ts as the single source of
truth. Remove the local sanitizedRetryAfter function from passthrough-error.ts
and import the shared helper, preserving the 128-character limit and
parseRetryAfterMs behavior for both passthrough and combo failures.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 61cb37ac-110c-472f-ad95-488b93de0053

📥 Commits

Reviewing files that changed from the base of the PR and between 4e5d010 and fc6bc25.

📒 Files selected for processing (5)
  • src/adapters/openai-chat.ts
  • src/server/index.ts
  • src/server/responses/core.ts
  • src/server/responses/passthrough-error.ts
  • tests/issue-452-empty-503.test.ts

Comment on lines +4 to +8
function sanitizedRetryAfter(value: string | null | undefined, now: number): string | undefined {
const trimmed = value?.trim();
if (!trimmed || trimmed.length > 128) return undefined;
return parseRetryAfterMs(trimmed, now) !== undefined ? trimmed : undefined;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n "sanitizedRetryAfter" -g '*.ts' -B2 -A6

Repository: lidge-jun/opencodex

Length of output: 157


🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files 'src/server/responses/passthrough-error.ts' 'src/**/core.ts' 'src/**/failover.ts' | sed 's#^`#FILE` #'
echo '---'
rg -n "sanitizedRetryAfter|parseRetryAfterMs|formatPassthroughUpstreamError|passthrough-error" src -g '*.ts' -A4 -B4

Repository: lidge-jun/opencodex

Length of output: 12117


Extract sanitizedRetryAfter into a shared helper. src/server/responses/core.ts:291-294 and src/server/responses/passthrough-error.ts:4-7 duplicate the same Retry-After validation. Keep one source of truth so the 128-char guard and parse behavior can’t drift between passthrough and combo failures. Import it from a shared module instead of maintaining two copies.

🤖 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 `@src/server/responses/passthrough-error.ts` around lines 4 - 8, Extract the
duplicated Retry-After validation into a shared helper, using the existing
implementation in core.ts as the single source of truth. Remove the local
sanitizedRetryAfter function from passthrough-error.ts and import the shared
helper, preserving the 128-character limit and parseRetryAfterMs behavior for
both passthrough and combo failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant