spring-boot icon indicating copy to clipboard operation
spring-boot copied to clipboard

When a WebFlux-based management server is running on a separate port, the error infrastructure is not used

Open wilkinsona opened this issue 4 years ago • 3 comments

With the main server on 8080 and the management server on 8081, the error responses are different:

$ http :8080/404
HTTP/1.1 404 Not Found
Content-Length: 132
Content-Type: application/json

{
    "error": "Not Found",
    "message": null,
    "path": "/404",
    "requestId": "e06c56d2-1",
    "status": 404,
    "timestamp": "2021-07-27T09:49:12.467+00:00"
}

$ http :8081/404
HTTP/1.1 404 Not Found
content-length: 0

wilkinsona avatar Jul 27 '21 09:07 wilkinsona

I wonder if this may be a Framework bug. We use WebHttpHandlerBuilder.applicationContext to create the HttpHandler bean in the child context. Its javadoc states that it will "create a new builder instance by detecting beans in an ApplicationContext". There's no mention of how a context hierarchy is handled so it's not clear if beans in a parent context should be detected. Currently, they are not. I think this is the root cause of the problem.

We could inject the ErrorWebExceptionHandler and apply it to the builder if it's available:

@Bean
public HttpHandler httpHandler(ApplicationContext applicationContext, ManagementServerProperties properties,
        ObjectProvider<ErrorWebExceptionHandler> errorWebExceptionHandler) {
    WebHttpHandlerBuilder builder = WebHttpHandlerBuilder.applicationContext(applicationContext);
    errorWebExceptionHandler.ifUnique(builder::exceptionHandler);
    HttpHandler httpHandler = builder.build();
    …
}

If something's already defined the ErrorWebExceptionHandler in the child context or Framework's behaviour changes and it starts considering a hierarchy, this could result in the same handler being registered twice. This would be benign most (all?) of the time, but it feels rather clunky.

Flagging for team attention to see how we should proceed. @rstoyanchev, your input on the behaviour of WebHttpHandlerBuilder.applicationContext would be useful too please.

wilkinsona avatar Aug 11 '21 09:08 wilkinsona

Flagging for team attention to see how we should proceed.

Seems like for: team-meeting label is missing here.

vpavic avatar Aug 12 '21 20:08 vpavic

After looking at this in the context of #31811, it seems that we missed something here.

Spring WebFlux, with its WebHttpHandlerBuilder does resolve infrastructure beans from the context using:

List<WebFilter> webFilters = context.getBeanProvider(WebFilter.class).orderedStream().collect(Collectors.toList());

This does resolve beans from the parent application context - this explains why WebFilter (including the metrics one) are applied on the management context and metrics are collected for actuator endpoints. On the other hand, Spring Boot's ErrorWebExceptionHandler should then be detected and applied to the management port.

It seems that ErrorWebExceptionHandler is indeed resolved from the parent context, but that the ordering is wrong: the ErrorWebExceptionHandler is resolved after the WebFluxResponseStatusExceptionHandler, whereas the order is reverse in the main application context. This ordering issues causes the ErrorWebExceptionHandler to be shadowed by the WebFlux one, so it's never been invoked.

I've created spring-projects/spring-framework#29105 to address this inconsistency in the core container. Once that's fixed, error handling should be applied to actuator endpoints in WebFlux.

bclozel avatar Sep 08 '22 08:09 bclozel

Confirmed, since Spring Framework 5.3.23 (so Spring Boot 2.6.12 and 2.7.4), the error infrastructure is now used with WebFlux on the management context.

HTTP/1.1 404 Not Found
Content-Length: 7112
Content-Type: application/json

{
    "error": "Not Found",
    "message": null,
    "path": "/",
    "requestId": "b16ef8c3-7",
    "status": 404,
    "timestamp": "2022-11-07T08:56:05.730+00:00",
    "trace": "..."
}

I'm closing this issue as superseded by spring-projects/spring-framework#29105

bclozel avatar Nov 07 '22 09:11 bclozel