Spring WebSocket support with @EnableWebFlux

It seems like I almost never need to do anything with WebSocketHandlerAdapter when using WebFlux and websockets. I just need to register it, like this:

@Bean
SimpleUrlHandlerMapping simpleUrlHandlerMapping(WebSocketHandler wsh) {
    return new SimpleUrlHandlerMapping() {
        {
            setUrlMap(Map.of("/ws/time", wsh));
            setOrder(10);
        }
    };
}

@Bean
WebSocketHandlerAdapter wsha() {
    return new WebSocketHandlerAdapter();
}

@Bean
WebSocketHandler wsh(Timer timer) {
    return session -> session.send(timer.greet().share().map(session::textMessage));
}

I can see why i need to register the WebSocketHandler (that's where my custom websocket logic lives) and I can see why I need to register the SimpleUrlHandlermapping bean (that's where my custom routing lives). But why the WebSocketHandlerAdapter? Could it be auto-configuration registered based on the presence of websockets on the classpath and subject to a @ConditionalOnMissingBean so that users who want to contribute their own can do so? it seems ceremonial.

thanks in advance..

Comment From: philwebb

Could it be auto-configuration registered based on the presence of websockets on the classpath

If we're talking about org.springframework.web.reactive.socket.server.support.WebSocketHandlerAdapter then I'm not sure we have a strong classpath signal that websockets are being used since that class is part of spring-webflux.

@joshlong or @rstoyanchev Do you know if a spring-websocket jar dependency is also needed? Or do reactive websockets work differently?

Comment From: rstoyanchev

No, spring-websocket is not need/used for this but we could probably register WebSocketHandlerAdapter automatically with the @EnableWebFlux setup, and perhaps add some extra convenience methods to SimpleUrlHandlerMapping to improve the above sample. @philwebb feel free to transfer to spring-framework.

Comment From: rstoyanchev

Simply registering WebSocketHandlerAdapter as part of @EnableWebFlux is not a straight forward solution it seems as it can be customized with a WebSocketService that in turn provides access to customizations, via RequestUpgradeStrategy, for server specific properties like buffer sizes, idle session timeout, etc. So we would need some callbacks on WebFluxConfigurer related to WebSocket which means also wrapping the URL mappings. In other words something like @EnableWebSocket but for WebFlux.

Comment From: jhoeller

@rstoyanchev As with #23751, if there are any intentions here, can we consider this for 5.3 RC1 still, or otherwise decide to leave it as-is?

Comment From: eleftherias

It seems the changes that were made to add this enhancement have caused some of our existing tests to fail. Specifically we're seeing the following error

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'webHandler' defined in 
org.springframework.web.reactive.config.DelegatingWebFluxConfiguration: Initialization of bean failed; nested exception is 
org.springframework.beans.factory.BeanCreationException: Error creating bean with name 
'webFluxWebSocketHandlerAdapter' defined in org.springframework.web.reactive.config.DelegatingWebFluxConfiguration: 
Bean instantiation via factory method failed; nested exception is org.springframework.beans.BeanInstantiationException: 
Failed to instantiate [org.springframework.web.reactive.socket.server.support.WebSocketHandlerAdapter]: Factory method 
'webFluxWebSocketHandlerAdapter' threw exception; nested exception is java.lang.IllegalStateException: No suitable default 
RequestUpgradeStrategy found

Here is a sample with a test that reproduces the issue we are seeing https://github.com/eleftherias/web-test-client-error

Comment From: rstoyanchev

@eleftherias thanks for the feedback and for the sample.

The WebTestClient loads the the WebFlux Java config which now includes a WebSocketHandlerAdapter that in turn checks which WebSocket server library is present. I guess in your case with just spring-webflux on the classpath it leads to this failure.

I expect this should not happen at runtime since it has to run on a server and that server should have that dependency, and it probably isn't too bad to expect an application to add libraries for the server they run on to their test classpath. However I'm more concerned that WebSocket is usually a separate dependency and even applications that perform integration tests with a running server might not need to have that dependency otherwise.

So perhaps we can adjust HandshakeWebSocketService to only fail at runtime if a WebSocket handshake does come in. Again I don't expect this to ever be an issue in production so delaying the error could only have potential impact on tests which seems like an acceptable trade-off.