feat!: add consuming into_raw_value to RpcSend#37
Merged
Conversation
…` supertrait `RpcSend` previously required `Serialize` as a supertrait, forcing lazy producers like iterators to use `Mutex<Option<T>>` wrappers for interior mutability during serialization. This replaces the marker trait with a consuming `into_raw_value(self)` method, enabling direct ownership transfer during serialization. The blanket impl for `T: Serialize` is preserved, so all existing code that returns `Serialize` types from handlers continues to work. Types that do not implement `Serialize` can now implement `RpcSend` directly with custom serialization logic. BREAKING CHANGE: `RpcSend` no longer has `Serialize` as a supertrait. Code that used `T: RpcSend` and relied on the `Serialize` bound will need to add an explicit `Serialize` bound. `HandlerCtx::notify` now takes `T` by value instead of `&T` (callers passing `&item` still work via the blanket impl). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
prestwich
commented
Feb 14, 2026
prestwich
commented
Feb 14, 2026
| return Some(Self::serialization_failure(id)); | ||
| } | ||
| }; | ||
| let resp = Response { id, payload: &raw }; |
Member
Author
There was a problem hiding this comment.
isn't this one extra serialization round?
Member
Author
There was a problem hiding this comment.
[Claude Code]
Yes — the payload was serialized to Box<RawValue> via into_raw(), then the Response struct wrapping it was serialized again via to_raw_value. While Box<RawValue>'s Serialize impl just copies bytes (no re-parsing), it's still an unnecessary serde machinery invocation.
Fixed by building the JSON envelope directly via format! with id.get() and result.get() (or serde_json::to_string(&error) for the error case), bypassing the Response struct entirely for build_response.
- Deprecate `ErrorPayload::serialize_payload` in favor of `into_raw` - Eliminate extra serialization round in `build_response` by constructing the JSON envelope directly via format!, bypassing the Response struct Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
prestwich
commented
Feb 14, 2026
| } | ||
| Err(error) => match serde_json::to_string(&error) { | ||
| Ok(error_json) => { | ||
| format!( |
Member
Author
There was a problem hiding this comment.
no hand crafting json. use types.
Member
Author
There was a problem hiding this comment.
[Claude Code]
Fixed — build_response now uses Response { id, payload: &raw }.to_json() (types, not format strings).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Response now owns the payload via Cell<Option<...>> so the Serialize impl can consume it. Bounds are T: RpcSend, E: RpcSend instead of T: Serialize, E: Serialize. build_response no longer needs to call into_raw explicitly — it's handled inside the Serialize impl. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
prestwich
commented
Feb 14, 2026
Comment on lines
+43
to
+44
| pub(crate) fn serialization_failure(id: &RawValue) -> Box<RawValue> { | ||
| RawValue::from_string(format!( | ||
| r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32700,"message":"response serialization error"}}}}"#, | ||
| id.get() | ||
| )) | ||
| .expect("valid json") | ||
| let resp = Response::new( |
Member
Author
There was a problem hiding this comment.
this reserializes on each invocation. memoize in a lazy and clone on demand
Member
Author
There was a problem hiding this comment.
[Claude Code]
Addressed both comments:
- "no hand crafting json. use types":
build_responsenow usesResponse { id, payload: &raw }.to_json(). - "memoize in a lazy and clone on demand":
parse_error()now clones from aLazyLock<Box<RawValue>>(serialized once).serialization_failure()borrows astatic ResponsePayloadfor the error payload — only the envelope serialization (with the dynamic id) happens per call.
This reverts commit 1383288.
- parse_error() now clones from a LazyLock<Box<RawValue>> instead of serializing on every call - serialization_failure() borrows a static ResponsePayload instead of reconstructing it per invocation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RpcSendmarker trait with a consuminginto_raw_value(self)method, removingSerializeas a supertraitMutex<Option<T>>wrappers — types are consumed directly during serializationRpcSendimpls (pre-serialized data, non-serde sources) by allowing types to bypassSerializeentirelySerializetypes continue to work via the blanket impl — no downstream breakage for typical usageSerializesupertrait removed,notifysignature changed to takeTby value)Test plan
cargo clippy -p ajj --all-features --all-targets— cleancargo clippy -p ajj --no-default-features --all-targets— cleancargo +nightly fmt --check— cleancargo t -p ajj— all 18 doc-tests + 15 unit tests + 3 integration tests passctx.notify(&item)callers verified to work via&T: RpcSendblanket impl🤖 Generated with Claude Code