ClickHouse / ClickHouse/clickhouse-rs
Strongly typed server response error
- Dominant language
- Rust
- Stars
- 559
- Forks
- 172
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 3
Description
### Use case
https://github.com/ClickHouse/clickhouse-rs/pull/426
### Describe the solution you'd like
Change `Error::BadResponse(String)` to `Error::ServerError(ServerError)` where `ServerError` is a new struct:
```rust
pub struct ServerError {
// Expose these fields as getters
code: u32,
name: Option,
message: String,
}
```
This should just be a loud breaking change so users know it happened. #426 suggested adding this as a new variant alongside `BadResponse(String)`, but that would change existing error handling in a way that is unintuitive and could break existing deployments.
There is still the possibility of failing to parse the response, however. I think we could return that as `Error::Other()` instead.
### Describe the alternatives you've considered
### Additional context
Contributor guide
Research direction
Start by locating the existing Error::BadResponse(String) definition and its response-parsing call sites, then read the linked PR #426 for the prior approach. Trace how server error code, name, and message are currently handled. Done means the error exposes the requested typed ServerError data, preserves the intended parse-failure behavior, and passes the relevant existing tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100