Skip to content

Model QUIC ConnectionClosed separately from ApplicationClose#336

Open
iadev09 wants to merge 1 commit into
hyperium:masterfrom
iadev09:feat/connection-closed-error
Open

Model QUIC ConnectionClosed separately from ApplicationClose#336
iadev09 wants to merge 1 commit into
hyperium:masterfrom
iadev09:feat/connection-closed-error

Conversation

@iadev09

@iadev09 iadev09 commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Summary

QUIC transport-level connection closures are currently mapped to ConnectionErrorIncoming::Undefined, which erases type information and makes downstream handling rely on debug strings or heuristics.

This change adds a dedicated ConnectionClosed { error_code } variant to ConnectionErrorIncoming and maps QUIC ConnectionClosed errors explicitly instead of treating them as undefined.

QUIC ConnectionClosed(NO_ERROR) represents graceful shutdown and should not be treated as an undefined error.

What changed

  • add ConnectionErrorIncoming::ConnectionClosed { error_code }
  • map quinn::ConnectionError::ConnectionClosed(...) explicitly in h3-quinn
  • keep ApplicationClose strictly HTTP/3-level
  • keep Undefined as the fallback for errors that still do not have a structured mapping
  • treat QUIC NO_ERROR as graceful shutdown in ConnectionError::is_h3_no_error()
  • pass the new variant through the datagram adapter as well

Why

ApplicationClose and QUIC ConnectionClosed carry different semantics and belong to different protocol layers.

ApplicationClose is an HTTP/3 application close.
ConnectionClosed is a QUIC transport close.

This PR keeps those separate instead of collapsing transport shutdown into Undefined.

This is also the connection-level equivalent of the fix in #315 for StreamError::is_h3_no_error(), which already handles the stream-level case via RemoteTerminate.

Compatibility

This is a public enum API change: ConnectionErrorIncoming gains a new ConnectionClosed variant.

That is not strictly backwards-compatible for exhaustive matches, but it avoids erasing structured QUIC transport shutdowns behind Undefined, and matches the maintainer guidance that this crate can tolerate such updates in practice.

Unlike an open-ended Other(...)-style enum, this does not silently remap behavior at runtime; downstream exhaustive matches get a compile-time signal to handle the new transport-level case explicitly.

Verification

  • cargo test -p h3-quinn connection_closed_is_mapped_to_structured_variant
  • cargo check -p h3 -p h3-quinn -p h3-datagram

Refs #325.

Preserve transport-level shutdown semantics by mapping QUIC ConnectionClosed explicitly instead of falling back to Undefined.

@Ruben2424 Ruben2424 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, thanks

@iadev09

iadev09 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Hi, gentle ping on this one.

This PR was approved a while ago, and we are currently carrying a [patch.crates-io] override for the whole h3 family (h3, h3-quinn, h3-datagram, and h3-webtransport) pinned to this PR commit. We need that only because this typed ConnectionClosed mapping has not landed upstream yet.

It works, but it keeps our dependency graph tied to a fork/rev pin, which is the part I’m a little concerned about long-term.

Has this been covered by a broader change elsewhere, or is there anything you’d like me to adjust here before it can move forward?

Thanks again.

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