Skip to content

check final response in poll_informational#889

Merged
seanmonstar merged 2 commits intohyperium:masterfrom
zh-jq:poll-informational
Apr 14, 2026
Merged

check final response in poll_informational#889
seanmonstar merged 2 commits intohyperium:masterfrom
zh-jq:poll-informational

Conversation

@zh-jq
Copy link
Copy Markdown
Contributor

@zh-jq zh-jq commented Apr 7, 2026

Make sure poll_informational returns Poll::Ready(None) after the final response received. So the caller can then use the ResponseFuture::poll() method to get the final response.

The lock in OpaqueStreamRef get released when poll_informational returned, so if it didn't returns Poll::Ready(None) on received final response header, the caller has to call ResponseFuture::poll() every time to check the final response, this may get some informational response get ignored.

@zh-jq zh-jq force-pushed the poll-informational branch from a5c966d to 3ace95f Compare April 14, 2026 00:02
@seanmonstar
Copy link
Copy Markdown
Member

Thanks for the PR!

I looked at the code already in the function, and noticed some things. Since it matches other, it will get pushed back into the queue. Are you saying that stream.state.ensure_recv_open() will be true, even though the informational events are complete?

@zh-jq
Copy link
Copy Markdown
Contributor Author

zh-jq commented Apr 14, 2026

This is the code I used:

    fn poll_recv(&mut self, cx: &mut Context<'_>) -> Poll<Result<Response<()>, Error>> {
        if let Some(r) = ready!(self.rsp_fut.poll_informational(cx)) {
            return Poll::Ready(r);
        }

        let r = ready!(Pin::new(&mut self.rsp_fut).poll(cx))?;
        // ...
    }

I have to know there won't be any informational headers so I can poll for the final response. I can't call poll directly when poll_informational return Poll::Pending as poll will drop informational headers silently.

@seanmonstar
Copy link
Copy Markdown
Member

Hm, ok. I wonder, would it possible to add a test to https://github.com/hyperium/h2/blob/master/tests/h2-tests/tests/informational_responses.rs that fails without this change? 🤓

@zh-jq
Copy link
Copy Markdown
Contributor Author

zh-jq commented Apr 14, 2026

Hm, ok. I wonder, would it possible to add a test to https://github.com/hyperium/h2/blob/master/tests/h2-tests/tests/informational_responses.rs that fails without this change? 🤓

Added one here: #890

Copy link
Copy Markdown
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Got it, thanks for the test!

@seanmonstar seanmonstar merged commit dbc204e into hyperium:master Apr 14, 2026
6 checks passed
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.

2 participants