Skip to content

fix: preserve HTTPException status in proxy service error wrapping#768

Open
jayesh-bansal wants to merge 1 commit into
Alishahryar1:mainfrom
jayesh-bansal:fix/preserve-http-exception-status
Open

fix: preserve HTTPException status in proxy service error wrapping#768
jayesh-bansal wants to merge 1 commit into
Alishahryar1:mainfrom
jayesh-bansal:fix/preserve-http-exception-status

Conversation

@jayesh-bansal

Copy link
Copy Markdown

When provider resolution fails (e.g. a provider API key is missing), api.dependencies._resolve_with_registry raises a curated HTTPException 503 whose detail is an actionable config hint ("NVIDIA_NIM_API_KEY is not set. Add it to your .env file."). ClaudeProxyService.create_message and count_tokens caught it in the generic except Exception handler and re-wrapped it as HTTPException(500, "503: ") - clients saw an opaque 500 Internal Server Error with the real status embedded in the message text (as reported in #754).

Re-raise HTTPException before the generic handler so the curated status and detail reach the client unchanged. Claude Code then receives a proper 503 and can surface the configuration hint instead of failing sub-agent runs with a generic 500.

  • api/services.py: add except HTTPException: raise ahead of the generic wrapper in create_message and count_tokens
  • tests: 503 status and detail preserved end-to-end for both paths

Refs #754

When provider resolution fails (e.g. a provider API key is missing),
api.dependencies._resolve_with_registry raises a curated HTTPException
503 whose detail is an actionable config hint ("NVIDIA_NIM_API_KEY is
not set. Add it to your .env file."). ClaudeProxyService.create_message
and count_tokens caught it in the generic `except Exception` handler and
re-wrapped it as HTTPException(500, "503: <detail>") - clients saw an
opaque 500 Internal Server Error with the real status embedded in the
message text (as reported in Alishahryar1#754).

Re-raise HTTPException before the generic handler so the curated status
and detail reach the client unchanged. Claude Code then receives a
proper 503 and can surface the configuration hint instead of failing
sub-agent runs with a generic 500.

- api/services.py: add `except HTTPException: raise` ahead of the
  generic wrapper in create_message and count_tokens
- tests: 503 status and detail preserved end-to-end for both paths

Refs Alishahryar1#754
Copilot AI review requested due to automatic review settings June 11, 2026 05:13

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR ensures curated HTTPExceptions raised by provider resolution/token counting propagate unchanged (status code + detail), instead of being re-wrapped into generic 500 responses.

Changes:

  • Re-raise HTTPException in ClaudeProxyService.create_message and count_tokens to preserve original status/detail.
  • Add regression tests asserting HTTPException status/detail are preserved.
  • Bump project version to 1.2.42.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/api/test_safe_logging.py Adds tests verifying HTTPException is not re-wrapped as 500 in create_message/count_tokens.
api/services.py Updates exception handling to re-raise HTTPException unchanged.
pyproject.toml Increments project version for release.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread api/services.py
Comment on lines 210 to +216
except ProviderError:
raise
except HTTPException:
# Provider resolution raises curated HTTPExceptions (e.g. 503 with a
# config hint when a provider key is missing); preserve their status
# instead of re-wrapping as a generic 500.
raise
Comment on lines +151 to +152
"""Curated HTTPExceptions (e.g. 503 missing-key hints) must not become 500s."""
detail = "NVIDIA_NIM_API_KEY is not set. Add it to your .env file."
@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a bug where HTTPException instances raised during provider resolution (e.g., a curated 503 with a config hint) were being silently caught by the generic except Exception handler in ClaudeProxyService and re-wrapped as opaque 500 errors. Adding except HTTPException: raise before the generic handler restores the original status and detail to the client.

  • api/services.py: Adds except HTTPException: raise ahead of the except Exception wrapper in both create_message and count_tokens, so curated errors from provider resolution pass through unchanged.
  • tests/api/test_safe_logging.py: Adds two regression tests confirming 503 status and detail are preserved end-to-end for both paths.

Confidence Score: 5/5

Safe to merge — the change is a two-line guard that lets already-curated HTTPExceptions pass through unmodified, with no impact on the existing ProviderError or generic Exception paths.

The fix is minimal and surgical: inserting except HTTPException: raise above the generic handler is the standard Python idiom for this pattern. ProviderError is a plain Exception subclass with no relationship to HTTPException, so exception ordering is correct and there is no overlap between the three handlers. Both new paths in create_message and count_tokens are covered by dedicated regression tests that assert the exact status code and detail string are preserved.

No files require special attention.

Important Files Changed

Filename Overview
api/services.py Adds except HTTPException: raise before the generic Exception handler in both create_message and count_tokens — minimal, targeted fix with no logic side-effects.
tests/api/test_safe_logging.py Two new regression tests verify that 503 HTTPExceptions are re-raised with their original status code and detail for both create_message and count_tokens.
pyproject.toml Version bump from 1.2.41 to 1.2.42.
uv.lock Lock file updated to reflect the version bump; no dependency changes.

Sequence Diagram

sequenceDiagram
    participant Client
    participant ClaudeProxyService
    participant ProviderResolver

    Client->>ClaudeProxyService: create_message / count_tokens
    ClaudeProxyService->>ProviderResolver: resolve provider
    ProviderResolver-->>ClaudeProxyService: raises HTTPException(503, "key missing hint")

    note over ClaudeProxyService: Before fix: caught by except Exception<br/>→ re-wrapped as HTTPException(500, "503: ...")
    note over ClaudeProxyService: After fix: caught by except HTTPException: raise<br/>→ original 503 + detail passed through

    ClaudeProxyService-->>Client: HTTPException(503, "NVIDIA_NIM_API_KEY is not set...")
Loading

Reviews (1): Last reviewed commit: "fix: preserve HTTPException status in pr..." | Re-trigger Greptile

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