Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't allocate until we know it's worth it #420

Merged
merged 11 commits into from
Aug 16, 2021

Conversation

dvdplm
Copy link
Contributor

@dvdplm dvdplm commented Jul 15, 2021

For http server, check first byte before allocating space for the body.

Also re-work error handling and prefer sending JSON-RPC-style errors (JSON doc, spec'd error codes) combined with http status codes.

Alternative to #419

Closes #296

dvdplm added 6 commits July 15, 2021 17:42
…batch request

This works around the serde limitations around `untagged` enums and `RawValue`.
Also, rework the way we return errors: prefer JSON-RPC errors according to spec (application/json) wherever sensible.
/// Returns `Err` if the body was too large or the body couldn't be read.
pub async fn read_response_to_body(
pub async fn read_body(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The relevant changes are in this function.

return Ok::<_, HyperError>(response::too_large("The request was too large"))
}

let (body, mut is_single) = match read_body(&parts.headers, body, max_request_body_size).await {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the relevant change of the PR.

Copy link
Contributor

@maciejhirsz maciejhirsz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same stuff, marking as approved since those are easy fixes :)

utils/src/hyper_helpers.rs Outdated Show resolved Hide resolved
ws-server/src/server.rs Outdated Show resolved Hide resolved
http-server/src/tests.rs Outdated Show resolved Hide resolved
ws-server/src/tests.rs Outdated Show resolved Hide resolved
@dvdplm dvdplm requested a review from niklasad1 August 15, 2021 21:15
@niklasad1 niklasad1 merged commit 326d0c9 into master Aug 16, 2021
@niklasad1 niklasad1 deleted the dp-dont-allocate-until-we-know-its-worth-it branch August 16, 2021 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#[serde(untagged)] doesn't work with RawValue
3 participants