Skip to content

Conversation

@geoffkizer
Copy link
Contributor

We are currently throwing OperationCancelledException when the peer aborts our write. We should be throwing QuicStreamAbortedException with the appropriate error code. Also fix mock provider to handle write abort and add test.

Also, #54798 changed CanRead/CanWrite behavior to return false when completed or aborted, which broke some tests that rely on CanRead/CanWrite to distinguish between unidirectional and bidirectional streams. Revert the behavior of CanRead/CanWrite to previous behavior -- that is, only change these to false when the object is disposed.

@ghost ghost added the area-System.Net label Jul 8, 2021
@ghost
Copy link

ghost commented Jul 8, 2021

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

Issue Details

We are currently throwing OperationCancelledException when the peer aborts our write. We should be throwing QuicStreamAbortedException with the appropriate error code. Also fix mock provider to handle write abort and add test.

Also, #54798 changed CanRead/CanWrite behavior to return false when completed or aborted, which broke some tests that rely on CanRead/CanWrite to distinguish between unidirectional and bidirectional streams. Revert the behavior of CanRead/CanWrite to previous behavior -- that is, only change these to false when the object is disposed.

Author: geoffkizer
Assignees: -
Labels:

area-System.Net

Milestone: -

@geoffkizer geoffkizer requested review from CarnaViire, ManickaP and wfurt and removed request for wfurt July 8, 2021 16:06
@ghost
Copy link

ghost commented Jul 8, 2021

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

Issue Details

We are currently throwing OperationCancelledException when the peer aborts our write. We should be throwing QuicStreamAbortedException with the appropriate error code. Also fix mock provider to handle write abort and add test.

Also, #54798 changed CanRead/CanWrite behavior to return false when completed or aborted, which broke some tests that rely on CanRead/CanWrite to distinguish between unidirectional and bidirectional streams. Revert the behavior of CanRead/CanWrite to previous behavior -- that is, only change these to false when the object is disposed.

Author: geoffkizer
Assignees: -
Labels:

area-System.Net.Quic

Milestone: -

@geoffkizer geoffkizer added this to the 6.0.0 milestone Jul 8, 2021
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

{
await using QuicStream stream = await connection.AcceptStreamAsync();

QuicStreamAbortedException ex = await Assert.ThrowsAsync<QuicStreamAbortedException>(() => WriteForever(stream));
Copy link
Member

Choose a reason for hiding this comment

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

We discussed previously that it's good to add .WaitAsync(someReasonableTimeout) to WriteForever-like operations, it would still fail the assertion, but we won't have a hanging test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RunClientServer has a timeout, so the test won't hang forever.

@wfurt wfurt merged commit d9f1ade into dotnet:main Jul 8, 2021
@geoffkizer geoffkizer deleted the canwrite branch July 8, 2021 21:08
@ghost ghost locked as resolved and limited conversation to collaborators Aug 7, 2021
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.

3 participants