Skip to content

Allow early decoration of request and response in WebFlux #25633

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

Closed
fred84 opened this issue Aug 23, 2020 · 2 comments
Closed

Allow early decoration of request and response in WebFlux #25633

fred84 opened this issue Aug 23, 2020 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@fred84
Copy link
Contributor

fred84 commented Aug 23, 2020

Current behaviour

WebHttpHandlerBuilder.build() creates instance of HttpWebHandlerAdapter (https://github.com/spring-projects/spring-framework/blob/master/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java#L356).

However, HttpWebHandlerAdapter allows to override some of it's methods (e.g. createExchange).

Proposed behaviour

Supply HttpWebHandlerAdapterFactory to WebHttpHandlerBuilder.build and create HttpWebHandlerAdapter via this factory. Naive approach could be as simple as:

    public HttpHandler build() {

        WebHandler decorated = new FilteringWebHandler(this.webHandler, this.filters);
        decorated = new ExceptionHandlingWebHandler(decorated,  this.exceptionHandlers);

        HttpWebHandlerAdapter adapted = httpHandlerCreator.apply(decorated);
        ...
        return adapted;
    }

Motivation

Sometimes we need to decorate ServerHttpResponse and ServerHttpRequest (e.g. body logging/caching). Doing this inside a WebFilter is not sufficient in case of thrown exception flow. We have to do it "earlier" than ExceptionHandlingWebHandler

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 23, 2020
@rstoyanchev
Copy link
Contributor

It's true HttpWebHandlerAdapter has a couple of protected methods although probably more an oversight than intent due to the lack of Javadoc or an option to customize via WebHttpHandlerBuilder.

Rather than exposing all of HttpWebHandlerAdapter in this way we could add a couple of methods that can decorate the request and/or the response which would be invoked before the exchange is created. Another option is an HttpHandler decorator. That would effectively decorate the HttpWebHandlerAdapter itself. I like this latter option.

@rstoyanchev rstoyanchev self-assigned this Aug 24, 2020
@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Aug 24, 2020
@rstoyanchev rstoyanchev added this to the 5.3 RC1 milestone Aug 24, 2020
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 24, 2020
@rstoyanchev rstoyanchev changed the title Allow to create subclasses of HttpWebHandlerAdapter in WebHttpHandlerBuilder Allow early decoration of request and response in WebFlux Aug 26, 2020
@sp00m
Copy link
Contributor

sp00m commented Jan 31, 2021

We were facing the same issue on qudini/qudini-reactive#40: we needed to populate the reactive context "around" the ExceptionHandlingWebHandler (i.e. not via WebFilter), so that uncaught exceptions still have a debugging context when logged (which is quite convoluted to do given the manual instantiation of HttpWebHandlerAdapter).

Do you think we could have an additional webHandlerDecorator, the same way we now have this new httpHandlerDecorator? This would hook just after decorated = new ExceptionHandlingWebHandler(decorated, this.exceptionHandlers);, and would allow us to deal with the enhanced ServerWebExchange instead of the ServerHttpRequest/ServerHttpResponse pair.

Also, do you think the static builder WebHttpHandlerBuilder#applicationContext(ApplicationContext) could initialise those decorators by looking for specific beans in the context somehow? I guess custom interfaces will be needed (possibly inheriting Function<HttpHandler, HttpHandler>/Function<WebHandler, WebHandler> for backward compatibility).

I'm happy to contribute if you agree with the above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants