-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Conversation
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."); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
Thanks |
Fixes #8696