feat: add global exception handling middleware with RFC 7807 support #321
Conversation
- Create ExceptionMiddleware with RFC 7807 Problem Details format - Map exception types to appropriate HTTP status codes - Include stack traces in Development environment only - Add structured logging with Serilog integration - Register middleware in pipeline after Serilog request logging - Cache JsonSerializerOptions instance to avoid CA1869 warning - Exclude Middleware folder from Codecov and Codacy coverage - Maintain backward compatibility with existing validation handling
- Add HasStarted check before modifying response - Prevent errors when exception occurs after headers sent - Log warning when unable to write error response - Use authoritative Mozilla documentation for error types
WalkthroughAdds a global ExceptionMiddleware that returns RFC 7807 Problem Details for unhandled exceptions, an extension method UseExceptionHandling to register it in the pipeline, and updates CI/config files to use double-quoted patterns and exclude /Middlewares/ from analysis tools. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ASP as "ASP.NET Pipeline"
participant EM as "ExceptionMiddleware"
participant Logger as "ILogger/Serilog"
participant Http as "HttpContext"
Client->>ASP: HTTP Request
ASP->>EM: Invoke(next) (try)
alt Handler throws
EM-->>EM: Map exception → status/title/type
EM->>Logger: Log error (structured, traceId)
EM->>Http: Build ProblemDetails (type,title,status,detail,instance)
EM->>Http: Set Content-Type: application/problem+json
EM->>Http: Check Response.HasStarted
alt Not started
EM->>Http: Write JSON payload
EM-->>Client: ProblemDetails response (status)
else Already started
EM->>Logger: Log warning (response started)
EM-->>Client: (no modification)
end
else No exception
ASP->>ASP: Continue pipeline
ASP-->>Client: Normal response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs
Dismissed
Show dismissed
Hide dismissed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.codacy.yml (1)
28-28: Consider whether to exclude Middlewares from static analysis.Excluding
**/Middlewares/**from Codacy analysis means the newExceptionMiddlewarewon't be analyzed for code quality issues. Since this middleware is a core infrastructure component handling all unhandled exceptions, consider whether static analysis should still apply to catch potential issues.src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs (1)
88-90: Redundant pattern match forArgumentNullException.
ArgumentNullExceptioninherits fromArgumentException, so it will already match theArgumentExceptioncase. The explicitArgumentNullExceptionarm is unreachable.return exception switch { ValidationException => (StatusCodes.Status400BadRequest, "Validation Error"), - ArgumentException - or ArgumentNullException - => (StatusCodes.Status400BadRequest, "Bad Request"), + ArgumentException => (StatusCodes.Status400BadRequest, "Bad Request"), InvalidOperationException => (StatusCodes.Status400BadRequest, "Invalid Operation"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.codacy.yml(1 hunks)codecov.yml(3 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Program.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: UsePascalCasefor class names, methods, and public properties in C#
UsecamelCasefor local variables and private fields in C#
Useasync/awaitconsistently for asynchronous code in C#
Prefervarfor local variable declarations where the type is obvious in C#
Format code with CSharpier formatting standards
Use Dependency Injection for all services and repositories in ASP.NET Core
Write EF Core queries using LINQ with async operations (e.g., FirstOrDefaultAsync instead of FirstOrDefault)
UseAsNoTracking()for read-only EF Core queries to improve performance
UseILogger<T>for structured logging in ASP.NET Core
Do not use EF Core synchronous APIs (use async alternatives like FirstOrDefaultAsync)
Do not useConfigureAwait(false)in ASP.NET Core contexts
Use DbContextPool configuration for improved EF Core performance in ASP.NET Core
Files:
src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cssrc/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cssrc/Dotnet.Samples.AspNetCore.WebApi/Program.cs
🧠 Learnings (4)
📚 Learning: 2025-11-25T18:04:01.330Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T18:04:01.330Z
Learning: Applies to **/Controllers/**/*.cs : Implement consistent error response formats in ASP.NET Core controllers
Applied to files:
src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cssrc/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cssrc/Dotnet.Samples.AspNetCore.WebApi/Program.cs
📚 Learning: 2025-11-25T18:04:01.330Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T18:04:01.330Z
Learning: Avoid adding unnecessary middleware or filters without clear purpose in ASP.NET Core
Applied to files:
src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cssrc/Dotnet.Samples.AspNetCore.WebApi/Program.cs
📚 Learning: 2025-11-25T18:04:01.330Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T18:04:01.330Z
Learning: Applies to **/*.cs : Use `ILogger<T>` for structured logging in ASP.NET Core
Applied to files:
src/Dotnet.Samples.AspNetCore.WebApi/Program.cs
📚 Learning: 2025-11-25T18:04:01.330Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T18:04:01.330Z
Learning: Applies to **/Services/**/*.cs : Use Service Layer for business logic encapsulation in ASP.NET Core
Applied to files:
src/Dotnet.Samples.AspNetCore.WebApi/Program.cs
🧬 Code graph analysis (1)
src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs (1)
src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs (1)
ExceptionMiddleware(11-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: coverage (codacy)
- GitHub Check: coverage (codecov)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (csharp)
🔇 Additional comments (8)
codecov.yml (1)
62-62: Verify intent to exclude Middlewares from code coverage.The PR objectives mention adding unit tests for the middleware. Excluding
Middlewares/from Codecov means any tests written won't contribute to coverage metrics. If unit tests are planned (per issue #184), consider removing this exclusion to track middleware test coverage.src/Dotnet.Samples.AspNetCore.WebApi/Program.cs (1)
57-59: LGTM!The middleware is correctly positioned after
UseSerilogRequestLogging()(ensuring request logging is captured) and early enough in the pipeline to catch exceptions from downstream middleware likeUseHttpsRedirection(),UseRateLimiter(), andMapControllers(). Based on learnings, this middleware has a clear purpose for centralized exception handling.src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs (1)
1-21: LGTM!Clean implementation following ASP.NET Core conventions. The extension method pattern is appropriate, XML documentation is thorough, and returning
WebApplicationenables fluent chaining. This aligns with the codebase's existing extension method patterns in theExtensionsnamespace.src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs (5)
1-16: LGTM!Good use of C# primary constructor syntax for dependency injection. Static
JsonOptionswith camelCase naming is appropriate for JSON serialization. UsingILogger<ExceptionMiddleware>aligns with coding guidelines for structured logging.
21-31: LGTM!Standard ASP.NET Core middleware pattern with proper async/await usage. The try-catch wrapping
next(context)correctly catches exceptions from all downstream middleware and handlers.
36-78: Well-structured exception handling with RFC 7807 compliance.The implementation correctly:
- Builds
ProblemDetailswith required fields (type, title, status, detail, instance)- Adds
traceIdfor request correlation- Uses structured logging with the exception object
- Guards against
HasStartedbefore writing the response
103-116: LGTM!Environment-aware detail generation is a good security practice. Development includes full stack traces for debugging, while production sanitizes sensitive information. The pattern matching for specific exception types in production ensures validation errors remain informative.
93-93: Reconsider mappingOperationCanceledExceptionto 408 without distinguishing causation.HTTP 408 Request Timeout is semantically correct only when the server times out.
OperationCanceledExceptioncan arise from two different sources:
- Client disconnect (HttpContext.RequestAborted): the exception occurs because the client closed the connection mid-request. In this case, you cannot send a response at all—attempting to write 408 will fail.
- Server-side timeout: your code's cancellation token was triggered by a server timeout. Here, 408 is appropriate if response hasn't started.
The current static
MapExceptionToStatusCodemethod lacks HttpContext access, so it cannot differentiate. Either:
- Pass HttpContext to detect
HttpContext.RequestAborted.IsCancellationRequestedand branch accordingly (no response for client disconnect, 408 for server timeout).- Stop catching
OperationCanceledExceptionaltogether and let Kestrel handle it (avoids the incorrect 408 for client disconnects).- Map to 500 if causation is unknown.
Note: HTTP 499 is non-standard and only used by proxies/infrastructure for logging; do not return it from your application.
Add suppression comments trusting Serilog's @ destructuring to automatically escape control characters, avoiding redundant sanitization.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs (2)
85-95: Minor:ArgumentNullExceptionis redundant in the pattern.
ArgumentNullExceptionderives fromArgumentException, so the explicitor ArgumentNullExceptionclause is unnecessary—theArgumentExceptioncase already catches it.Additionally, consider whether
OperationCanceledExceptionshould always map to 408. If the client disconnects mid-request, the cancellation token fires, but 408 implies the server timed out waiting for the client. Some APIs use 499 (nginx-style) or simply don't respond. This is acceptable if intentional.return exception switch { ValidationException => (StatusCodes.Status400BadRequest, "Validation Error"), - ArgumentException - or ArgumentNullException - => (StatusCodes.Status400BadRequest, "Bad Request"), + ArgumentException => (StatusCodes.Status400BadRequest, "Bad Request"), InvalidOperationException => (StatusCodes.Status400BadRequest, "Invalid Operation"), DbUpdateConcurrencyException => (StatusCodes.Status409Conflict, "Concurrency Conflict"), OperationCanceledException => (StatusCodes.Status408RequestTimeout, "Request Timeout"), _ => (StatusCodes.Status500InternalServerError, "Internal Server Error") };
103-116: Handle potentially nullStackTrace.
Exception.StackTracecan benullin edge cases (e.g., exception created but not thrown). Consider null-coalescing to avoid "null" appearing in the output.private string GetExceptionDetail(Exception exception) { if (environment.IsDevelopment()) { - return $"{exception.Message}\n\nStack Trace:\n{exception.StackTrace}"; + return $"{exception.Message}\n\nStack Trace:\n{exception.StackTrace ?? "N/A"}"; } return exception switch { ValidationException => exception.Message, ArgumentException => exception.Message, _ => "An unexpected error occurred while processing your request." }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: UsePascalCasefor class names, methods, and public properties in C#
UsecamelCasefor local variables and private fields in C#
Useasync/awaitconsistently for asynchronous code in C#
Prefervarfor local variable declarations where the type is obvious in C#
Format code with CSharpier formatting standards
Use Dependency Injection for all services and repositories in ASP.NET Core
Write EF Core queries using LINQ with async operations (e.g., FirstOrDefaultAsync instead of FirstOrDefault)
UseAsNoTracking()for read-only EF Core queries to improve performance
UseILogger<T>for structured logging in ASP.NET Core
Do not use EF Core synchronous APIs (use async alternatives like FirstOrDefaultAsync)
Do not useConfigureAwait(false)in ASP.NET Core contexts
Use DbContextPool configuration for improved EF Core performance in ASP.NET Core
Files:
src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cssrc/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs
**/Controllers/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/Controllers/**/*.cs: Generate idiomatic ASP.NET Core controller actions with proper HTTP status codes (200, 201, 400, 404, 409, 500)
Use proper HTTP verbs (GET, POST, PUT, DELETE) in ASP.NET Core controller routes
Return appropriate status codes (200, 201, 400, 404, 409, 500) from ASP.NET Core controllers
Implement consistent error response formats in ASP.NET Core controllers
Use route parameters for resource identification in ASP.NET Core APIs
Files:
src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs
🧠 Learnings (4)
📚 Learning: 2025-11-25T18:04:01.330Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T18:04:01.330Z
Learning: Applies to **/Controllers/**/*.cs : Use proper HTTP verbs (GET, POST, PUT, DELETE) in ASP.NET Core controller routes
Applied to files:
src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs
📚 Learning: 2025-11-25T18:04:01.330Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T18:04:01.330Z
Learning: Applies to **/Controllers/**/*.cs : Generate idiomatic ASP.NET Core controller actions with proper HTTP status codes (200, 201, 400, 404, 409, 500)
Applied to files:
src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs
📚 Learning: 2025-11-25T18:04:01.330Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T18:04:01.330Z
Learning: Avoid adding unnecessary middleware or filters without clear purpose in ASP.NET Core
Applied to files:
src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs
📚 Learning: 2025-11-25T18:04:01.330Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T18:04:01.330Z
Learning: Applies to **/Controllers/**/*.cs : Implement consistent error response formats in ASP.NET Core controllers
Applied to files:
src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: coverage (codecov)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (csharp)
🔇 Additional comments (2)
src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs (1)
192-193: LGTM! Good defensive documentation.The CodeQL suppression comment appropriately addresses the log-forging concern for logging user-provided input. The explanation that Serilog's
@destructuring operator escapes control characters is accurate and helps future maintainers understand why this is safe.For consistency, you might consider whether line 45 (logging validation errors derived from user input) would benefit from a similar comment, though FluentValidation's error messages are already sanitized.
src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs (1)
36-78: Exception handling implementation looks solid.The method correctly implements RFC 7807 Problem Details with all required fields. Good defensive check for
HasStartedbefore writing the response, and appropriate fallback logging when the response has already started.The log forging concern on line 57 (flagged by code scanning) is mitigated by Serilog's structured logging, which automatically escapes control characters when using message templates with placeholders.
Closes #184
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.