Skip to content

Diagnostics Exception Handler override #3134

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

Closed
MoeHamdan opened this issue May 11, 2018 · 7 comments
Closed

Diagnostics Exception Handler override #3134

MoeHamdan opened this issue May 11, 2018 · 7 comments

Comments

@MoeHamdan
Copy link

Hello

I want to override ExceptionHandlerMiddleware and DeveloperExceptionPageMiddleware to enable them to support "application/json" content type, so they could return proper json error when the content is json. However your invoke method is not marked as virtual, may you mark it as virtual so we could extend this?

@Tratcher
Copy link
Member

The Invoke method is the bulk of the middleware, you'd be better off writing a new middleware.

@MoeHamdan
Copy link
Author

Hello @Tratcher

If write a new middleware I will lose all the functionality implemented by these middleware, I will need to re implement all functionalities, it does not make sense to do so.

@Tratcher
Copy link
Member

If you override Invoke you'll need to re-implement most of it anyways. What do you gain?

Maybe you could give an example of what you had in mind?

@MoeHamdan
Copy link
Author

Well all I want is to check if the content type is JSON then return JSON error instead of HTML, which is a couple of simple lines, I wonder why this is not there, it used to be part of .NET.

@Tratcher
Copy link
Member

I don't see how this applies to the ExceptionHandlerMiddleware, it doesn't generate any HTML, it forwards the request to a component that can such as an MVC controller. You can already enhance that component to serve HTML or JSON.

The DeveloperExceptionPage is only for use in a development environment so the format of the error should be less relevant, it's not intended for programmatic processing. As for changing that to JSON, the only code you would still want is inside the Invoke method (https://github.com/aspnet/Diagnostics/blob/8dce01f654962580e22e7c96ad4f313574407a40/src/Microsoft.AspNetCore.Diagnostics/DeveloperExceptionPage/DeveloperExceptionPageMiddleware.cs#L70-L107) so overriding it doesn't help you.

@MoeHamdan
Copy link
Author

Hello @Tratcher

Thank you very much for your help. I know that DeveloperExceptionPage is for development, it is very important to us as you know most of the time you are in development mode, so the thing is I would like to keep the current behavior of the HTML part and introduce the JSON behavior as well when the content type is "application/json". If you see that inheriting the middleware is not the best practice, is it possible that you enhance these middleware to server JSON?

Thanks,

@Tratcher
Copy link
Member

Adding JSON support directly is a different matter. That's tracked over at #2573.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants