Skip to content

Conversation

@taooceros
Copy link
Member

fix #357

@taooceros
Copy link
Member Author

taooceros commented Feb 23, 2021

I originally think that taskcancelledexception may be thrown by task cancellation token, but in reality it throws an operation canceledexception. Therefore, we should also change some place where using TaskCancelledException for handling. We do use OperationCanceledException else where due to compatibility except websearch plugin.

@jjw24
Copy link
Member

jjw24 commented Feb 23, 2021

except websearch plugin.

Do we need to update websearch too?

We do use OperationCanceledException else where...

Sounds strange, how are we going to differentiate task cancelled and actual error from operation cancelled?

@taooceros
Copy link
Member Author

taooceros commented Feb 23, 2021

I just read some new change toward HttpClient, and they seems not wrap a Timeout exception within the TaskCanceledException.
Check out the discussion here dotnet/runtime#21965
and the pr here dotnet/runtime#2281
I don't know why the error trace from @solymr is actually OperationCanceledException. That's weird, because it supposed to be a taskcenceledexception when timeout.

Our original OperationCanceledException is for one excpetion handling for both taskcanceledexception and parallel exercution canceled exception.

@taooceros
Copy link
Member Author

Based on the discussion there and the pr, we should distinguish this by checking whether innerexception is timeoutexception.

@taooceros
Copy link
Member Author

emmm seems that that pr is for .Net 5, so we can only do that after upgrading to .net 5

@taooceros taooceros modified the milestones: 1.8.0, 1.7.3 Feb 23, 2021
jjw24
jjw24 previously approved these changes Feb 25, 2021
Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

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

if we get more users reporting this issue, then we can do a hotfix

@jjw24 jjw24 enabled auto-merge February 25, 2021 03:00
@jjw24 jjw24 merged commit 890f114 into dev Feb 25, 2021
@jjw24 jjw24 deleted the FixUpdateCancelledException branch February 25, 2021 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Application crash from time to time

3 participants