dotnet / AspNetCore.Docs

Documentation for ASP.NET Core
https://docs.microsoft.com/aspnet/core
Creative Commons Attribution 4.0 International
12.64k stars 25.29k forks source link

UseExceptionHandler vs GlobalExceptionMiddleware #20949

Closed aehomay closed 3 years ago

aehomay commented 3 years ago

I read this article but what I am not understanding is that while there is Middleware Exception Handling in place since the early stages of .Net Core which allows us to implement a custom Middleware for capturing all exceptions and logging them without requiring a controller (see here) why one should go with UseExceptionHandler("/error") solution that involves the implementation of an extra controller (called ErrorController) to handle exceptions. As you know this will take the opportunity of having a common library for exception handling across all projects (Web APIs) in a product.


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

pranavkm commented 3 years ago

You're free to roll out a custom middleware if you'd like to. ExceptionHandlerMiddleware makes it easier to use a familiar construct - a controller action to handle these error scenarios. Also, it's implementation is much more robust compared to the sample implementation from the linked blog post.

aehomay commented 3 years ago

But then I have to consider implementing controller action to handle errors in every each API, while I can implement custom exception handling Middleware and put it a common library and just consume the library in every API that is being developed.

pranavkm commented 3 years ago

while I can implement custom exception handling Middleware and put it a common library and just consume the library in every API that is being developed

That's a great idea if you have the same error handling requirements across your apps.

aehomay commented 3 years ago

Would it be possible to enlighten me more on what kind of requirement could be different? For example in the domain that I am developing APIs, every exception handling mechanism requires logging the exception via ILogger this is common in all Web APIs, so this could go to a common library via Middleware Exception Handling. Am I wrong?

Rick-Anderson commented 3 years ago

@aehomay would you be willing to help update this doc with your approach once it's working?

aehomay commented 3 years ago

@Rick-Anderson Yes sure, in fact, my approach is working and it is not a new approach it has been discussed here already. The difference between my approach and the article that I mentioned from 2017 is that in my approach I considered RFC7807 as well to cover the "Problem Details" as well.

aehomay commented 3 years ago

@Rick-Anderson Are you going to enhance this document for covering custom middleware exception handling by yourself? If yes maybe RFC7807 could also be considered.

aehomay commented 3 years ago

@Rick-Anderson I am thinking to take the idea of exception middleware further to problem details middleware. Do you think would make sense to create a separate page for that or just update this document?

Rick-Anderson commented 3 years ago

@Rick-Anderson I am thinking to take the idea of exception middleware further to problem details middleware. Do you think would make sense to create a separate page for that or just update this document?

Probably just this doc, but we could always move it .

pranavkm commented 3 years ago

Honestly I think this makes for a good blog post but not for good doc content. The error handling doc that this doc article links to already covers all of the patterns that we would recommend to our users for their error handling.

aehomay commented 3 years ago

Okay so if you want I can add only the Middleware exception handling in this doc. Do I need special permission to do this?

Rick-Anderson commented 3 years ago

At the top right of the article, select the Edit pen icon:

image

Select the pen icon again. Save.

aehomay commented 3 years ago

Hi @Rick-Anderson I tried to add my own example of implementing Middleware Exception Handling but I faced with 403 when I wanted to commit my source to the repository. Therefore I only added the main section with a link to an example from 2017 in here. Is any way that you can grant me access to add my own implementation or you think it is not necessary.

Rick-Anderson commented 3 years ago

You don't have write permissions to this repo, so you need to fork the repo. That's how GitHub works.

aehomay commented 3 years ago

@Rick-Anderson Yes my bad, then I will fork it and create a feature branch and do the work there then will create a pull request. Will take care of it soon.

aehomay commented 3 years ago

@Rick-Anderson I moved changes to aehomay:homay-patch1 and created a pull request, as you have already reviewed I didn`t assign it to you to review again because I only fixed a typo on the title and in the paragraph. I am going to create a new branch and name it aehomay:homay-patch2 which will contain the example implementation.