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

add the next multipart delimiter at the end of the chunk #1596

Merged
merged 3 commits into from
Aug 25, 2022

Conversation

Geal
Copy link
Contributor

@Geal Geal commented Aug 24, 2022

Fix #1588

deferred responses come as multipart elements, send as individual HTTP
response chunks. When a client receives one chunk, it should contain the
next delimiter, so the client knows that the response can be processed,
instead of waiting for the next chunk to see the delimiter.

Side note: fixed the format of the last delimiter that was missing a
trailing --

deferred responses come as multipart elements, send as individual HTTP
response chunks. When a client receives one chunk, it should contain the
next delimiter, so the client knows that the response can be processed,
instead of waiting for the next chunk to see the delimiter.

Side note: fixed the format of the last delimiter that was missing a
trailing --
@Geal Geal requested a review from BrynCooke August 24, 2022 08:22
@github-actions

This comment has been minimized.

@Geal Geal requested a review from bnjjj August 24, 2022 08:22
@Geal Geal self-assigned this Aug 24, 2022
Comment on lines +536 to 561
// each chunk contains a response and the next delimiter, to let client parsers
// know that they can process the response right away
let mut first_buf = Vec::from(
&b"\r\n--graphql\r\ncontent-type: application/json\r\n\r\n"[..],
);
serde_json::to_writer(&mut first_buf, &response).unwrap();
first_buf.extend_from_slice(b"\r\n--graphql\r\n");

let body = once(ready(Ok(Bytes::from(first_buf)))).chain(
stream.map(|res| {
let mut buf = Vec::from(
&b"content-type: application/json\r\n\r\n"[..],
);
serde_json::to_writer(&mut buf, &res).unwrap();

// the last chunk has a different end delimiter
if res.has_next.unwrap_or(false) {
buf.extend_from_slice(b"\r\n--graphql\r\n");
} else {
buf.extend_from_slice(b"\r\n--graphql--\r\n");
}

Ok::<_, BoxError>(buf.into())
}),
);

Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: might be more readable in a function

@Geal Geal enabled auto-merge (squash) August 25, 2022 08:39
@Geal Geal merged commit 670601a into main Aug 25, 2022
@Geal Geal deleted the geal/multipart-delimiters branch August 25, 2022 09:04
@abernix abernix mentioned this pull request Aug 29, 2022
@abernix abernix added this to the v1.0.0-alpha.0 milestone Aug 29, 2022
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.

Defer: send the multipart boundary at the end of each part instead of at the beginning?
4 participants