feat!: add IntoErrorPayload trait for structured JSON-RPC errors#42
feat!: add IntoErrorPayload trait for structured JSON-RPC errors#42
Conversation
Adds the public IntoErrorPayload trait with error_code, error_message, error_data, and into_error_payload methods. Adds convenience impls for ErrorPayload, InternalError<T>, String, &'static str, and (). ENG-2085 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Unifies Result and ResponsePayload conversion paths for the handler macro. Result<T, E: IntoErrorPayload> delegates to into_error_payload(). ResponsePayload<T, E> passes through unchanged. ENG-2085 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prepares for IntoErrorPayload by routing through the new internal IntoResponsePayload trait instead of From<Result<T, E>>. Also updates OutputResult handler bounds from ErrData: RpcSend to ErrData: IntoErrorPayload, as these changes are co-dependent. ENG-2085 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
BREAKING: Removes From<Result<T, E>> for ResponsePayload, From<T> for ErrorPayload<T> where T: Error + RpcSend, and ResponsePayload::convert_internal_error_msg. These are replaced by the IntoErrorPayload and IntoResponsePayload traits. ENG-2085 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ENG-2085 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ENG-2085 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ENG-2085 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Breaking changes in this release: - Handler Result<T, E> error types must implement IntoErrorPayload - String/&str errors now populate the message field (not data) - Removed From<Result> for ResponsePayload, From<T> for ErrorPayload, and convert_internal_error_msg Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Demonstrates custom error types with per-variant JSON-RPC error codes, messages, and structured error data via the IntoErrorPayload trait. ENG-2085 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the blanket `From<T>` impl on `InternalError<T>` so that wrapping a value requires explicit `InternalError(val)` construction. This prevents silent semantic shifts when switching between `String` (→ message field) and `InternalError<String>` (→ data field) as handler error types. Also gitignore /docs to prevent plan files from being committed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fraser999
left a comment
There was a problem hiding this comment.
All just non-blocking nits.
| let data = match &self { | ||
| Self::CrystalBallCloudy => serde_json::value::to_raw_value(&CloudyDetail { | ||
| visibility: "opaque", | ||
| suggestion: "try polishing the ball", | ||
| }) | ||
| .ok(), | ||
| Self::StarsMisaligned { sign } => { | ||
| serde_json::value::to_raw_value(&MisalignmentDetail { | ||
| sign: sign.clone(), |
There was a problem hiding this comment.
| let data = match &self { | |
| Self::CrystalBallCloudy => serde_json::value::to_raw_value(&CloudyDetail { | |
| visibility: "opaque", | |
| suggestion: "try polishing the ball", | |
| }) | |
| .ok(), | |
| Self::StarsMisaligned { sign } => { | |
| serde_json::value::to_raw_value(&MisalignmentDetail { | |
| sign: sign.clone(), | |
| let data = match self { | |
| Self::CrystalBallCloudy => serde_json::value::to_raw_value(&CloudyDetail { | |
| visibility: "opaque", | |
| suggestion: "try polishing the ball", | |
| }) | |
| .ok(), | |
| Self::StarsMisaligned { sign } => { | |
| serde_json::value::to_raw_value(&MisalignmentDetail { | |
| sign, |
| Self::CrystalBallCloudy => -32001, | ||
| Self::StarsMisaligned { .. } => -32002, | ||
| Self::MercuryRetrograde => -32003, | ||
| Self::UnreadableAura(_) => -32004, |
There was a problem hiding this comment.
My interpretation of the spec makes me think we shouldn't be using error codes in the range [-32099, -32000] for application errors like this, but only for errors specific to our JSON RPC implementation - i.e. for ajj to use itself.
| Fut: Future<Output = Result<Payload, ErrData>> + Send + 'static, | ||
| Payload: RpcSend, | ||
| ErrData: RpcSend, | ||
| ErrData: IntoErrorPayload, |
There was a problem hiding this comment.
Now that we have IntoErrorPayload with an associated type ErrData, it might be helpful to rename all these ErrData type params in this file to something like E or Error?
| /// Self::NotFound(_) => 404, | ||
| /// Self::RateLimited { .. } => 429, |
There was a problem hiding this comment.
nit: these look like http error codes, which could give readers the wrong impression that an http error would be returned in these cases.
| -32603 | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
We could maybe add an impl IntoErrorPayload for core::convert::Infallible?
| /// Only [`error_code`] is required. The remaining methods have sensible | ||
| /// defaults: |
There was a problem hiding this comment.
| /// Only [`error_code`] is required. The remaining methods have sensible | |
| /// defaults: | |
| /// Only [`error_code`] and the associated type `ErrData` must be specified. | |
| /// The remaining methods have sensible defaults: |
- Use application-level error codes instead of reserved JSON-RPC range - Fix doc example codes that looked like HTTP status codes - Add IntoErrorPayload impl for core::convert::Infallible - Clarify that ErrData associated type is required in trait docs - Refactor magic8ball match to take ownership, removing unnecessary clone Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
IntoErrorPayloadtrait that lets handler error types control JSON-RPC error code, message, and data fields when returningResult<T, E>InternalError<T>wrapper for placing a value in the errordatafield with code-32603ErrorPayload,String,&'static str,()IntoResponsePayloadtrait to unify the macro conversion pathFrom<Result<T, E>> for ResponsePayload,From<T> for ErrorPayload<T> where T: Error + RpcSend, andconvert_internal_error_msgmagic8ballexample showcasing custom error types with per-variant error codes and structured dataBreaking changes
Result<T, E>handlers now requireE: IntoErrorPayloadinstead ofE: RpcSendString/&strerrors now populate themessagefield (previously went indata)From<Result<T, E>> for ResponsePayload,From<T> for ErrorPayload<T> where T: Error + RpcSend, andResponsePayload::convert_internal_error_msgInternalError<E>for the old behavior, or implementIntoErrorPayloadfor custom error codesTest plan
IntoErrorPayloadimpls (ErrorPayload identity, InternalError, String, &str, (), custom error type)IntoResponsePayload(Result and ResponsePayload paths)--all-featuresand--no-default-featuresCloses ENG-2085
🤖 Generated with Claude Code