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
Show file tree
Hide file tree
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 Sep 27, 2024
b057c74
remove unused import
sdelamo Sep 27, 2024
ad1e55b
remove svgs
sdelamo Sep 27, 2024
a36464e
rename to error response body provider
sdelamo Sep 27, 2024
845e94a
extract request into a local variable and reuse
sdelamo Sep 27, 2024
5fd2b03
flip the condition
sdelamo Sep 27, 2024
4ded5ae
make class final
sdelamo Sep 27, 2024
0136f34
use @Requires missing beans instead of Secondary
sdelamo Sep 27, 2024
a50ca6d
fill empty method
sdelamo Sep 27, 2024
6f5b14c
Update http-server-netty/src/test/resources/logback.xml
sdelamo Sep 30, 2024
a288899
Update http-server/src/main/java/io/micronaut/http/server/exceptions/…
sdelamo Sep 30, 2024
de6f89c
initialize arraylist with size
sdelamo Sep 30, 2024
031a604
javadoc: add missing javadoc
sdelamo Sep 30, 2024
810bc55
check request does not accept json
sdelamo Sep 30, 2024
6918fc3
for HtmlErrorResponseBodyProvider specify generic as String
sdelamo Sep 30, 2024
810eb8e
don’t use generic for HtmlErrorResponseBodyProvider
sdelamo Sep 30, 2024
4b7c7d1
test matchesExtension
sdelamo Sep 30, 2024
27a15c3
Use HttpResponse:code() and HttpResponse::reason()
sdelamo Sep 30, 2024
6d63118
use MediaType::matchesExtensions
sdelamo Oct 2, 2024
3f95b0c
Add HtmlSanitizer
sdelamo Oct 2, 2024
23fc57d
fix test
sdelamo Oct 2, 2024
9ad37a7
add @Requires missingBeans
sdelamo Oct 2, 2024
b61a6d7
improve docs
sdelamo Oct 2, 2024
14eee77
Merge branch '4.7.x' into error-body-processor
sdelamo Oct 2, 2024
6757e00
remove javadoc of an exception not thrown
sdelamo Oct 2, 2024
8155278
remove trailing space
sdelamo Oct 2, 2024
0522e84
remove @Singleton annotation
sdelamo Oct 2, 2024
fb3590d
remove extra whitespace
sdelamo Oct 2, 2024
dfb9d75
simpler font-family
sdelamo Oct 2, 2024
91ebabd
fix test
sdelamo Oct 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import io.micronaut.http.codec.MediaTypeCodecRegistry;
import io.micronaut.inject.annotation.MutableAnnotationMetadata;

import java.io.IOException;
import java.util.Collections;
import java.util.function.Function;

Expand Down Expand Up @@ -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.

*/
public void run(String[] args, Function<ParseContext, ?> supplier) {
ApplicationContext applicationContext = this.applicationContext;
Expand Down
5 changes: 5 additions & 0 deletions http-server-netty/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ dependencies {
testImplementation project(":websocket")

testImplementation(libs.mimepull)

testImplementation(libs.micronaut.test.junit5) {
exclude group: 'io.micronaut'
}
testImplementation(libs.junit.jupiter.api)
}

tasks.withType(Test).configureEach {
Expand Down
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) {
}
}
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.
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) {

}
}
15 changes: 15 additions & 0 deletions http-server/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@ dependencies {
annotationProcessor project(":inject-java")

testImplementation libs.managed.netty.codec.http

testAnnotationProcessor(project(":inject-java"))
testAnnotationProcessor platform(libs.test.boms.micronaut.validation)
testAnnotationProcessor (libs.micronaut.validation.processor) {
exclude group: 'io.micronaut'
}
testImplementation platform(libs.test.boms.micronaut.validation)
testImplementation (libs.micronaut.validation) {
exclude group: 'io.micronaut'
}
testImplementation(libs.micronaut.test.junit5) {
exclude group: 'io.micronaut'
}
testImplementation(libs.junit.jupiter.api)
testImplementation(libs.junit.jupiter.params)
}

//compileTestGroovy.groovyOptions.forkOptions.jvmArgs = ['-Xdebug', '-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005']
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());
}
}
Loading
Loading