1

We are upgrading cometd from version 2.5 to 3.0.9 but with websockets disabled. One of the changes we notice is :- org.cometd.server.ServerSessionImpl disconnect() method no longer sets the successful flag on the message, before publishing it to "/meta/disconnect" channel. Noticed from the GitHub cometd repository that it was removed as part of commit on Oct. 14, 2015 - Improved handling of server-side disconnects (user sbordet).

public void disconnect()
{
    boolean connected = _bayeux.removeServerSession(this, false);
    if (connected)
    {
        ServerMessage.Mutable message = _bayeux.newMessage();
        message.setChannel(Channel.META_DISCONNECT);
        // message.setSuccessful(true);
        deliver(this, message);
        flush();
    }
}

Now, on client side, we are using jquery to interface with cometd (jquery.cometd.js). We issue a reconnect whenever we receive a cometd disconnect message from the server side. We check for the below condition before attempting to reconnect.

$.cometd.isDisconnected() && (message.channel == "/meta/disconnect" && message.successful)

message.successful check fails as its never set on the server side due to change to the disconnect API. Hence, the session never gets reconnected / restored, thereby leading the server to not know about this session at all and therefore, server side does not push any server to client side service messages..

We want to retain this check, as during logout, this flag is successfully set.During logout, we call the below client side method which in turn causes the DisconnectHandler on server side (under BayeuxServerImpl) to get invoked. DisconnectHandler message event sets this flag to true on the reply message.

$.cometd.disconnect()

In the first place, want to understand why is the success flag not being set on cometd disconnect message any more, when disconnect is initiated from the server side (would expect it to be consistent with DisconnectHandler behaviour). Secondly, is there a possible alternative to still set this flag i.e. may be through an override either on client or server side??

crathour
  • 60
  • 6

1 Answers1

1

The successful flag was removed from the server-side disconnect message because that is an unsolicited message, not a reply to a client-initiated disconnect, and there was the need to differentiate between the two.

Unsolicited messages don't have message id nor successful fields.

If the server disconnected a client, and you want to reconnect that client, it is enough that you register a listener for the /meta/disconnect channel. For both unsolicited disconnects and for disconnect replies the listener will be invoked and you can re-handshake() if you want.

sbordet
  • 16,856
  • 1
  • 50
  • 45
  • Thanks Simone for your reply..So as I understand, we can have the message.successful check optional. That is, under /meta/disconnect listener, check for this flag only if its available. Hence, the check will continue to happen as is for client initiated disconnects and will not happen for server initiated ones (i.e. unsolicited messages) due to the absence of this flag.. – crathour Sep 30 '16 at 08:43
  • On a separate note, we always observe the below info exception logged onto the browser console after the upgrade by cometd javascript, though this does not impact any functionality. Is this something we should be worried about? `Exception during execution of extension reload Abstract` Our initial observation is that this happens generally when a message belongs to /meta/connect channel and id is "402:Unknown client". May very well be happening in other cases but not sure..Eventually, reconnect logic leads the message to have a valid id and hence do not see any impact. – crathour Sep 30 '16 at 08:57
  • `Exception during execution of extension reload Abstract` is due to the fact that you don't have a cookie library that overrides those "abstract" methods. What do you use, jQuery or Dojo ? – sbordet Sep 30 '16 at 17:19
  • Hi Simone, we are using jQuery. Is there a cookie library that's advised to use? Notice the below in cometd 3.0.9 tar distribution.. cometd-3.0.9-distribution\cometd-3.0.9\cometd-javascript\examples-jquery\src\main\webapp\jquery\jquery.cookie.js Is that the one?? What are the impacts on application, if we start using it, apart from getting rid of the exception? Thanks.. – crathour Oct 03 '16 at 09:06
  • `jquery.cookie.js` is one library that you can use, yes. If you don't use it, the reload extension will not work - and yes you are using the reload extension otherwise that exception would not have been triggered. – sbordet Oct 03 '16 at 20:49
  • Thanks Simone..I tried using the latest version of jquery.cookie (v2.1.3) but looks like jQuery support has been withdrawn now..See the following link :- [https://github.com/js-cookie/js-cookie/releases]. Hence, I've used v1.4.1 which is the latest version having jQuery support. The one inside cometd tar distribution, was about couple of years older than this.I no longer get the extension reload exception. So thanks for that.. Do you have any documentation which I can refer to study the significance of each javascript file in cometd library i.e. what are they being used for? – crathour Oct 13 '16 at 12:40
  • Thanks Simone..I tried using the latest version of jquery.cookie (v2.1.3) but looks like jQuery support has been withdrawn now..See the following link :-[link](https://github.com/js-cookie/js-cookie/releases). Hence, I've used v1.4.1 which is the latest version having jQuery support. The one inside cometd tar distribution, was about couple of years older than this.I no longer get the extension reload exception. So thanks for that.. Do you have any documentation which I can refer to study the significance of each javascript file in cometd library i.e. what are they being used for? – crathour Oct 13 '16 at 12:43
  • The documentation about the reload extension does mention the need of cookie support, and for jQuery the need for a plugin that does that. This will not be required with CometD 3.1.x because the reload extension will now use the `sessionStorage` facility. – sbordet Oct 13 '16 at 22:11