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: SHUTDOWN_INITIATED_BY_TRANSPORT on connection should not result in QuicOperationAbortedException #55157

Closed
geoffkizer opened this issue Jul 4, 2021 · 8 comments
Assignees
Milestone

Comments

@geoffkizer
Copy link
Contributor

SHUTDOWN_INITIATED_BY_TRANSPORT can indicate a variety of different errors, including stateless reset or a QUIC protocol error. It's determined locally by MsQuic (hence the name).

Currently, if we receive SHUTDOWN_INITIATED_BY_TRANSPORT from MsQuic, we generate a QuicOperationAbortedException for any attempted operations on the connection (e.g. AcceptStreamAsync).

This seems like the wrong exception. QuicOperationAbortedException is intended to mean that the local user closed the connection, and thus the outstanding operation was aborted as a result.

We could use QuicConnectionAbortedException here, except that this requires an error code, and is intended for cases where the peer explicitly closed the connection, so it doesn't seem ideal.

We probably need a new exception type for this. And we may want to distinguish different error cases within this.

@geoffkizer geoffkizer added this to the 6.0.0 milestone Jul 4, 2021
@ghost
Copy link

ghost commented Jul 4, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

SHUTDOWN_INITIATED_BY_TRANSPORT can indicate a variety of different errors, including stateless reset or a QUIC protocol error. It's determined locally by MsQuic (hence the name).

Currently, if we receive SHUTDOWN_INITIATED_BY_TRANSPORT from MsQuic, we generate a QuicOperationAbortedException for any attempted operations on the connection (e.g. AcceptStreamAsync).

This seems like the wrong exception. QuicOperationAbortedException is intended to mean that the local user closed the connection, and thus the outstanding operation was aborted as a result.

We could use QuicConnectionAbortedException here, except that this requires an error code, and is intended for cases where the peer explicitly closed the connection, so it doesn't seem ideal.

We probably need a new exception type for this. And we may want to distinguish different error cases within this.

Author: geoffkizer
Assignees: -
Labels:

area-System.Net.Quic

Milestone: 6.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jul 4, 2021
@wfurt
Copy link
Member

wfurt commented Jul 5, 2021

can we reliably detect the reason?
cc: @nibanks

@nibanks
Copy link

nibanks commented Jul 5, 2021

can we reliably detect the reason?
cc: @nibanks

The event does give you a status code. It might not always clearly differentiate all possible reasons, but if you ever need more, we can add more specific status codes.

@ManickaP ManickaP removed the untriaged New issue has not been triaged by the area owner label Jul 6, 2021
@karelz
Copy link
Member

karelz commented Jul 15, 2021

Triage: QUIC does not have direct users. Unless ASP.NET needs this, it is not critical for 6.0. Moving to 7.0 for now.

@karelz karelz modified the milestones: 6.0.0, 7.0.0 Jul 15, 2021
@karelz
Copy link
Member

karelz commented Oct 26, 2021

Triage: This should be fixed - @ManickaP can you please verify (for extra points :))

@ManickaP
Copy link
Member

This has been partially fixed for 6.0 (and main as well) in #60181, i.e. it's a hack solution to cause a minimal churn.
We still need to decide the proper way to surface the difference between peer application close and transport close. Either by a different exception or a different property on an existing one.
The 7.0 milestone stands, this should be solved together with the general "exceptions" discussion.

@karelz
Copy link
Member

karelz commented Jun 14, 2022

Triage: @rzikm drives exception design, should be part of that.

@karelz
Copy link
Member

karelz commented Jun 28, 2022

Duplicate of #70669

@karelz karelz marked this as a duplicate of #70669 Jun 28, 2022
@karelz karelz closed this as completed Jun 28, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants