Skip to content

Mask upstream errors from downstream users - #5444

Closed
haveagoodday1205-png wants to merge 2 commits into
QuantumNous:mainfrom
haveagoodday1205-png:codex/upstream-error-503-logmask
Closed

Mask upstream errors from downstream users#5444
haveagoodday1205-png wants to merge 2 commits into
QuantumNous:mainfrom
haveagoodday1205-png:codex/upstream-error-503-logmask

Conversation

@haveagoodday1205-png

@haveagoodday1205-png haveagoodday1205-png commented Jun 11, 2026

Copy link
Copy Markdown

Summary

  • Return upstream-origin relay errors to downstream clients as HTTP 503 with a generic service-unavailable message.
  • Preserve raw upstream details for admin/server logs, including real status codes and upstream/channel metadata.
  • Mask upstream details from user-visible token/self logs while keeping local New API errors unchanged.

Verification

  • go test ./types ./service ./model -run 'TestApplyDownstreamNewAPIErrorPolicy|TestRelayErrorHandler|TestFormatUserLogs'
  • Live smoke on 178 with /responses: client responses returned 503 generic message; /api/log/token showed masked 503 entries; raw DB logs preserved upstream/admin details.

Summary by CodeRabbit

Release Notes

  • New Features

    • Upstream provider errors now return HTTP 503 with a generic "Service temporarily unavailable" message to protect sensitive error details.
    • User-visible logs mask upstream error information while retaining full details in server logs for troubleshooting.
  • Documentation

    • Added upstream error masking policy documentation.
  • Tests

    • Added comprehensive test coverage for error masking and policy application.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR implements an upstream error masking policy that classifies provider errors as upstream, applies a downstream policy to mask them as HTTP 503 for external clients, sanitizes user-visible logs while preserving admin details, and systematically marks all relay provider errors across OpenAI, Claude, Gemini, and other handlers.

Changes

Upstream Error Masking and Classification

