Check how EF7 handles task cancellations #1040
Labels
Milestone
Comments
|
Yea, now with EF7 we can look to see what they have actually changed. I thought they promised that this would no longer be a thrown exception. I was hoping it'd just stop being an exception and they would just log instead. But maybe that's not what they ended up doing (which would be unfortunate). |
brockallen
changed the title
|
.NET 6 .NET 7 |
So then for @Stra1ghter -- we are still getting an unhandled exception in the So... then I'm not sure what's better? I guess it's still one less log entry. Thoughts? |
|
@brockallen the remaining logs stem from SqlClient, right? Reading roji's answer in the issue, I thought that this is fixed with SqlClient as well, but according to #26988 this is not the case. In case your middleware actions need to accept
|
I believe yes, this line:
Yep, 6.2 is our release for targeting .NET 7.
Where? I just tried and the DI system is not happy. I was thinking we could just look at the HttpContext's RequestAborted.
Why? Why not simply suppress our log entry if the exception is a TaskCanceledException and RequestAborted.IsCancellationRequested is true? So in other words -- we still have 2 log entries to deal with. One from MS we can't do anything about, but the one from IS we can. Currently our middleware does this: But I was thinking something like this: |
Well, correct me if I'm wrong (I'm not a full time ASP.NET Core guy), but Example: you use your own If you would not provide a
The more I think about it, the more my feelings say that you should do this. You use the tokens and don't do anything with the exception. You could in fact also catch |
|
@Stra1ghter PR has been submitted to allow flexibility here. Please have a look. |
|
Hello, Sorry to comment on this closed issue. I have went down the rabbit hole of github issues related to this in the efcore repo. Judging from the response I read from @Stra1ghter there is no way for us to avoid one of the TaskCanceledExceptions coming out of the SqlClient? We just updated to 6.2.1 and I can see the exception handler you added in the LoggingOptions allows us to remove one of both as stated. We tried migrating to .NET 7 (Including our deps to EF Core 7) as I thought it would partially resolve but it doesn't seem like it? |
There is bunch of opportunities where the CancellationToken can cancel already issued query -> When opening connection, waiting for connection pool or even while reading data - just to name few. In general, you should not log expected exception as critical - When you cancel a request using cancellation token, then some exception is expected. |
|
Hello,
is possible to ignore the exception or is the error due to some other problem? |
You can wrap our middleware in your own try/catch and control that any way you want. |
|
Hi, maybe my question wasn't very clear, since the task cancellation is not requested, this type of exception is still a false positive and therefore I don't have to worry or is it a problem with my application/database and I have to investigate what? Thank you |
|
Oh I see -- so you're getting this exception but the request was not cancelled by the caller? In that case, I'm not sure -- do you have more details of the exception? |
|
Here is the entire exception log: |
|
Well... all that shows is that the cancellation token that was passed into the EF layer was triggered. Unless you did anything special, then it's ASP.NET doing it due to some reason. We did make the filter (UnhandledExceptionLoggingFilter) we use extensible so you can look closer at the exception and decide if you want to ignore it or not: https://docs.duendesoftware.com/identityserver/v6/reference/options/#logging |
|
Thanks, I'll try to investigate and understand the problem. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Which version of Duende IdentityServer are you using?
6.1.5
Which version of .NET are you using?
net6.0
Describe the bug
When a user cancels a browser request that is handled by
IdentityServerMiddlewaretheTaskCanceledExceptionis logged as a critical log. We see the errors in different stack traces, but it is always a task that finally queries some store via EF Core.To Reproduce
I couldn't absolutely reproduce the behaviour, but it seems to be known.
This usually always happens when you accept a
CancellationTokenin your actions, e.g. for long running operations, but don't handle the cancellation of the task.Please note that it's now possible in EF Core to distinguish between cancellation and failure.
Expected behavior
Handle user caused task cancellations (e.g. clicking stop or refresh in the browser) by a custom filter, that could log this as information or debug.
Another solution would be to simply stop accepting and passing
CancellationTokenon IdentityServer's actions. I don't think IdentityServer has long running tasks that would benefit from knowing about the cancellation of a task, but I might be wrong here.Log output/exception with stacktrace
Stack trace is trimmed due to the fact that it exceeds maximum number of characters.
The text was updated successfully, but these errors were encountered: