Skip to content
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

ASP.NET Core 2 middleware error handling #48

Closed
madskristensen opened this issue Oct 27, 2017 · 6 comments
Closed

ASP.NET Core 2 middleware error handling #48

madskristensen opened this issue Oct 27, 2017 · 6 comments

Comments

@madskristensen
Copy link

madskristensen commented Oct 27, 2017

When the HTML is malformed leading to an HTML parse error inside the WebMarkupMin middleware, there is no way to gracefully handle it.

Repro steps

  1. Add the WebMarkupMin middleware and verify it minifies the response HTML
  2. Add an unclosed <br tag anywhere on the page
  3. Refresh the browser

Actual
The middleware throws an exception and the page is therefore not shown - but instead an error message is displayed if no custom error page is defined.

Expected
When an error occurs, simple just about the minification and serve the unminified. That way the page isn't broken and the user can see the content

@Taritsyn
Copy link
Owner

Hello, Mads!

This issue has been discussed many times (for example, “drop minification in case of errors”). Just nobody reads the documentation.

@madskristensen
Copy link
Author

I did read it, but I don't know how to use the NullLogger. Any code samples?

@madskristensen
Copy link
Author

The NullLogger fixes the issue with the original repro step. However, add the following to any HTML reponse and it breaks again:

<?xml:namespace prefix />

@madskristensen
Copy link
Author

Also, another issue with the NullLogger is that it disables logging for everything in the application and not just WebMarkupMin. How about adding a Boolean property to the service options called something like options.ContinueOnError = true

@madskristensen
Copy link
Author

Never mind. I got confused by the names ILogger and NullLogger which both exist in a MS nuget package

@Taritsyn
Copy link
Owner

Hello, Mads!

<?xml:namespace prefix />

This error is handled correctly.

I got confused by the names ILogger and NullLogger which both exist in a MS nuget package

Unfortunately, I began to use such type names still in 2013.

Expected
When an error occurs, simple just about the minification and serve the unminified. That way the page isn't broken and the user can see the content

In next release I will make NullLogger the logger by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants