fix(server): return API-specific error responses - #320
Conversation
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
|
WalkthroughThe server now formats errors by endpoint, returns structured 404 responses for unknown paths, and documents the response envelopes. Tests cover malformed requests, semantic validation errors, empty messages, and unknown routes. ChangesEndpoint Error Response Standardization
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/test_switchyard_app_factory.py (1)
88-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the async test-client pattern.
Convert this test to
async def. Use an async ASGI client and await the request. Pytest automatic asyncio mode removes the need forTestClienthere.As per coding guidelines: “Use async-first APIs. If synchronous execution is necessary, use
asyncio.run().”🤖 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 `@tests/test_switchyard_app_factory.py` around lines 88 - 101, Convert test_unknown_path_uses_switchyard_error_envelope to async def, replace the synchronous TestClient context with the project’s async ASGI client pattern, and await the GET request while preserving the existing raise-server-exceptions setting and assertions.Source: Coding guidelines
tests/test_build_and_serve.py (1)
111-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
pytest-mockfor the Uvicorn stub.Replace the
monkeypatchfixture with themockerfixture for this general mock. Update the helper callers to passMockerFixture.As per coding guidelines: “Use
respxfor HTTP mocking andpytest-mockfor general mocking.”🤖 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 `@tests/test_build_and_serve.py` around lines 111 - 132, The _capture_uvicorn helper currently uses monkeypatch for a general mock; switch it to the pytest-mock mocker fixture and annotate it with MockerFixture. Update every caller of _capture_uvicorn to pass mocker, and replace the uvicorn.run monkeypatch with the corresponding mocker.patch while preserving captured arguments and behavior.Source: Coding guidelines
🤖 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 `@docs/cli_reference.md`:
- Around line 90-105: Update the error-code documentation in the
OpenAI-compatible section of the CLI reference to avoid attributing
empty_messages and internal_chain_error to the Rust server. Remove those codes
from the standalone Rust list, or clearly label the list as project-wide and
provide a separate list of Rust-specific codes.
---
Nitpick comments:
In `@tests/test_build_and_serve.py`:
- Around line 111-132: The _capture_uvicorn helper currently uses monkeypatch
for a general mock; switch it to the pytest-mock mocker fixture and annotate it
with MockerFixture. Update every caller of _capture_uvicorn to pass mocker, and
replace the uvicorn.run monkeypatch with the corresponding mocker.patch while
preserving captured arguments and behavior.
In `@tests/test_switchyard_app_factory.py`:
- Around line 88-101: Convert test_unknown_path_uses_switchyard_error_envelope
to async def, replace the synchronous TestClient context with the project’s
async ASGI client pattern, and await the GET request while preserving the
existing raise-server-exceptions setting and assertions.
🪄 Autofix
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: CHILL
Plan: Enterprise
Run ID: b373fe23-28df-459c-8fa4-c1de6469df2c
📒 Files selected for processing (4)
docs/cli_reference.mdswitchyard/server/switchyard_app.pytests/test_build_and_serve.pytests/test_switchyard_app_factory.py
| OpenAI Chat Completions, OpenAI Responses, and unknown URL paths return this | ||
| error body: | ||
|
|
||
| ```json | ||
| {"error": {"message": "...", "type": "...", "code": "..."}} | ||
| ``` | ||
|
|
||
| Anthropic Messages returns the Anthropic error body: | ||
|
|
||
| ```json | ||
| {"type": "error", "error": {"type": "...", "message": "..."}} | ||
| ``` | ||
|
|
||
| Common OpenAI-compatible `code` values include `invalid_body`, `empty_messages`, | ||
| `model_not_found`, `endpoint_not_found`, `upstream_error`, | ||
| `internal_chain_error`, and `context_length_exceeded`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect standalone-server error handling and error-code definitions.
fd -t f . crates/switchyard-server
rg -n -C 3 \
'endpoint_not_found|invalid_body|empty_messages|context_length_exceeded|Not Found' \
crates/switchyard-server || trueRepository: NVIDIA-NeMo/Switchyard
Length of output: 4971
🏁 Script executed:
#!/bin/bash
# Inspect the Rust error-envelope helpers, route handlers, and focused tests.
ast-grep outline crates/switchyard-server/src/lib.rs --match 'fn error_response' --view expanded
sed -n '780,970p' crates/switchyard-server/src/lib.rs
sed -n '1020,1060p' crates/switchyard-server/src/lib.rs
rg -n -C 4 'empty_messages|model_not_found|upstream_error|internal_chain_error|context_length_exceeded|error_response|render_error_response' crates/switchyard-server/src/lib.rs crates/switchyard-server/tests/server.rs
sed -n '70,115p' docs/cli_reference.mdRepository: NVIDIA-NeMo/Switchyard
Length of output: 26197
🏁 Script executed:
#!/bin/bash
# Determine whether every documented code exists in the Rust server and inspect
# the request-validation paths that could emit empty_messages or internal_chain_error.
rg -n -C 3 \
'empty_messages|internal_chain_error|invalid_body|model_not_found|endpoint_not_found|upstream_error|context_length_exceeded' \
. ':!target' || true
printf '\n-- focused Rust request and response tests --\n'
sed -n '1588,1695p' crates/switchyard-server/tests/server.rs
sed -n '560,645p' crates/switchyard-server/src/lib.rs
sed -n '470,520p' crates/switchyard-server/src/lib.rs
printf '\n-- exact error_response code arguments in Rust --\n'
python3 - <<'PY'
from pathlib import Path
import re
path = Path("crates/switchyard-server/src/lib.rs")
text = path.read_text()
for match in re.finditer(r'error_response\((.*?)\)', text, re.S):
args = match.group(1)
strings = re.findall(r'"([^"]+)"', args)
if strings:
print(strings[-1])
PYRepository: NVIDIA-NeMo/Switchyard
Length of output: 50380
🏁 Script executed:
#!/bin/bash
# Extract only the Rust server's documented-code occurrences and the request
# validation/dispatch paths. Avoid the broad repository output.
printf '%s\n' '-- Rust documented codes --'
for code in invalid_body empty_messages model_not_found endpoint_not_found upstream_error internal_chain_error context_length_exceeded; do
printf '%-25s ' "$code"
rg -n --count-matches "\"$code\"" crates/switchyard-server/src crates/switchyard-server/tests || true
done
printf '\n%s\n' '-- Rust request validation and dispatch --'
sed -n '560,640p' crates/switchyard-server/src/lib.rs
sed -n '1595,1660p' crates/switchyard-server/tests/server.rs
printf '\n%s\n' '-- Rust error-envelope tests --'
rg -n -C 8 'wire_format|AnthropicMessages|OpenAiResponses|invalid_body|endpoint_not_found' \
crates/switchyard-server/tests/server.rs crates/switchyard-server/src/lib.rsRepository: NVIDIA-NeMo/Switchyard
Length of output: 28725
Scope the error-code list to the correct server.
The Rust server uses both documented error envelopes, but it does not define empty_messages or internal_chain_error. Remove these codes from the standalone Rust section, or label the list as project-wide and document the Rust-specific codes separately.
🤖 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/cli_reference.md` around lines 90 - 105, Update the error-code
documentation in the OpenAI-compatible section of the CLI reference to avoid
attributing empty_messages and internal_chain_error to the Rust server. Remove
those codes from the standalone Rust list, or clearly label the list as
project-wide and provide a separate list of Rust-specific codes.
The Python server returns an OpenAI error response for this malformed Anthropic Messages request:
{"error":{"message":"Invalid JSON body","type":"invalid_request_error","code":"invalid_body"}}The Python server should return the Anthropic error response that Anthropic clients expect. Unknown URLs also return FastAPI's
{"detail":"Not Found"}response instead of Switchyard's OpenAI-compatible error response.Fix
The app factory now selects the error response from the requested API.
/v1/messagesreturns Anthropic error JSON, while Chat Completions and Responses continue to return OpenAI error JSON. Unknown URLs now return Switchyard's OpenAI-compatibleendpoint_not_foundresponse.Malformed Anthropic request:
{"type":"error","error":{"type":"invalid_request_error","message":"Invalid JSON body"}}Unknown URL:
{"error":{"message":"Not Found","type":"not_found","code":"endpoint_not_found"}}The CLI reference documents both response formats and the common OpenAI-compatible error codes.
How tested
uv run ruff check .,uv run mypy switchyard,cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings, andmake -C docs publishalso pass.Summary by CodeRabbit
New Features
Documentation
Bug Fixes