refactor(server): add ErrorResponse::internal_server_error and collapse the eight hand-rolled 500s
- Dominant language
- Rust
- Stars
- 467
- Forks
- 54
- Avg merge
- 4h 25m
- Merged PRs (30d)
- 310
Description
## Summary
Add a named `ErrorResponse::internal_server_error` constructor and route the eight hand-rolled HTTP 500 sites through it, so the 500 case is expressed the same way as the other status codes.
## Background
`ErrorResponse` already exposes named constructors for its other error statuses, but the 500 case has none. Each 500 site instead repeats the same two lines: `ErrorResponse::new(message, "server_error")` followed by `response.status = StatusCode::INTERNAL_SERVER_ERROR`. Besides the duplication, a site that forgets the status line silently downgrades a 500 to the `new` default of 400, which is exactly the defect tracked in #1695.
## Proposed Solution
Add `ErrorResponse::internal_server_error(message)` next to the existing four constructors, matching their doc-comment style, then replace the eight call sites with it. No behavior change.
## Implementation Notes
- Constructors to mirror: `src/server/types/response.rs:500` (`service_unavailable`), `:515` (`gateway_timeout`), `:535` (`not_supported`), `:549` (`not_implemented`). The base `ErrorResponse::new` is at `:489`.
- Hand-rolled 500 sites to convert: `src/server/routes/rerank.rs:70-75`, `:279-281`, `:285-294`, `:305-310`; `src/server/routes/embeddings.rs:75-81`, `:365-368`, `:383-388`; `src/server/gcp_compat.rs:274-277`.
- Confirmed pattern in `rerank.rs`: `ErrorResponse::new(format!("rerank inference failed: {message}"), "server_error")` then `response.status = StatusCode::INTERNAL_SERVER_ERROR`.
- Pairs with #1695: once this lands, the audio routes can adopt the same constructor.
## Acceptance Criteria
- [ ] One `internal_server_error` constructor added beside its four siblings.
- [ ] Eight call sites converted, with status codes and error strings unchanged.
---
## Original Suggestion
### Title: refactor(server): add ErrorResponse::internal_server_error and collapse the eight hand-rolled 500s
`ErrorResponse` has named constructors for 503, 504, 501, and not-supported — but the 500 case is hand-rolled at eight sites, each repeating the same two-line `new(...)` + `status = INTERNAL_SERVER_ERROR` dance.
## Evidence
- Existing constructors: `src/server/types/response.rs:500` (`service_unavailable`), `:515` (`gateway_timeout`), `:535` (`not_supported`), `:549` (`not_implemented`)
- Hand-rolled 500s: `src/server/routes/rerank.rs:70-75`, `:279-281`, `:285-294`, `:305-310`; `src/server/routes/embeddings.rs:75-81`, `:365-368`, `:383-388`; `src/server/gcp_compat.rs:274-277`
## Suggested fix
Add `ErrorResponse::internal_server_error(message)` next to its four siblings (same doc-comment style) and replace the eight call sites mechanically. Pairs naturally with the audio-route status fix, which can then use it too.
## Acceptance criteria
- [ ] One constructor, eight call sites converted, zero behavior change
Contributor guide
Research direction
Start with the constructors in src/server/types/response.rs, then inspect the eight listed sites in src/server/routes/rerank.rs, src/server/routes/embeddings.rs, and src/server/gcp_compat.rs. Compare each existing 500 response with the neighboring named constructors. Done means one constructor is added and all eight sites use it without changing status codes or error strings.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, backend
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 84/100