@SendToUser should provide a single session reply mode [SPR-11506]

[spring-projects-issues]

Mark Galea opened SPR-11506 and commented

Given a user has two tabs open and the client sends a message to the server from tab 1, it is impossible to send a message to a specific client connection (tab 1). Using the @SendToUser("/queue/position-updates") annotation will push the message to both user sessions.

---

Affects: 4.0.3

Reference URL: rstoyanchev/spring-websocket-portfolio#28

Issue Links:

• Not able to send message specific to user using STOMP over WebSocket [SPR-12444] #17049 Not able to send message specific to user using STOMP over WebSocket

Referenced from: commits c50887c, 75c70fa, 9598a1e, 088b80f

3 votes, 5 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

Indeed currently a reply via @SendToUser targets all user sessions. We could limit that to the session of the input message.

[spring-projects-issues]

Mark Galea commented

Hi Rossen, I have worked out a fix:

cloudmark@8f9e159

Can you have a look and see whether it is in line with what you were thinking. If it's ok I'll polish things up and do a pull-request on master.

The solution outline is as follows.

• Add a flag on the @SendToUser annotation. Right now this is a Boolean field onSession but I think an enumeration would be a better fit. Something like replyMode with the following values.

  1. ON_SESSION: Send this message only to a client with the same session web socket identifier as the received message.
  2. BROADCAST: Broadcast this message to all connected clients.

• Add a header simpSessionFilter to the Message and update the SimpMessageHeaderAccessor to provide assessors to this header. This header will be later used by the SimpleBrokerMessageHandler to determine whether the message should pass through the broker or directly go through the client outbound channel.

• Updated the SendToMethodReturnValueHandler to read the flag defined in point 1 and added a post processor SendToUserHeaderPostProcessor to place the simpSessionFilter flag defined in point 2.

Additionally we could also add another reply mode ON_HTTP_SESSION. The semantics of this replyMode are as follows. Send this message to all connected clients sharing the same httpSession. E.g. Imagine a user having two tabs open on two different browsers (4 tabs in total). Each browser will have its own session. Using this annotation the reply would send the message to two of these tabs the ones having the same session. This use case might not be needed though.

Do you think it would be a possibility to push this fix on version 4.0.3? Obviously there is still some polishing up to do.

[spring-projects-issues]

Mark Galea commented

One scenario where this kind of semantics would be extremely useful is in scenarios where the underlying broker does not support heartbeats e.g. RabbitMQ. Using the following annotations the client could still send a message to the server and the server could reply to the web socket session directly simulating a heartbeat.

[spring-projects-issues]

Rossen Stoyanchev commented

Something like @SendToUser(value="/dest1", singleSession=true) perhaps.

The UserDestinationResolver is the one that needs to take notice, the broker should not be aware at all. If you look at DefaultUserDestinationResolver, the parseUserDestination method -- for SimpMessageType.MESSAGE it gets all known session ids. However, if the message has one, it could use it instead. Of course that means SendToMethodReturnValueHandler should add the source message session id only if singleSession=true. Does that make sense?

For the ON_HTTP_SESSION mode, the singleSession=false mode is supposed to target all known user session id's as obtained from the configured UserSessionRegistry. Currently we have only a simple implementation of UserSessionRegistry keeping track of user sessions within one application instance. That should work fine. For environments with multiple application instances we could create a Redis-backed UserSessionRegistry implementation in which case you'd be able to target user sessions regardless of which application instance they're connected to.

By the way RabbitMQ stomp plugin does have heartbeart support so there is no need to simulate it.

[spring-projects-issues]

Rossen Stoyanchev commented

By the way do submit a PR. Implementation details can be discussed there and the PR resubmitted if necessary.

[spring-projects-issues]

Mark Galea commented

Great, I'll open a PR. Regarding the heart beat simulation, you are right RabbitMQ does support the heartbeat however it is currently not supported with sockjs so we are trying to emulate it. Basically we want to be aware of any session disconnections on the client side so that we can re-connect. It would be great if you could share any alternatives on how to achieve this.

[spring-projects-issues]

Rossen Stoyanchev commented

RabbitMQ does support the heartbeat however it is currently not supported with sockjs

All that you need is the RabbitMQ STOMP plugin, i.e. you don't need Rabbit's Web STOMP plugin because you're not connecting directly to Rabbit from the browser and therefore Rabbit's SockJS support is not used here. Instead clients connect to the Spring application via STOMP over SockJS and in turn the Spring application connects to Rabbit and relays messages back and forth. In effect all that Rabbit sees are STOMP over TCP connections from the Spring application.

Have you tried it? It works.

[spring-projects-issues]

Rossen Stoyanchev commented

You can enable TRACE logging for org.springframework.messaging.simp.stomp by the way and you'll see the heartbeats going to and from Rabbit.

[spring-projects-issues]

Mark Galea commented

Thanks for your clarification. Just some clarification on my comment. We encountered an issue when trying to detect when a user has lost connection to the server. Currently there is no way in sockjs.js through stomp.js to receive a heartbeat event in the application code. The idea was to check periodically for a heartbeat event and if this is not received a disconnection is assumed. Since this message was not available I wrongfully assumed that there was something wrong with the heartbeat (some format discrepancy). Looking into the ponger snippet it is now clear that this had nothing to do with the heartbeat backend code, no messages are in fact dispatched.

return this.ponger = Stomp.setInterval(ttl, function() {
  var delta;
  delta = now() - _this.serverActivity;
  if (delta > ttl * 2) {
    if (typeof _this.debug === "function") {
      _this.debug("did not receive server activity for the last " + delta + "ms");
    }
    return _this.ws.close();
  }
});

In order to detect when the connection is lost we are now registering an errorCallback as follows:

var socket = new SockJS('http://localhost:15100/platform/comet',null,{});
var sc = Stomp.over(socket);
sc.connect({}, function (frame) {
    console.log('Connected ' + frame);
}, function (error) {
    console.error(error);
});

and listen for an error object with the message "Connection to broker closed". A sample of the message follows. In retrospect this makes more sense than what we initially had in mind.

body: ""
command: "ERROR"
headers: Object
content-length: "0"
message: "Connection to broker closed"

Sorry if my comment was misleading.

[spring-projects-issues]

Rossen Stoyanchev commented

Yes it should work and it should be transparent. Simply the connection should simply automatically be closed when the server misses a heartbeat. You could also register an onclose or onerror callbacks on the (SockJS) socket.

[spring-projects-issues]

Mark Galea commented

The UserDestinationResolver is the one that needs to take notice, the broker should not be aware at all. If you look at DefaultUserDestinationResolver, the parseUserDestination method – for SimpMessageType.MESSAGE it gets all known session ids. However, if the message has one, it could use it instead. Of course that means SendToMethodReturnValueHandler should add the source message session id only if singleSession=true. Does that make sense?

Makes sense, actually this was simpler than my initial approach! Thanks! I submitted a pull request here #487

[spring-projects-issues]

Mark Galea commented

Rossen Stoyanchev do you think it would be possible to incorporate this fix in the 4.0.3 artifact?