-
Notifications
You must be signed in to change notification settings - Fork 10.3k
IExceptionHandler for exception handler middleware #46280 #47923
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Thanks for your PR, @Kahbazi. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
@Tratcher is there a reason you didn't merge this? |
src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs
Outdated
Show resolved
Hide resolved
Do we have any plans to use this outside of the unit tests? (in the product itself) @Tratcher |
I was just giving it a day for other reviews. We haven't identified anywhere in the product we'd use this feature. I suppose we could refactor the BadHttpRequest and ProblemDetails handling to use it? |
Yes, I was hoping this would be the case. |
@Kahbazi, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work! Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers. This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement. Thanks! |
Can we track this @Tratcher ? |
Fixes #46280