Skip to content

Suppress log when request is cancelled #16691

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

Merged
merged 3 commits into from
Nov 20, 2019
Merged

Conversation

Kahbazi
Copy link
Member

@Kahbazi Kahbazi commented Oct 31, 2019

Fixes #8696

if (!RequestContext.DisconnectToken.IsCancellationRequested)
{
// This is logged rather than thrown because it is too late for an exception to be visible in user code.
LogHelper.LogError(Logger, "ResponseStream::Dispose", "Fewer bytes were written than were specified in the Content-Length.");
Copy link
Contributor

Choose a reason for hiding this comment

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

@Kahbazi can you write a unit test to verify this behavior? Or is there a race and it makes it hard to write a test.

Copy link
Member

Choose a reason for hiding this comment

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

I plan to try some manual verification before merging this. I don't know if DisconnectToken.IsCancellationRequested is consistent enough in these scenarios for this to work as expected.

@analogrelay analogrelay added this to the 5.0.0-preview1 milestone Nov 20, 2019
Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

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

This works at least as well as DisconnectToken. There's room for future improvement in DisconnectToken where requesting it for the first time after the disconnect returns an uncancellable token rather than a cancelled one.

@Tratcher Tratcher merged commit 642fe16 into dotnet:master Nov 20, 2019
@Tratcher
Copy link
Member

Thanks

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong error logging when a client cancels the request
6 participants