fix: implement ASP.NET Core 10 error handling best practices by Copilot · Pull Request #685 · grandnode/grandnode2

Copilot · 2026-04-29T19:01:45Z

- Add GrandExceptionHandler implementing IExceptionHandler (ASP.NET Core 8+ pattern): logs exceptions, returns RFC 7807 ProblemDetails JSON for API (Bearer) requests, returns false for web requests so the configured error page handles the response. Guards with HasStarted before writing. - Register AddExceptionHandler<GrandExceptionHandler>() and AddProblemDetails() in ErrorHandlerStartup.ConfigureServices. - Fix UseGrandExceptionHandler: remove dual UseExceptionHandler registration bug. Single UseExceptionHandler('/errorpage.htm') now invokes the IExceptionHandler chain first, then falls back to re-executing at /errorpage.htm for non-API requests. - Fix UsePageNotFound: replace 302 Redirect with UseStatusCodePagesWithReExecute ('/page-not-found') which preserves the original 404 status code. Add inline middleware to disable status code pages for API (Bearer) and static-resource requests so those callers receive their original response unchanged. - Fix ApiAuthenticationRegistrar: replace catch(Exception ex) { throw new Exception(ex.Message) } with bare throw to preserve the original stack trace; update OnAuthenticationFailed to return application/problem+json via IProblemDetailsService instead of plain text. - Fix BaseController.LogException: use a constant message template instead of exception.Message to avoid structured-logging format issues. Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/ecc051c4-72f1-4d90-8ec9-025f19ab46c3 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>

… Split()[0])" Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/ecc051c4-72f1-4d90-8ec9-025f19ab46c3 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>

…der check Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/1ec7db4a-11fc-49b4-8aa2-85f12990ec16 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>

Copilot · 2026-04-30T18:27:04Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

ccsca2021.crl.certum.pl
- Triggering command: /usr/share/dotnet/dotnet dotnet build src/Web/Grand.Web.Common/Grand.Web.Common.csproj --no-incremental -v q (dns block)
ccsca2021.ocsp-certum.com
- Triggering command: /usr/share/dotnet/dotnet dotnet build src/Web/Grand.Web.Common/Grand.Web.Common.csproj --no-incremental -v q (dns block)
crl.certum.pl
- Triggering command: /usr/share/dotnet/dotnet dotnet build src/Web/Grand.Web.Common/Grand.Web.Common.csproj --no-incremental -v q (dns block)
subca.ocsp-certum.com
- Triggering command: /usr/share/dotnet/dotnet dotnet build src/Web/Grand.Web.Common/Grand.Web.Common.csproj --no-incremental -v q (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Copilot

Pull request overview

Updates GrandNode’s ASP.NET Core error-handling pipeline to use the IExceptionHandler + ProblemDetails pattern, aiming for consistent API error responses while preserving existing HTML error pages for browser requests.

Changes:

Registers AddProblemDetails() and AddExceptionHandler<GrandExceptionHandler>() and introduces GrandExceptionHandler to produce RFC7807 responses for API requests.
Refactors API-request detection into ApplicationBuilderExtensions.IsApiRequest(HttpRequest) and reworks 404 handling to use UseStatusCodePagesWithReExecute.
Updates API JWT auth failure handling to return ProblemDetails and preserves stack traces on token-validation failures.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Web/Grand.Web.Common/Startup/ErrorHandlerStartup.cs	Registers ProblemDetails + exception handler services for the new error pipeline.
src/Web/Grand.Web.Common/Infrastructure/GrandExceptionHandler.cs	Adds `IExceptionHandler` implementation to log and emit RFC7807 for API errors.
src/Web/Grand.Web.Common/Infrastructure/ApplicationBuilderExtensions.cs	Adds `IsApiRequest` helper, removes duplicate exception-handler registration, and changes 404 handling behavior.
src/Web/Grand.Web.Common/Controllers/BaseController.cs	Adjusts error logging template for consistency.
src/Modules/Grand.Module.Api/Infrastructure/ApiAuthenticationRegistrar.cs	Returns ProblemDetails on auth failures and preserves exception stack traces.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-30T18:40:20Z

@@ -52,9 +60,9 @@ public void AddAuthentication(AuthenticationBuilder builder, IConfiguration conf
                            throw new Exception("API is disable");
                        }
                    }
-                    catch (Exception ex)
+                    catch (Exception)
                    {
-                        throw new Exception(ex.Message);
+                        throw;
                    }


The try/catch (Exception) { throw; } block is functionally redundant and adds noise. Since the goal is preserving the stack trace, you can remove the try/catch entirely and let exceptions propagate naturally.

