-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Add missing ConfigureAwait(false) to PolicyHttpMessageHandler #39874
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
Add missing ConfigureAwait(false) to PolicyHttpMessageHandler #39874
Conversation
Add missing ConfigureAwait(false) on task await. Add EditorConfig file to catch future occurrences.
[*.{cs,vb}] | ||
|
||
# CA2007: Consider calling ConfigureAwait on the awaited task | ||
dotnet_diagnostic.CA2007.severity = warning |
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.
Could we leave this file out? I'd like to see if we can enable this rule at the global level and make it a non-warning for projects that are part of ASP.NET Core / reference the shared fx?
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 can take it out if you like - I'd added it based on #39865 (comment)
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.
Yes please. I might give it a go this week. FYI @BrennanConroy
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.
if we can enable this rule at the global level and make it a non-warning for projects that are part of ASP.NET Core / reference the shared fx
Sounds good, didn't know you were planning on doing this 👍
Remove based on PR feedback.
Thanks |
@martincostello is there any chance this fix can be backported to 6.0? My team just hit a deadlock using Polly in our WinForms app because of this missing |
Hi @wjrogers. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
@wjrogers That's a question for the ASP.NET Core team, not me 😄 |
Hi @martincostello. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
Add missing ConfigureAwait(false)
Add missing
ConfigureAwait(false)
on task await.Description
Add missing
ConfigureAwait(false)
on task await inPolicyHttpMessageHandler
.Looks like this got overlooked in #36162 when
async
was added to the method.