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

[QUIC] Fixed exception type for ConnectionShutdownInitiatedByTransport #60181

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

ManickaP
Copy link
Member

@ManickaP ManickaP commented Oct 8, 2021

In case of a shutdown by a transport we would throw QuicOperationAbortedException out of AcceptStreamAsync, which would wrongly imply that it was our side who initiated the connection shutdown.
This is a quick safe fix to make the logic throw QuicConnectionAbortedException instead.

However, we should properly propagate outside the information that it was transport and not the peer who closed the connection --> not closing the original issue.

Fixes #60133

Verified manually, now seeing in Kestrel log:

dbug: Microsoft.AspNetCore.Server.Kestrel.Transport.Quic[5]
      Connection id "0HMCADU0GKLVK" aborted by peer with error code 0.
      System.Net.Quic.QuicConnectionAbortedException: Connection aborted by peer (0).
         at System.Net.Quic.Implementations.MsQuic.MsQuicConnection.AcceptStreamAsync(CancellationToken cancellationToken)
         at System.Net.Quic.QuicConnection.AcceptStreamAsync(CancellationToken cancellationToken)
         at Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.Internal.QuicConnectionContext.AcceptAsync(CancellationToken cancellationToken)
dbug: Microsoft.AspNetCore.Server.Kestrel.Transport.Quic[6]
      Connection id "0HMCADU0GKLVK" aborted by application with error code 0 because: "The client closed the connection.".

cc @JamesNK @CarnaViire

@sebastienros
Copy link
Member

After testing, I confirm I can't repro the issue anymore (from an E2E perspective).

Copy link
Member

@CarnaViire CarnaViire left a comment

Choose a reason for hiding this comment

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

Thanks!

@ManickaP ManickaP merged commit 565ff52 into dotnet:main Oct 11, 2021
@ManickaP ManickaP deleted the mapichov/60133_exception_fix branch October 11, 2021 09:01
@ManickaP
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1328653526

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QuicConnection.AcceptStreamAsync throws incorrect exception on browser close
4 participants