Copilot · 2026-04-30T18:40:20Z

+        // UseStatusCodePagesWithReExecute sets IStatusCodePagesFeature.Enabled = true and re-executes
+        // the pipeline at the specified path when a 404 occurs, preserving the original 404 status code.
+        application.UseStatusCodePagesWithReExecute("/page-not-found");
+
+        // Disable status code pages for API (Bearer) requests and static resource requests so that
+        // those callers receive the original response rather than the HTML not-found page.
+        application.Use(async (context, next) =>


UseStatusCodePagesWithReExecute("/page-not-found") will re-execute for all 4xx/5xx responses with an empty body, not just 404. This can accidentally route 400/401/403/etc through the not-found endpoint (and also interferes with the earlier UseBadRequestResult() middleware). Consider disabling status-code pages unless Response.StatusCode == 404 (e.g., by toggling IStatusCodePagesFeature.Enabled after next runs) or using a path format that includes the status code and mapping 404 separately.

Copilot · 2026-04-30T18:40:20Z

                var contentTypeProvider = new FileExtensionContentTypeProvider();
-                var staticResource = contentTypeProvider.TryGetContentType(context.HttpContext.Request.Path, out _);
-
-                if (!apiRequest && !staticResource)
+                if (contentTypeProvider.TryGetContentType(context.Request.Path, out _))
                {
-                    const string location = "/page-not-found";
-                    context.HttpContext.Response.Redirect(context.HttpContext.Request.PathBase + location);
+                    var feature = context.Features.Get<IStatusCodePagesFeature>();
+                    if (feature != null)
+                        feature.Enabled = false;


FileExtensionContentTypeProvider is instantiated on every non-API request here. Since this middleware runs for all requests, this becomes a per-request allocation; consider reusing a static/singleton FileExtensionContentTypeProvider (it’s stateless) or another cheap static-resource check to avoid unnecessary allocations.

Copilot · 2026-04-30T18:40:21Z

+                    context.Response.StatusCode = StatusCodes.Status401Unauthorized;
+                    var problemDetailsService = context.HttpContext.RequestServices.GetService<IProblemDetailsService>();
+                    if (problemDetailsService != null)
+                        await problemDetailsService.WriteAsync(new ProblemDetailsContext {
+                            HttpContext = context.HttpContext,
+                            ProblemDetails = new ProblemDetails {
+                                Status = StatusCodes.Status401Unauthorized,
+                                Title = "Authentication failed"
+                            }
+                        });
+                },


OnAuthenticationFailed now writes a response body only when IProblemDetailsService is registered; if it isn’t, the handler returns with a 401 and an empty body/content-type. Add a fallback (e.g., set ContentType and write a minimal RFC7807 JSON payload) so API clients consistently receive a usable 401 response.

Copilot AI and others added 2 commits April 29, 2026 18:59

fix: use StartsWith for Authorization header scheme check (safer than…

4684aef

… Split()[0])" Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/ecc051c4-72f1-4d90-8ec9-025f19ab46c3 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>

Copilot AI assigned Copilot and KrzysztofPajak Apr 29, 2026

Copilot created this pull request from a session on behalf of KrzysztofPajak April 29, 2026 19:02 View session

Copilot finished work on behalf of KrzysztofPajak April 29, 2026 19:02

Copilot AI requested a review from KrzysztofPajak April 29, 2026 19:02

KrzysztofPajak reviewed Apr 30, 2026

View reviewed changes

Comment thread src/Web/Grand.Web.Common/Infrastructure/ApplicationBuilderExtensions.cs Outdated

Copilot started work on behalf of KrzysztofPajak April 30, 2026 18:23 View session

refactor: extract IsApiRequest helper to eliminate duplicate auth-hea…

ad1c5be

…der check Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/1ec7db4a-11fc-49b4-8aa2-85f12990ec16 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>

Copilot finished work on behalf of KrzysztofPajak April 30, 2026 18:27

Copilot AI requested a review from KrzysztofPajak April 30, 2026 18:27

KrzysztofPajak marked this pull request as ready for review April 30, 2026 18:34

Copilot AI review requested due to automatic review settings April 30, 2026 18:34

Copilot started reviewing on behalf of KrzysztofPajak April 30, 2026 18:35 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: implement ASP.NET Core 10 error handling best practices#685

fix: implement ASP.NET Core 10 error handling best practices#685
Copilot wants to merge 3 commits intodevelopfrom
copilot/check-error-handling-implementation

Copilot AI commented Apr 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI commented Apr 30, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Apr 30, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Apr 29, 2026 •

edited

Loading