Skip to content

Add requestId information to ErrorAttributes and whitelabel error page in WebFlux #15952

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.

Sign up for GitHub

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
wants to merge 2 commits into from
Closed

Add requestId information to ErrorAttributes and whitelabel error page in WebFlux #15952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c446b0e
Add logPrefix information to ErrorAttributes and whitelabel error pag…
nosan Feb 14, 2019
9c49795
update license headers
nosan Feb 14, 2019
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
3 changes: 2 additions & 1 deletion ...ringframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,11 @@ protected Mono<ServerResponse> renderDefaultErrorView(
Date timestamp = (Date) error.get("timestamp");
Object message = error.get("message");
Object trace = error.get("trace");
Object logPrefix = error.get("logPrefix");
builder.append("<html><body><h1>Whitelabel Error Page</h1>").append(
"<p>This application has no configured error view, so you are seeing this as a fallback.</p>")
.append("<div id='created'>").append(timestamp).append("</div>")
.append("<div>There was an unexpected error (type=")
.append(logPrefix).append("<div>There was an unexpected error (type=")
.append(htmlEscape(error.get("error"))).append(", status=")
.append(htmlEscape(error.get("status"))).append(").</div>");
if (message != null) {
Expand Down
1 change: 0 additions & 1 deletion ...pringframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ protected Mono<ServerResponse> renderErrorView(ServerRequest request) {
protected Mono<ServerResponse> renderErrorResponse(ServerRequest request) {
boolean includeStackTrace = isIncludeStackTrace(request, MediaType.ALL);
Map<String, Object> error = getErrorAttributes(request, includeStackTrace);
HttpStatus errorStatus = getHttpStatus(error);
return ServerResponse.status(getHttpStatus(error))
.contentType(MediaType.APPLICATION_JSON_UTF8)
.body(BodyInserters.fromObject(error));
Expand Down
111 changes: 65 additions & 46 deletions ...oot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.springframework.boot.autoconfigure.web.reactive.HttpHandlerAutoConfiguration;
import org.springframework.boot.autoconfigure.web.reactive.ReactiveWebServerFactoryAutoConfiguration;
import org.springframework.boot.autoconfigure.web.reactive.WebFluxAutoConfiguration;
import org.springframework.boot.test.context.assertj.AssertableReactiveWebApplicationContext;
import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner;
import org.springframework.boot.testsupport.rule.OutputCapture;
import org.springframework.context.annotation.Configuration;
Expand All @@ -42,6 +43,8 @@
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.server.ResponseStatusException;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.WebFilter;
import org.springframework.web.server.WebFilterChain;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
Expand All @@ -57,6 +60,8 @@ public class DefaultErrorWebExceptionHandlerIntegrationTests {
@Rule
public OutputCapture outputCapture = new OutputCapture();

private final LogIdFilter logIdFilter = new LogIdFilter();

private ReactiveWebApplicationContextRunner contextRunner = new ReactiveWebApplicationContextRunner()
.withConfiguration(AutoConfigurations.of(
ReactiveWebServerFactoryAutoConfiguration.class,
Expand All @@ -71,15 +76,15 @@ public class DefaultErrorWebExceptionHandlerIntegrationTests {
@Test
public void jsonError() {
this.contextRunner.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
client.get().uri("/").exchange().expectStatus()
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR).expectBody()
.jsonPath("status").isEqualTo("500").jsonPath("error")
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR.getReasonPhrase())
.jsonPath("path").isEqualTo(("/")).jsonPath("message")
.isEqualTo("Expected!").jsonPath("exception").doesNotExist()
.jsonPath("trace").doesNotExist();
.jsonPath("trace").doesNotExist().jsonPath("logPrefix")
.isEqualTo(this.logIdFilter.getLogId());
this.outputCapture.expect(Matchers.allOf(
containsString("500 Server Error for HTTP GET \"/\""),
containsString("java.lang.IllegalStateException: Expected!")));
Expand All @@ -89,20 +94,19 @@ public void jsonError() {
@Test
public void notFound() {
this.contextRunner.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
client.get().uri("/notFound").exchange().expectStatus().isNotFound()
.expectBody().jsonPath("status").isEqualTo("404").jsonPath("error")
.isEqualTo(HttpStatus.NOT_FOUND.getReasonPhrase()).jsonPath("path")
.isEqualTo(("/notFound")).jsonPath("exception").doesNotExist();
.isEqualTo(("/notFound")).jsonPath("exception").doesNotExist()
.jsonPath("logPrefix").isEqualTo(this.logIdFilter.getLogId());
});
}

@Test
public void htmlError() {
this.contextRunner.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
String body = client.get().uri("/").accept(MediaType.TEXT_HTML).exchange()
.expectStatus().isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR)
.expectHeader().contentType(MediaType.TEXT_HTML)
Expand All @@ -117,14 +121,14 @@ public void htmlError() {
@Test
public void bindingResultError() {
this.contextRunner.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
client.post().uri("/bind").contentType(MediaType.APPLICATION_JSON)
.syncBody("{}").exchange().expectStatus().isBadRequest().expectBody()
.jsonPath("status").isEqualTo("400").jsonPath("error")
.isEqualTo(HttpStatus.BAD_REQUEST.getReasonPhrase()).jsonPath("path")
.isEqualTo(("/bind")).jsonPath("exception").doesNotExist()
.jsonPath("errors").isArray().jsonPath("message").isNotEmpty();
.jsonPath("errors").isArray().jsonPath("message").isNotEmpty()
.jsonPath("logPrefix").isEqualTo(this.logIdFilter.getLogId());
});
}

Expand All @@ -134,63 +138,62 @@ public void includeStackTraceOnParam() {
.withPropertyValues("server.error.include-exception=true",
"server.error.include-stacktrace=on-trace-param")
.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
client.get().uri("/?trace=true").exchange().expectStatus()
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR).expectBody()
.jsonPath("status").isEqualTo("500").jsonPath("error")
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR.getReasonPhrase())
.jsonPath("exception")
.isEqualTo(IllegalStateException.class.getName())
.jsonPath("trace").exists();
.jsonPath("trace").exists().jsonPath("logPrefix")
.isEqualTo(this.logIdFilter.getLogId());
});
}

@Test
public void alwaysIncludeStackTrace() throws Exception {
public void alwaysIncludeStackTrace() {
this.contextRunner.withPropertyValues("server.error.include-exception=true",
"server.error.include-stacktrace=always").run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
client.get().uri("/?trace=false").exchange().expectStatus()
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR).expectBody()
.jsonPath("status").isEqualTo("500").jsonPath("error")
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR.getReasonPhrase())
.jsonPath("exception")
.isEqualTo(IllegalStateException.class.getName())
.jsonPath("trace").exists();
.jsonPath("trace").exists().jsonPath("logPrefix")
.isEqualTo(this.logIdFilter.getLogId());
});
}

@Test
public void neverIncludeStackTrace() {
this.contextRunner.withPropertyValues("server.error.include-exception=true",
"server.error.include-stacktrace=never").run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
client.get().uri("/?trace=true").exchange().expectStatus()
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR).expectBody()
.jsonPath("status").isEqualTo("500").jsonPath("error")
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR.getReasonPhrase())
.jsonPath("exception")
.isEqualTo(IllegalStateException.class.getName())
.jsonPath("trace").doesNotExist();

.jsonPath("trace").doesNotExist().jsonPath("logPrefix")
.isEqualTo(this.logIdFilter.getLogId());
});
}

@Test
public void statusException() {
this.contextRunner.withPropertyValues("server.error.include-exception=true")
.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
client.get().uri("/badRequest").exchange().expectStatus()
.isBadRequest().expectBody().jsonPath("status")
.isEqualTo("400").jsonPath("error")
.isEqualTo(HttpStatus.BAD_REQUEST.getReasonPhrase())
.jsonPath("exception")
.isEqualTo(ResponseStatusException.class.getName());
.isEqualTo(ResponseStatusException.class.getName())
.jsonPath("logPrefix").isEqualTo(this.logIdFilter.getLogId());
});
}

Expand All @@ -200,14 +203,14 @@ public void defaultErrorView() {
.withPropertyValues("spring.mustache.prefix=classpath:/unknown/",
"server.error.include-stacktrace=always")
.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
String body = client.get().uri("/").accept(MediaType.TEXT_HTML)
.exchange().expectStatus()
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR).expectHeader()
.contentType(MediaType.TEXT_HTML).expectBody(String.class)
.returnResult().getResponseBody();
assertThat(body).contains("Whitelabel Error Page")
.contains(this.logIdFilter.getLogId())
.contains("<div>Expected!</div>").contains(
"<div style='white-space:pre-wrap;'>java.lang.IllegalStateException");
});
Expand All @@ -218,14 +221,14 @@ public void escapeHtmlInDefaultErrorView() {
this.contextRunner
.withPropertyValues("spring.mustache.prefix=classpath:/unknown/")
.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
String body = client.get().uri("/html").accept(MediaType.TEXT_HTML)
.exchange().expectStatus()
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR).expectHeader()
.contentType(MediaType.TEXT_HTML).expectBody(String.class)
.returnResult().getResponseBody();
assertThat(body).contains("Whitelabel Error Page")
.contains(this.logIdFilter.getLogId())
.doesNotContain("<script>").contains("&lt;script&gt;");
});
}
Expand All @@ -235,22 +238,21 @@ public void testExceptionWithNullMessage() {
this.contextRunner
.withPropertyValues("spring.mustache.prefix=classpath:/unknown/")
.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
String body = client.get().uri("/notfound")
.accept(MediaType.TEXT_HTML).exchange().expectStatus()
.isNotFound().expectHeader().contentType(MediaType.TEXT_HTML)
.expectBody(String.class).returnResult().getResponseBody();
assertThat(body).contains("Whitelabel Error Page")
.contains(this.logIdFilter.getLogId())
.contains("type=Not Found, status=404");
});
}

@Test
public void responseCommitted() {
this.contextRunner.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
assertThatExceptionOfType(RuntimeException.class)
.isThrownBy(
() -> client.get().uri("/commit").exchange().expectStatus())
Expand All @@ -263,8 +265,7 @@ public void responseCommitted() {
public void whitelabelDisabled() {
this.contextRunner.withPropertyValues("server.error.whitelabel.enabled=false",
"spring.mustache.prefix=classpath:/unknown/").run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
client.get().uri("/notfound").accept(MediaType.TEXT_HTML).exchange()
.expectStatus().isNotFound().expectBody().isEmpty();
});
Expand All @@ -276,8 +277,7 @@ public void exactStatusTemplateErrorPage() {
.withPropertyValues("server.error.whitelabel.enabled=false",
"spring.mustache.prefix=" + getErrorTemplatesLocation())
.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
String body = client.get().uri("/notfound")
.accept(MediaType.TEXT_HTML).exchange().expectStatus()
.isNotFound().expectBody(String.class).returnResult()
Expand All @@ -292,8 +292,7 @@ public void seriesStatusTemplateErrorPage() {
.withPropertyValues("server.error.whitelabel.enabled=false",
"spring.mustache.prefix=" + getErrorTemplatesLocation())
.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
String body = client.get().uri("/badRequest")
.accept(MediaType.TEXT_HTML).exchange().expectStatus()
.isBadRequest().expectBody(String.class).returnResult()
Expand All @@ -302,21 +301,41 @@ public void seriesStatusTemplateErrorPage() {
});
}

private String getErrorTemplatesLocation() {
String packageName = getClass().getPackage().getName();
return "classpath:/" + packageName.replace('.', '/') + "/templates/";
}

@Test
public void invalidAcceptMediaType() {
this.contextRunner.run((context) -> {
WebTestClient client = WebTestClient.bindToApplicationContext(context)
.build();
WebTestClient client = getWebClient(context);
client.get().uri("/notfound").header("Accept", "v=3.0").exchange()
.expectStatus().isEqualTo(HttpStatus.NOT_FOUND);
});
}

private String getErrorTemplatesLocation() {
String packageName = getClass().getPackage().getName();
return "classpath:/" + packageName.replace('.', '/') + "/templates/";
}

private WebTestClient getWebClient(AssertableReactiveWebApplicationContext context) {
return WebTestClient.bindToApplicationContext(context).webFilter(this.logIdFilter)
.build();
}

private static final class LogIdFilter implements WebFilter {

private String logId;

@Override
public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
this.logId = exchange.getLogPrefix();
return chain.filter(exchange);
}

String getLogId() {
return this.logId;
}

}

@Configuration
public static class Application {

Expand Down
3 changes: 2 additions & 1 deletion ...oot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* Copyright 2012-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -86,6 +86,7 @@ public Map<String, Object> getErrorAttributes(ServerRequest request,
errorAttributes.put("status", errorStatus.value());
errorAttributes.put("error", errorStatus.getReasonPhrase());
errorAttributes.put("message", determineMessage(error));
errorAttributes.put("logPrefix", request.exchange().getLogPrefix());
handleException(errorAttributes, determineException(error), includeStackTrace);
return errorAttributes;
}
Expand Down
12 changes: 11 additions & 1 deletion ...rc/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* Copyright 2012-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -206,6 +206,16 @@ public void includePath() {
assertThat(attributes.get("path")).isEqualTo("/test");
}

@Test
public void includeLogPrefix() {
MockServerHttpRequest request = MockServerHttpRequest.get("/test").build();
ServerRequest serverRequest = buildServerRequest(request, NOT_FOUND);
Map<String, Object> attributes = this.errorAttributes
.getErrorAttributes(serverRequest, false);
assertThat(attributes.get("logPrefix"))
.isEqualTo(serverRequest.exchange().getLogPrefix());
}

@Test
public void extractBindingResultErrors() throws Exception {
Method method = getClass().getMethod("method", String.class);
Expand Down