At the start of Drupal.Nodejs.runCallbacks there's this:

  if (message.clientSocketId == Drupal.Nodejs.socket.sessionid) {
    return;
  }

In my testing both is undefined, resulting in all messages being dropped.

Pursuing the history at https://github.com/Automattic/socket.io-client , it seems that there was a sessionid in 0.9.6, but it has gone in 1.x.

I've created a quick hack by randomly generating an sessionid, but there's a case for renaming the variable and assorted functions to reflect the fact that it's more of a client id than a socket id, but that's not for me to decide.

CommentFileSizeAuthor
#7 Selection_104.png13.38 KBxen
#6 socket_sessionid_has-2288625-6.patch808 bytesxen
sessionid.patch632 bytesxen

Comments

xen’s picture

xen commented
Status: Active » Needs review

And forgot status...

  • slv_ committed 077fbee on 7.x-1.x 
    Issue #2288625 by Xen: socket.sessionid has gone away?
slv_’s picture

slv_ commented
Status: Needs review » Active

It seems this was introduced back in June last year. The id seems to be in "Drupal.Nodejs.socket.id", but the refactor was done to

"Drupal.Nodejs.socket.sessionid", causing the problem you described Xen. Just fixed that and pushed to dev branch:
http://cgit.drupalcode.org/nodejs/commit/?id=077fbee

Does this need to be applied to D8 version? Haven't got an environment set up for that one.

slv_’s picture

slv_ commented
Status: Active » Patch (to be ported)
Anonymous’s picture

Anonymous (not verified) commented

Does this also apply to 'sessionid' at:

Drupal.ajax.prototype.beforeSerialize = function(element_settings, options) {
        options.data['nodejs_client_socket_id'] = Drupal.Nodejs.socket.sessionid;
        return this.nodejsOriginalBeforeSerialize(element_settings, options);
      };
xen’s picture

xen commented
StatusFileSize
new socket_sessionid_has-2288625-6.patch808 bytes

@rhclayto
You're absolutely right. Patch attached.

How it's been working for over a year, but fails after upgrading to the latest dev, I don't know. But the old version looks suspiciously wrong too.

And I'd assume the D8 version needs same patch, but I haven't looked at that.

xen’s picture

xen commented
Status: Patch (to be ported) » Active
StatusFileSize
new Selection_104.png13.38 KB

@slv_
No wonder I can't get it working when what's committed isn't what I wrote.

Fixing the name doesn't work as there *is no* id. See attached screenshot and https://github.com/socketio/socket.io-client/blob/master/lib/socket.js

Secondly, as this causes total failure when the id is undefined and cost me hours in debugging, it's worth considering what to do when message.clientSocketId and Drupal.Nodejs.socket.id is undefined. Just dropping messages is a pain.

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Fixed

thanks for the patch, this is fixed in the latest release.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

xen’s picture

xen commented
Version: 7.x-1.8 » 7.x-1.12
Status: Closed (fixed) » Active

This is still not fixed. There is no id on the socket, as shown in #7.

This causes all callbacks to be ignored in our case.

glekli’s picture

glekli commented
Status: Active » Fixed

I'm not experiencing this issue. Drupal.Nodejs.socket.id returns the id as expected. Is there any chance you may be running an older version of the nodejs backend app that has an older socket.io version?
That said, it is certainly a good idea to check if the variable is set before the comparison, so I added that check.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.