Conversation
…hitespace and newlines Right now we're technically off spec from JSON-RPC, as we don't trim whitespace or newlines at all from any incoming requests, but we should. This leads to problems like making it more cumbersome to use `curl` to create raw requests to routers that use ajj. `serde_json` handles this just fine. Our issue is when handling the conversion to a partially desered json rpc request. Right now this needs an extra allocation since `bytes`'s `.trim_ascii()` [only trims whitespaces according to its definition, but does not trim newlines at all](https://doc.rust-lang.org/nightly/std/primitive.u8.html#method.is_ascii_whitespace). This kinda sucks, and can be improved as this has an overhead (this would be interesting to benchmark. Closes ENG-996
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
prestwich
reviewed
Apr 16, 2025
src/types/batch.rs
Outdated
| let rv: &RawValue = serde_json::from_slice(bytes.as_ref())?; | ||
|
|
||
| // First, check if it's a single request | ||
| let rv: &RawValue = serde_json::from_slice(bytes.as_ref())?; |
Member
There was a problem hiding this comment.
this does mean batch requests are doing 2 linear passes, once for this from_slice and once for the from_str
we should be able to do that all in one pass
what if we did something like
enum DeserHelper<'a> {
Batch(Vec<&'a RawValue>),
Single(&'a RawValue)
}
Member
There was a problem hiding this comment.
so this doesn't work because of serde-rs/serde#1183 but that's fine, we can use the deserializer directly
rswanson
approved these changes
Apr 17, 2025
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
3 participants
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.
Right now we're technically off spec from JSON, as we don't trim whitespace or newlines at all from any incoming requests, but we should, as json requires you to ignore them. This leads to problems like making it more cumbersome to use
curlto create raw requests to routers that use ajj.serde_jsonhandles this just fine. The fix is roughly to useserde_jsonto parse the bytes and check through it'sRawValueto see if it's a batch or not.