Skip to content

feat(error): add h2_go_away_reason() to expose GOAWAY reason code#4052

Open
edef1c wants to merge 1 commit intohyperium:masterfrom
edef1c:push-okymqovvyvmu
Open

feat(error): add h2_go_away_reason() to expose GOAWAY reason code#4052
edef1c wants to merge 1 commit intohyperium:masterfrom
edef1c:push-okymqovvyvmu

Conversation

@edef1c
Copy link
Copy Markdown

@edef1c edef1c commented Apr 13, 2026

Add a public method on Error that lets callers distinguish h2 GOAWAY rejections from other h2 errors. Returns the GOAWAY reason as Option, avoiding leaking h2::Reason into hyper's public API.

This unblocks tonic and other middleware from implementing transparent retry of GOAWAY-rejected streams per RFC 9113 §6.8.

Add a public method on Error that lets callers distinguish h2 GOAWAY
rejections from other h2 errors. Returns the GOAWAY reason as Option<u32>,
avoiding leaking h2::Reason into hyper's public API.

This unblocks tonic and other middleware from implementing transparent
retry of GOAWAY-rejected streams per RFC 9113 §6.8.
@seanmonstar
Copy link
Copy Markdown
Member

Fair goal. I've been thinking about how reqwest currently peeks at the source h2::Error, but it really shouldn't, considering the explicit instability warning in hyper docs. (And will break when we merge in h2 v0.5).

I've wondered, one option could be to just add an is_protocol_nack() -> bool, which can handle GOAWAY and RST_STREAMs (like a REFUSED_STREAM), and could also work for h3 once added.

Or another option is to finally get around to designing #2845. I'd very much like that, but it's a bigger piece of conceptual design work.

@edef1c
Copy link
Copy Markdown
Author

edef1c commented Apr 14, 2026

I'm specifically exposing the error code so tonic can retry in NO_ERROR case (and indeed on RST_STREAM with REFUSED_STREAM), without blindly retrying in the other cases. Would we want is_protocol_nack to expose just that one? I think ENHANCE_YOUR_CALM might be useful to surface as well. That could be handled by exposing the already-existing h2_reason method.

A complication is that hyperium/tonic@master...edef1c:tonic:push-rosuyzxnysvw also depends on being able to recognise DispatchGone, but that should presumably count as a NACK as well, since we know the request hasn't been dispatched. h2_reason would presumably return InternalError, though.

@seanmonstar
Copy link
Copy Markdown
Member

DispatchGone isn't necessarily a NACK, since it doesn't include information of what step the request was at. It means that the runtime that spawned the dispatch task disappeared, who knows in what state.

I wouldn't expect NO_ERROR to be a NACK. Usually that's used for a server to say "yea yea that request was fine, here's a response, you can stop sending the rest of the request body". Or I suppose in gRPC terms where the messages are not the normal request/response exchange, but the body, maybe it does mean you might want to retry. But it likely wouldn't be a GOAWAY, but a RST_STREAM.

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