Layer / File(s) Summary
Error type system and downstream policy framework
types/error.go, types/error_policy_test.go
Adds ErrorCodeServiceUnavailable constant, upstreamError flag to NewAPIError, and three policy functions (MarkAsUpstreamError, IsUpstreamError, ApplyDownstreamNewAPIErrorPolicy) that classify errors and apply downstream masking with tests verifying local errors are preserved and upstream statuses (401/403/429/502) map to 503 with a fixed message.
Service-level error classification and wrapping
service/error.go, service/violation_fee.go, service/error_test.go
RelayErrorHandler defers MarkAsUpstreamError to mark all relay errors as upstream; WrapAsViolationFeeGrokCSAM and NormalizeViolationFeeError conditionally mark wrapped errors as upstream; test verifies structured upstream error handling through downstream policy application.
Relay handler upstream error marking across all providers
relay/channel/openai/*.go, relay/channel/claude/*.go, relay/channel/gemini/*.go, relay/channel/ali/*.go, relay/channel/palm/*.go, relay/channel/tencent/*.go, relay/channel/zhipu/*.go, relay/channel/jimeng/*.go, relay/channel/minimax/*.go
Systematic wrapping of provider-specific errors with MarkAsUpstreamError across all relay handlers for OpenAI, Claude, Gemini, Ali, PaLM, Tencent, Zhipu, Jimeng, and MiniMax to consistently classify upstream failures before propagating to the main controller.
Main relay controller downstream policy and error logging
controller/relay.go
Routes errors through ApplyDownstreamNewAPIErrorPolicy to mask upstream failures as 503 for downstream clients; applies policy-transformed responses across relay formats (OpenAI/Claude/WebSocket); logs upstream error classification in database payload (upstream_error, client_status_code: 503, optional upstream_status_code) for operational tracking.
User-visible log sanitization and masking
model/log.go, model/log_test.go
Log formatting detects upstream error logs and masks user-visible content by rewriting generic message, clearing upstream identifiers, and removing upstream/client fields from JSON while admin logs retain full details; tests verify masking of upstream errors and preservation of local errors.
Policy documentation and operational guidance
docs/upstream-error-503-logmask.md
Comprehensive documentation describing the upstream error 503 masking behavior, affected backend components, test/build/deployment commands, verification procedures, rollback guidance, and git workflow for upstream sync.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • QuantumNous/new-api#1833: Updates Deepseek's DoResponse dispatch to route RelayFormatClaude responses through Claude handlers, which in this PR were updated to wrap upstream errors with MarkAsUpstreamError in relay/channel/claude/relay-claude.go.

Suggested reviewers

  • seefs001
  • Calcium-Ion

Poem

🐰 Upstream errors masked in mist,
Provider woes now 503'd,
Admin logs keep secrets still,
While clients see the gentle hill,
Error flows through policy's gate,
Downstream users never wait!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly and accurately reflects the main objective: masking upstream errors from downstream users. The title is clear, concise, and describes the primary change throughout the changeset.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
docs/upstream-error-503-logmask.md (1)

52-55: 💤 Low value

Consolidate repetitive sentence structure for better readability.

Three successive sentences in the file list all begin with "Adds", which reduces readability. Consider rephrasing to vary the sentence structure.

 - `types/error.go`
   - Adds `upstreamError` marker on `NewAPIError`.
-  - Adds `ApplyDownstreamNewAPIErrorPolicy`.
-  - Adds `MarkAsUpstreamError` and `IsUpstreamError`.
+  - Defines `ApplyDownstreamNewAPIErrorPolicy`, `MarkAsUpstreamError`, and `IsUpstreamError` functions.
🤖 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 `@docs/upstream-error-503-logmask.md` around lines 52 - 55, The three
consecutive bullet sentences that each begin with "Adds" are repetitive; update
the description for `types/error.go` to vary sentence structure by combining
related items and using alternative verbs—e.g., "Introduces an `upstreamError`
marker on `NewAPIError` and adds helper functions `MarkAsUpstreamError` and
`IsUpstreamError`," and then mention "implements
`ApplyDownstreamNewAPIErrorPolicy`" as a separate clause—so the list reads
smoothly and avoids three successive "Adds" starters while still referencing
`NewAPIError`, `ApplyDownstreamNewAPIErrorPolicy`, `MarkAsUpstreamError`, and
`IsUpstreamError`.
🤖 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.

Nitpick comments:
In `@docs/upstream-error-503-logmask.md`:
- Around line 52-55: The three consecutive bullet sentences that each begin with
"Adds" are repetitive; update the description for `types/error.go` to vary
sentence structure by combining related items and using alternative verbs—e.g.,
"Introduces an `upstreamError` marker on `NewAPIError` and adds helper functions
`MarkAsUpstreamError` and `IsUpstreamError`," and then mention "implements
`ApplyDownstreamNewAPIErrorPolicy`" as a separate clause—so the list reads
smoothly and avoids three successive "Adds" starters while still referencing
`NewAPIError`, `ApplyDownstreamNewAPIErrorPolicy`, `MarkAsUpstreamError`, and
`IsUpstreamError`.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cfe795dd-b523-4b01-86d8-a693461fcd95

📥 Commits

Reviewing files that changed from the base of the PR and between 6f41542 and e578441.

📒 Files selected for processing (23)
  • controller/relay.go
  • docs/upstream-error-503-logmask.md
  • model/log.go
  • model/log_test.go
  • relay/channel/ali/image.go
  • relay/channel/ali/rerank.go
  • relay/channel/claude/relay-claude.go
  • relay/channel/gemini/relay-gemini.go
  • relay/channel/jimeng/image.go
  • relay/channel/minimax/image.go
  • relay/channel/openai/chat_via_responses.go
  • relay/channel/openai/relay-openai.go
  • relay/channel/openai/relay_responses.go
  • relay/channel/openai/relay_responses_compact.go
  • relay/channel/palm/relay-palm.go
  • relay/channel/tencent/relay-tencent.go
  • relay/channel/zhipu/relay-zhipu.go
  • relay/channel/zhipu_4v/image.go
  • service/error.go
  • service/error_test.go
  • service/violation_fee.go
  • types/error.go
  • types/error_policy_test.go

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