-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(response): introduce trailers support #2788
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
base: master
Are you sure you want to change the base?
Conversation
|
After seeing your implementation, I just realized my implementation#2782 which mimics the fn chunk may consumes the res body and lead to inaccessibility of the body after the fn trailers called. Though, I think the user cases are mostly getting trailers after getting the body. Did not expect to be this complicate for this issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @0x676e67 - I am also interested in trailers support and find this PR. It looks pretty good to me, requiring the body to be consumed to have access to trailers is sensible and is how Go's stdlib http client works too.
I left one suggestion but does it seem worth giving the PR a go?
| /// be determined after processing the entire response body. | ||
| #[inline] | ||
| pub async fn trailers(&mut self) -> crate::Result<Option<HeaderMap>> { | ||
| match self.trailers_rx.try_recv() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this channel isn't actually used for synchronization, does it make sense to use a simpler OnceLock instead? Then trailers can be returned as a reference to match headers and doesn't need to be async.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to open a PR, I might not have the time to implement it myself.
from: #2626
from: #2776
Just a draft prototype for now. It’s a bit ugly, and there’s probably a better approach