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

feat: nicer error pages for HTML responses #11210

Merged
merged 30 commits into from
Oct 2, 2024
Merged

Conversation

sdelamo
Copy link
Contributor

@sdelamo sdelamo commented Sep 27, 2024

This PR changes the error processor to respond to a nice HTML error page if the request accepts an HTML Response. Similar to the error pages of Kamal proxy. The background blue, is the same as the blue used in the Micronaut Website.

This pull request also adds a HtmlSanitizer API, which Micronaut Security will further enhance.

I generated a SVG per status code, using a Shortcuts + Pixelmator script.

For JSON responses, the application will respond vnd.erroras it currently does.

It also allows multiple body assertions in TCK.

Currently, a not found page is displayed as:

CleanShot 2024-09-27 at 14 23 07@2x

With the changes in this pull-request, it is displayed as:

CleanShot 2024-09-27 at 14 22 33@2x

A Bad request (e.g., after a validation) currently is displayed as:

CleanShot 2024-09-27 at 14 24 00@2x

With the changes in this pull-request, it is displayed as:

CleanShot 2024-09-27 at 14 26 10@2x

If the request accepts an HTML Response, respond a nice error page.

Allow multiple body assertions in tck.
@sdelamo sdelamo added the type: enhancement New feature or request label Sep 27, 2024
Copy link
Contributor

@graemerocher graemerocher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't comment directly in the diff for https://github.com/micronaut-projects/micronaut-core/blob/b057c7457fb34429649b0967b1994f3dcc1ec0f0/http-server/src/main/java/io/micronaut/http/server/exceptions/response/DefaultHtmlBodyErrorResponseProvider.java for some reason

But this class should use a concurrent linked hash map to avoid growing too big or attacks where someone specifies infinite locales and thus filling memory

Copy link
Contributor Author

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@graemerocher, I have removed the SVG maps and replaced them with CSS, which has a similar effect. I did this to reduce the size of the file. With an SVG for every status code, the size was more than 1Mb.

You should now be able to see the file in the GitHub Pull-Request review.

*/
@Internal
@Singleton
final class DefaultHtmlErrorResponseBodyProvider implements HtmlErrorResponseBodyProvider<String> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe all the templates can be a property, so a user can override it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the class internal. If I want to the class extendable I would make it non internal. For now, I think you can provide your own implementation of HtmlErrorResponseBodyProvider if you want to customize it. Or provide localization for other languages.

@@ -96,7 +95,6 @@ public void close() {
*
* @param args The arguments passed to main
* @param supplier The function that executes this function
* @throws IOException If an error occurs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, Javadoc complains of a throw javadoc for an exception not being thrown.

Copy link

sonarcloud bot commented Oct 2, 2024

@sdelamo sdelamo merged commit eccab50 into 4.7.x Oct 2, 2024
21 checks passed
@sdelamo sdelamo deleted the error-body-processor branch October 2, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants