Skip to content

Commit

Permalink
Add requestId info to ErrorAttributes in WebFlux
Browse files Browse the repository at this point in the history 
See spring-projectsgh-15952
  • Loading branch information
@nosan @bclozel
nosan authored and bclozel committed Apr 11, 2019
1 parent b33944b commit 2c20d01
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 49 deletions.
3 changes: 2 additions & 1 deletion ...ringframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,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
111 changes: 65 additions & 46 deletions ...oot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java
View file
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(proxyBeanMethods = false)
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
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
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

0 comments on commit 2c20d01

Please sign in to comment.