Skip to content
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

UnhandledExceptionLoggingFilter should handle OperationCanceledException instead of TaskCanceledException #1377

Closed
danchristensen2000 opened this issue Aug 21, 2024 · 1 comment

Comments

@danchristensen2000
Copy link

Which version of Duende IdentityServer are you using?
7.0.6

Which version of .NET are you using?
.net 8

Describe the bug
OperationCanceledExceptions from EF core are logged as Critical errors.

To Reproduce
I don't know exactly how to reproduce this, but I can include the stack trace for our logs.

Expected behavior
I would expect the UnhandledExceptionLoggingFilter to prevent logging of an OperationCanceledException.

Log output/exception with stacktrace
System.OperationCanceledException: The operation was canceled.
at System.Threading.CancellationToken.ThrowOperationCanceledException()
at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteImplementationAsync[TState,TResult](Func4 operation, Func4 verifySucceeded, TState state, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func4 operation, Func4 verifySucceeded, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable1.AsyncEnumerator.MoveNextAsync() at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable1 source, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable1 source, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToArrayAsync[TSource](IQueryable1 source, CancellationToken cancellationToken)
at Duende.IdentityServer.EntityFramework.Stores.PersistedGrantStore.StoreAsync(PersistedGrant token) in //src/EntityFramework.Storage/Stores/PersistedGrantStore.cs:line 59
at Duende.IdentityServer.Stores.DefaultGrantStore1.StoreItemByHashedKeyAsync(String hashedKey, T item, String clientId, String subjectId, String sessionId, String description, DateTime created, Nullable1 expiration, Nullable1 consumedTime) in /_/src/IdentityServer/Stores/Default/DefaultGrantStore.cs:line 231 at Duende.IdentityServer.Stores.DefaultGrantStore1.CreateItemAsync(T item, String clientId, String subjectId, String sessionId, String description, DateTime created, Int32 lifetime) in /
/src/IdentityServer/Stores/Default/DefaultGrantStore.cs:line 177
at Duende.IdentityServer.ResponseHandling.AuthorizeResponseGenerator.CreateCodeFlowResponseAsync(ValidatedAuthorizeRequest request) in //src/IdentityServer/ResponseHandling/Default/AuthorizeResponseGenerator.cs:line 143
at Duende.IdentityServer.ResponseHandling.AuthorizeResponseGenerator.CreateResponseAsync(ValidatedAuthorizeRequest request) in /
/src/IdentityServer/ResponseHandling/Default/AuthorizeResponseGenerator.cs:line 100
at Duende.IdentityServer.Endpoints.AuthorizeEndpointBase.ProcessAuthorizeRequestAsync(NameValueCollection parameters, ClaimsPrincipal user, Boolean checkConsentResponse) in //src/IdentityServer/Endpoints/AuthorizeEndpointBase.cs:line 148
at Duende.IdentityServer.Endpoints.AuthorizeEndpointBase.ProcessAuthorizeRequestAsync(NameValueCollection parameters, ClaimsPrincipal user, Boolean checkConsentResponse) in /
/src/IdentityServer/Endpoints/AuthorizeEndpointBase.cs:line 160
at Duende.IdentityServer.Endpoints.AuthorizeEndpoint.ProcessAsync(HttpContext context) in //src/IdentityServer/Endpoints/AuthorizeEndpoint.cs:line 64
at Duende.IdentityServer.Hosting.IdentityServerMiddleware.Invoke(HttpContext context, IdentityServerOptions options, IEndpointRouter router, IUserSession userSession, IEventService events, IIssuerNameService issuerNameService, ISessionCoordinationService sessionCoordinationService) in /
/src/IdentityServer/Hosting/IdentityServerMiddleware.cs:line 106
at Duende.IdentityServer.Hosting.IdentityServerMiddleware.Invoke(HttpContext context, IdentityServerOptions options, IEndpointRouter router, IUserSession userSession, IEventService events, IIssuerNameService issuerNameService, ISessionCoordinationService sessionCoordinationService) in //src/IdentityServer/Hosting/IdentityServerMiddleware.cs:line 128
at Duende.IdentityServer.Hosting.MutualTlsEndpointMiddleware.Invoke(HttpContext context, IAuthenticationSchemeProvider schemes) in /
/src/IdentityServer/Hosting/MutualTlsEndpointMiddleware.cs:line 95
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
at Duende.IdentityServer.Hosting.DynamicProviders.DynamicSchemeAuthenticationMiddleware.Invoke(HttpContext context) in //src/IdentityServer/Hosting/DynamicProviders/DynamicSchemes/DynamicSchemeAuthenticationMiddleware.cs:line 51
at Duende.IdentityServer.Hosting.BaseUrlMiddleware.Invoke(HttpContext context) in /
/src/IdentityServer/Hosting/BaseUrlMiddleware.cs:line 27
at Microsoft.AspNetCore.ResponseCompression.ResponseCompressionMiddleware.InvokeCore(HttpContext context)
at Microsoft.AspNetCore.Diagnostics.StatusCodePagesMiddleware.Invoke(HttpContext context)
at Nbs.Framework.Startup.Middleware.HttpRequestLoggingPropertiesMiddleware.Invoke(HttpContext context)
at Nbs.Framework.Startup.Middleware.ResponseHandlerMiddleware.Invoke(HttpContext context)

Additional context
The default UnhandledExceptionLoggingFilter is handling TaskCanceledException, which is derived from OperationCancelledException. Please read Stephen Cleary's blog on this topic, where he recommends handling OperationCancelledException instead of TaskCanceledException because some APIs just raise OperationCanceledException, even if they deal with cancelled tasks.

https://blog.stephencleary.com/2022/03/cancellation-3-detecting-cancellation.html#:~:text=And%20since%20TaskCanceledException%20derives%20from,Catch%20OperationCanceledException%20instead.

@RolandGuijt
Copy link

RolandGuijt commented Sep 9, 2024

Thanks for reporting this. I've created an issue in the IdentityServer repository for this. We're going to investigate further and possibly make code changes.
Please track the new issue for updates. I'm closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants