-
Notifications
You must be signed in to change notification settings - Fork 40
Conversation
} | ||
|
||
// Execute last to first. This mimics a stack unwind. | ||
foreach (var actionPair in actions.Reverse()) |
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.
Catch and suppress any Exceptions, otherwise the rest of Dispose won't run and you'll have a resource leak.
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.
Updated.
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.
Should we log any exceptions that occur?
9a33463
to
255d5c2
Compare
@@ -147,4 +147,7 @@ | |||
<data name="Exception_WrongIAsyncResult" xml:space="preserve"> | |||
<value>The given IAsyncResult does not match this opperation.</value> | |||
</data> | |||
<data name="Warning_ExceptionInOnResponseCompletedAction" xml:space="preserve"> | |||
<value>Exception occured in a registered response completed action.</value> |
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.
Recommended slight rewording (and fixing typo 😄): An exception occurred while disposing a component registered with {0}.
and then pass in nameof(OnResponseCompleted) for the {0}
.
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.
It doesn't necessarily have to be dispose. Any action can be registered to be performed in OnResponseCompleted
. So not sure if we should say "while disposing".
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.
Oh good point! How about: An exception occurred while running an action registered with {0}.
255d5c2
to
c731872
Compare
c30fc2b
to
39b8d20
Compare
Related to aspnet/HttpAbstractions#196
@lodejard @GrabYourPitchforks @Tratcher