STOMP SEND should not map to @SubscribeMapping methods [SPR-16109]

[spring-projects-issues]

Matthew opened SPR-16109 and commented

The issue is:

A WebSocket request-response pattern using @SubscribeMapping does not currently work because of a null check.

The null check in question is from the handleReturnValue method in SubscriptionMethodReturnValueHandler:

if (subscriptionId == null) {
    throw new IllegalStateException("No simpSubscriptionId in " + message +
            " returned by: " + returnType.getMethod());
}

The steps to reproduce are:

Annotate a @Controller method with @SubscribeMapping in an application using Spring Boot's @EnableWebSocketMessageBroker, with a simple message broker configured.

Something like the following should reproduce the bug:

@Configuration
@EnableWebSocketMessageBroker
public class WebSocketSupport extends AbstractWebSocketMessageBrokerConfigurer {

    @Override
    public void registerStompEndpoints(StompEndpointRegistry registry) {
        registry.addEndpoint("/endpoint")
                .setAllowedOrigins("*")
                .withSockJS()
        ;
    }
    @Override
    public void configureMessageBroker(MessageBrokerRegistry config) {
        config
                .setApplicationDestinationPrefixes("/app")
                .enableSimpleBroker("/topic", "/queue")
        ;
    }

    @Controller
    public class SocketMessageController {

        @SubscribeMapping("/ping")
        public SocketMessage handle(SocketMessage socketMessage) {
            return Service.command(socketMessage);
        }
    }
}

(According to the docs, the @SubscribeMapping should cause the return value to be sent back to the calling client.)

Then, call the endpoint using STOMP over WebSockets.

What happens is:

When the SubscriptionMethodReturnValueHandler is called, the null check for the simpSubscriptionId header prevents sending the response message back to the client.

A workaround is:

Add an identically named class SubscriptionMethodReturnValueHandler in an identically named package org.springframework.messaging.simp.annotation.support which does not perform this null check.
The socket message responses are sent to the calling client, (and only the calling client.)

---

Affects: 5.0.1

Issue Links:

• Introduce null-safety of Spring Framework API [SPR-15540] #20099 Introduce null-safety of Spring Framework API

Referenced from: commits 64bc9b4

[spring-projects-issues]

Juergen Hoeller commented

As far as I can tell, that subscription id check was always there... It's the destination check that got introduced in that commit. Could you please clarify which part is causing issues for you? If it is the subscription id check, are you actually seeing a regression, i.e. has it ever worked before?

[spring-projects-issues]

Matthew commented

My mistake, the check was always there so this is unrelated to #20099. No regression.

The null check does seem to be preventing the controller's return value from being sent back to the calling client when using @SubscribeMapping. At least, I have been unable to see the following behvaviour described in the SubscribeMapping JavaDoc:

... if the method is not annotated with SendTo or SendToUser, the message is sent directly back to the connected user and does not pass through the message broker. This is useful for implementing a request-reply pattern.

[spring-projects-issues]

Rossen Stoyanchev commented

The subscription id is essential for returning a message that the STOMP client can correctly delegate to the right subscription callback. It should present after all on the incoming subscription message that's being handled unless an interceptor happens to removing it but that seems unlikely.

What version of Spring Boot are you trying this with? Also what client are you using to send the message? Check that the incoming subscribe message has an id header? If you set logging for org.springframework.messaging to TRACE you'll see the message details.

[spring-projects-issues]

Matthew commented

Spring Boot version: 2.0.0.M5
STOMP client: https://registry.npmjs.org/stompjs/-/stompjs-2.3.3.tgz
There is no SUBSCRIBE message involved in the example.
Setting TRACE logging shows o.s.w.s.m.StompSubProtocolHandler : No STOMP "subscription" header in GenericMessage ....

About the lack of a SUBSCRIBE frame (and no subscription id); the docs seem to suggest that the @SubscribeMapping can be used on a @Controller to cause the controller return value to be sent back as a MESSAGE frame to the calling client, and this seems to work as expected when the null check is removed.
The JavaDoc for @SubscribeMapping says:

The return value also follows the same rules as for @MessageMapping, except if the method is not annotated with SendTo or SendToUser, the message is sent directly back to the connected user and does not pass through the message broker. This is useful for implementing a request-reply pattern.

I see what you mean, though. The STOMP spec mandates a MESSAGE frame to contain a subscription id, but I can't work out how @SubscribeMapping can be used on to implement a request-reply pattern, even if a SUBSCRIBE frame is sent before sending the request.

[spring-projects-issues]

Rossen Stoyanchev commented

There is no SUBSCRIBE message involved in the example.

That makes no sense. I don't see how the message is even getting routed to the @SubscribeMapping method then. The Javadoc for @SubscribeMapping:

* Annotation for mapping subscription messages onto specific handler methods based
* on the destination of a subscription. Supported with STOMP over WebSocket only
* (e.g. STOMP SUBSCRIBE frame).

The STOMP spec mandates a MESSAGE frame to contain a subscription id, but I can't work out how @SubscribeMapping can be used on to implement a request-reply pattern...

Yes, in STOMP you cannot receive a message without a subscription, and each message is associated with a subscription. For a request-reply you subscribe as usual and receive only one message, that's it. Here is an example client code that loads portfolio positions on startup, client and server.

I guess I would have to see an example of your client-side code. Could you prepare a small sample?

[spring-projects-issues]

Matthew commented

I don't see how the message is even getting routed to the @SubscribeMapping method then.

Here are the frames, taken from the client side dev tools.

{{["CONNECT\naccept-version:1.1,1.0\nheart-beat:10000,10000\n\n\u0000"]

a["CONNECTED\nversion:1.1\nheart-beat:0,0\n\n\u0000"]

["SEND\ndestination:/app/greeting\ncontent-length:70\n\n{ ... }}\u0000"]

a["MESSAGE\ndestination:/app/greeting\ncontent-type:application/json;charset=UTF-8\nmessage-id:pi1g4f0d-1\ncontent-length:347\n\n{ ... }\u0000"]}}

I guess I would have to see an example of your client-side code. Could you prepare a small sample?
Using server code similar to what was raised in the ticket, I see these frames using client code similar to the following:

import Stomp from 'stompjs';
import SockJS from 'sockjs-client';
const socket = new SockJS(this.state.sockJSUrl);
const stomp = Stomp.over(socket);
stomp.connect(
          {},
          frame => { stomp.send("/app/ping", {}, null); }
);

I find that the SubscribeMapping controller method is invoked when the SEND frame is sent, and the return value is converted into the MESSAGE frame.
That seems extremely handy, and it would be tempting to leave the workaround in if it wasn't the case that a MESSAGE cannot be received without a subscription.

I will look for another way to implement request-response involving a payload on both the request and the response. The samples you linked to do not contain a payload on the request, but hopefully I can work it out.

[spring-projects-issues]

Rossen Stoyanchev commented

if it wasn't the case that a MESSAGE cannot be received without a subscription

STOMP is a pub-sub messaging protocol. You subscribe and receive messages, that's the whole point. You can however implement different patterns including request-reply.

The samples you linked to do not contain a payload on the request, but hopefully I can work it out.

A body is [only allowed\https://stomp.github.io/stomp-specification-1.2.html#Body] SEND, MESSAGE, and ERROR. I highly recommend reading the STOMP protocol. You'll need more than 10-15 min but not more than an hour so it's quite easy to do. It's worth getting that mental model.

[spring-projects-issues]

Matthew commented

Check that the incoming subscribe message has an id header
There is no SUBSCRIBE message involved in the example.
That makes no sense. I don't see how the message is even getting routed to the @SubscribeMapping method then.

A STOMP MESSAGE sent to a SubscribeMapping invokes the handler method, and then throws an exception when there is a return value on that method.

[spring-projects-issues]

Rossen Stoyanchev commented

Modified title (was: "An unwanted null check prevents sending a a socket message response to the requesting client.")