-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
e6ac16c
feat: Improved HTML Error pages
sdelamo b057c74
remove unused import
sdelamo ad1e55b
remove svgs
sdelamo a36464e
rename to error response body provider
sdelamo 845e94a
extract request into a local variable and reuse
sdelamo 5fd2b03
flip the condition
sdelamo 4ded5ae
make class final
sdelamo 0136f34
use @Requires missing beans instead of Secondary
sdelamo a50ca6d
fill empty method
sdelamo 6f5b14c
Update http-server-netty/src/test/resources/logback.xml
sdelamo a288899
Update http-server/src/main/java/io/micronaut/http/server/exceptions/…
sdelamo de6f89c
initialize arraylist with size
sdelamo 031a604
javadoc: add missing javadoc
sdelamo 810bc55
check request does not accept json
sdelamo 6918fc3
for HtmlErrorResponseBodyProvider specify generic as String
sdelamo 810eb8e
don’t use generic for HtmlErrorResponseBodyProvider
sdelamo 4b7c7d1
test matchesExtension
sdelamo 27a15c3
Use HttpResponse:code() and HttpResponse::reason()
sdelamo 6d63118
use MediaType::matchesExtensions
sdelamo 3f95b0c
Add HtmlSanitizer
sdelamo 23fc57d
fix test
sdelamo 9ad37a7
add @Requires missingBeans
sdelamo b61a6d7
improve docs
sdelamo 14eee77
Merge branch '4.7.x' into error-body-processor
sdelamo 6757e00
remove javadoc of an exception not thrown
sdelamo 8155278
remove trailing space
sdelamo 0522e84
remove @Singleton annotation
sdelamo fb3590d
remove extra whitespace
sdelamo dfb9d75
simpler font-family
sdelamo 91ebabd
fix test
sdelamo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
134 changes: 134 additions & 0 deletions
134
...o/micronaut/http/server/exceptions/response/DefaultHtmlErrorResponseBodyProviderTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
package io.micronaut.http.server.exceptions.response; | ||
|
||
import io.micronaut.context.MessageSource; | ||
import io.micronaut.context.annotation.Factory; | ||
import io.micronaut.context.annotation.Property; | ||
import io.micronaut.context.annotation.Requires; | ||
import io.micronaut.context.i18n.ResourceBundleMessageSource; | ||
import io.micronaut.core.annotation.Introspected; | ||
import io.micronaut.http.*; | ||
import io.micronaut.http.annotation.Body; | ||
import io.micronaut.http.annotation.Controller; | ||
import io.micronaut.http.annotation.Post; | ||
import io.micronaut.http.annotation.Produces; | ||
import io.micronaut.http.annotation.Status; | ||
import io.micronaut.http.client.BlockingHttpClient; | ||
import io.micronaut.http.client.HttpClient; | ||
import io.micronaut.http.client.annotation.Client; | ||
import io.micronaut.http.client.exceptions.HttpClientResponseException; | ||
import io.micronaut.test.extensions.junit5.annotation.MicronautTest; | ||
import jakarta.inject.Inject; | ||
import jakarta.inject.Singleton; | ||
import jakarta.validation.Valid; | ||
import jakarta.validation.constraints.Max; | ||
import jakarta.validation.constraints.NotBlank; | ||
import org.junit.jupiter.api.Test; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import spock.lang.Specification; | ||
|
||
import java.util.Optional; | ||
|
||
import static org.junit.Assert.assertFalse; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
||
@Property(name = "spec.name", value = "DefaultHtmlBodyErrorResponseProviderTest") | ||
@MicronautTest | ||
class DefaultHtmlErrorResponseBodyProviderTest extends Specification { | ||
private static final Logger LOG = LoggerFactory.getLogger(DefaultHtmlErrorResponseBodyProviderTest.class); | ||
|
||
@Inject | ||
HtmlErrorResponseBodyProvider htmlProvider; | ||
|
||
@Client("/") | ||
@Inject | ||
HttpClient httpClient; | ||
|
||
@Test | ||
void ifRequestAcceptsBothJsonAnHtmlJsonIsUsed() { | ||
BlockingHttpClient client = httpClient.toBlocking(); | ||
HttpClientResponseException ex = assertThrows(HttpClientResponseException.class, () -> | ||
client.exchange(HttpRequest.POST("/book/save", new Book("Building Microservices", "", 5000)) | ||
.accept(MediaType.TEXT_HTML, MediaType.APPLICATION_JSON))); | ||
assertEquals(HttpStatus.BAD_REQUEST, ex.getStatus()); | ||
assertTrue(ex.getResponse().getContentType().isPresent()); | ||
assertEquals(MediaType.APPLICATION_JSON, ex.getResponse().getContentType().get().toString()); | ||
Optional<String> jsonOptional = ex.getResponse().getBody(String.class); | ||
assertTrue(jsonOptional.isPresent()); | ||
String json = jsonOptional.get(); | ||
assertFalse(json.contains("<!doctype html>")); | ||
} | ||
|
||
@Test | ||
void validationErrorsShowInHtmlErrorPages() { | ||
BlockingHttpClient client = httpClient.toBlocking(); | ||
HttpClientResponseException ex = assertThrows(HttpClientResponseException.class, () -> | ||
client.exchange(HttpRequest.POST("/book/save", new Book("Building Microservices", "", 5000)) | ||
.accept(MediaType.TEXT_HTML))); | ||
assertEquals(HttpStatus.BAD_REQUEST, ex.getStatus()); | ||
assertTrue(ex.getResponse().getContentType().isPresent()); | ||
assertEquals(MediaType.TEXT_HTML, ex.getResponse().getContentType().get().toString()); | ||
Optional<String> htmlOptional = ex.getResponse().getBody(String.class); | ||
assertTrue(htmlOptional.isPresent()); | ||
String html = htmlOptional.get(); | ||
assertExpectedSubstringInHtml("<!doctype html>", html); | ||
assertExpectedSubstringInHtml("book.author: must not be blank", html); | ||
assertExpectedSubstringInHtml("book.pages: must be less than or equal to 4032", html); | ||
|
||
|
||
ex = assertThrows(HttpClientResponseException.class, () -> client.exchange(HttpRequest.GET("/paginanoencontrada").accept(MediaType.TEXT_HTML))); | ||
assertEquals(HttpStatus.NOT_FOUND, ex.getStatus()); | ||
htmlOptional = ex.getResponse().getBody(String.class); | ||
assertTrue(htmlOptional.isPresent()); | ||
html = htmlOptional.get(); | ||
assertExpectedSubstringInHtml("<!doctype html>", html); | ||
assertExpectedSubstringInHtml("Not Found", html); | ||
assertExpectedSubstringInHtml("The page you were looking for doesn’t exist", html); | ||
assertExpectedSubstringInHtml("You may have mistyped the address or the page may have moved", html); | ||
|
||
|
||
ex = assertThrows(HttpClientResponseException.class, () -> client.exchange(HttpRequest.GET("/paginanoencontrada").header(HttpHeaders.ACCEPT_LANGUAGE, "es").accept(MediaType.TEXT_HTML))); | ||
assertEquals(HttpStatus.NOT_FOUND, ex.getStatus()); | ||
htmlOptional = ex.getResponse().getBody(String.class); | ||
assertTrue(htmlOptional.isPresent()); | ||
html = htmlOptional.get(); | ||
assertExpectedSubstringInHtml("<!doctype html>", html); | ||
assertExpectedSubstringInHtml("No encontrado", html); | ||
assertExpectedSubstringInHtml("La página que buscabas no existe", html); | ||
assertExpectedSubstringInHtml("Es posible que haya escrito mal la dirección o que la página se haya movido.", html); | ||
} | ||
|
||
private void assertExpectedSubstringInHtml(String expected, String html) { | ||
if (!html.contains(expected)) { | ||
LOG.trace("{}", html); | ||
} | ||
assertTrue(html.contains(expected)); | ||
} | ||
|
||
@Requires(property = "spec.name", value = "DefaultHtmlBodyErrorResponseProviderTest") | ||
@Controller("/book") | ||
static class FooController { | ||
|
||
@Produces(MediaType.TEXT_HTML) | ||
@Post("/save") | ||
@Status(HttpStatus.CREATED) | ||
void save(@Body @Valid Book book) { | ||
throw new UnsupportedOperationException(); | ||
} | ||
} | ||
|
||
@Requires(property = "spec.name", value = "DefaultHtmlBodyErrorResponseProviderTest") | ||
@Factory | ||
static class MessageSourceFactory { | ||
@Singleton | ||
MessageSource createMessageSource() { | ||
return new ResourceBundleMessageSource("i18n.messages"); | ||
} | ||
} | ||
|
||
@Introspected | ||
record Book(@NotBlank String title, @NotBlank String author, @Max(4032) int pages) { | ||
} | ||
} |
5 changes: 4 additions & 1 deletion
5
http-server-netty/src/test/resources/i18n/messages_es.properties
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,5 @@ | ||
hello=Hola | ||
welcome.name=Bienvenido {0} | ||
welcome.name=Bienvenido {0} | ||
404.error.bold=La página que buscabas no existe | ||
404.error.title=No encontrado | ||
404.error=Es posible que haya escrito mal la dirección o que la página se haya movido. |
80 changes: 80 additions & 0 deletions
80
...er-tck/src/main/java/io/micronaut/http/server/tck/tests/exceptions/HtmlErrorPageTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
/* | ||
* Copyright 2017-2024 original authors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package io.micronaut.http.server.tck.tests.exceptions; | ||
|
||
import io.micronaut.context.annotation.Requires; | ||
import io.micronaut.core.annotation.Introspected; | ||
import io.micronaut.http.HttpHeaders; | ||
import io.micronaut.http.HttpRequest; | ||
import io.micronaut.http.HttpStatus; | ||
import io.micronaut.http.MediaType; | ||
import io.micronaut.http.annotation.*; | ||
import io.micronaut.http.tck.AssertionUtils; | ||
import io.micronaut.http.tck.BodyAssertion; | ||
import io.micronaut.http.tck.HttpResponseAssertion; | ||
import jakarta.validation.Valid; | ||
import jakarta.validation.constraints.Max; | ||
import jakarta.validation.constraints.NotBlank; | ||
import org.junit.jupiter.api.Test; | ||
|
||
import java.io.IOException; | ||
import java.util.Map; | ||
|
||
import static io.micronaut.http.tck.TestScenario.asserts; | ||
|
||
@SuppressWarnings({ | ||
"java:S5960", // We're allowed assertions, as these are used in tests only | ||
"checkstyle:MissingJavadocType", | ||
"checkstyle:DesignForExtension" | ||
}) | ||
public class HtmlErrorPageTest { | ||
private static final String SPEC_NAME = "HtmlErrorPageTest"; | ||
|
||
@Test | ||
void htmlErrorPage() throws IOException { | ||
asserts(SPEC_NAME, | ||
HttpRequest.POST("/book/save", new Book("Building Microservices", "", 5000)).accept(MediaType.TEXT_HTML), | ||
(server, request) -> AssertionUtils.assertThrows( | ||
server, | ||
request, | ||
HttpResponseAssertion.builder() | ||
.status(HttpStatus.BAD_REQUEST) | ||
.body(BodyAssertion.builder().body("<!doctype html>").contains()) | ||
.body(BodyAssertion.builder().body("book.author: must not be blank").contains()) | ||
.body(BodyAssertion.builder().body("book.pages: must be less than or equal to 4032").contains()) | ||
.headers(Map.of(HttpHeaders.CONTENT_TYPE, MediaType.TEXT_HTML)) | ||
.build() | ||
) | ||
); | ||
} | ||
|
||
@Requires(property = "spec.name", value = "HtmlErrorPageTest") | ||
@Controller("/book") | ||
static class FooController { | ||
|
||
@Produces(MediaType.TEXT_HTML) | ||
@Post("/save") | ||
@Status(HttpStatus.CREATED) | ||
void save(@Body @Valid Book book) { | ||
throw new UnsupportedOperationException(); | ||
} | ||
} | ||
|
||
@Introspected | ||
record Book(@NotBlank String title, @NotBlank String author, @Max(4032) int pages) { | ||
|
||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
61 changes: 61 additions & 0 deletions
61
...main/java/io/micronaut/http/server/exceptions/response/DefaultErrorResponseProcessor.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/* | ||
* Copyright 2017-2024 original authors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package io.micronaut.http.server.exceptions.response; | ||
|
||
import io.micronaut.context.annotation.Requires; | ||
import io.micronaut.core.annotation.Internal; | ||
import io.micronaut.http.*; | ||
import io.micronaut.http.hateoas.JsonError; | ||
import jakarta.inject.Singleton; | ||
|
||
/** | ||
* Default implementation of {@link ErrorResponseProcessor}. | ||
* It delegates to {@link JsonErrorResponseBodyProvider} for JSON responses and to {@link HtmlErrorResponseBodyProvider} for HTML responses. | ||
* | ||
* @author Sergio del Amo | ||
* @since 4.7.0 | ||
*/ | ||
@Internal | ||
@Singleton | ||
@Requires(missingBeans = ErrorResponseProcessor.class) | ||
final class DefaultErrorResponseProcessor implements ErrorResponseProcessor { | ||
private final JsonErrorResponseBodyProvider<?> jsonBodyErrorResponseProvider; | ||
private final HtmlErrorResponseBodyProvider htmlBodyErrorResponseProvider; | ||
|
||
DefaultErrorResponseProcessor(JsonErrorResponseBodyProvider<?> jsonBodyErrorResponseProvider, | ||
HtmlErrorResponseBodyProvider htmlBodyErrorResponseProvider) { | ||
this.jsonBodyErrorResponseProvider = jsonBodyErrorResponseProvider; | ||
this.htmlBodyErrorResponseProvider = htmlBodyErrorResponseProvider; | ||
} | ||
|
||
@Override | ||
public MutableHttpResponse processResponse(ErrorContext errorContext, MutableHttpResponse response) { | ||
HttpRequest<?> request = errorContext.getRequest(); | ||
if (request.getMethod() == HttpMethod.HEAD) { | ||
return (MutableHttpResponse<JsonError>) response; | ||
} | ||
final boolean isError = response.status().getCode() >= 400; | ||
if (isError | ||
&& request.accept().stream().anyMatch(mediaType -> mediaType.equals(MediaType.TEXT_HTML_TYPE)) | ||
&& request.accept().stream().noneMatch(m -> m.matchesExtension(MediaType.EXTENSION_JSON)) | ||
) { | ||
return response.body(htmlBodyErrorResponseProvider.body(errorContext, response)) | ||
.contentType(htmlBodyErrorResponseProvider.contentType()); | ||
} | ||
return response.body(jsonBodyErrorResponseProvider.body(errorContext, response)) | ||
.contentType(jsonBodyErrorResponseProvider.contentType()); | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.