Require a method to update inner ioSocket object for connect / disconnect

[davisford]

In my app, I have both traditional http and websockets -- both are protected by JSON web token authentication. When a user logs in (i.e. post to http endpoint), a token is returned, and then the client will attempt to connect to the websocket endpoint with the token. The connection is rejected if the token isn't valid.

When a user logs out, the websocket is disconnected. I don't want to leave that channel open anymore b/c the user is no longer authenticated. Likewise, I don't want to re-use that same websocket if a new user logs in, b/c the authentication token will be different (and not valid for the new user).

What I'm left with is something like this:

angular.module('app.security')
   .factory('Socket', ['socketFactory', '$rootScope', function (socketFactory, $rootScope) {

      var socket, token = /* get token */

      function connect(token) {
         socket = io.connect('', { query: 'token=' + token, forceNew: true });
         socket.on('connect', function () { /* etc. */ });
         // other event handling
      }

      // connect if we have a token
      if (token) { connect(token); }

      $rootScope.on('login', function ( ) {
          token = /* get token */
          connect(token);
      });

      $rootScope.on('logout', function () { socket.disconnect(); })

      return socketFactory({ ioSocket: socket });
}]);

..and this is an example of how it is in injected into a controller to be used...

angular.module('app.something')
   .controller('FooCtrl', ['Socket', '$scope', function (Socket, $scope) {
      Socket.forward('time:request', $scope);
      $scope.$on('socket:time:response', function (evt, data) {
         console.log('the current time is ', data);
      });
      Socket.emit('time:request');
}]);

On login, the websocket is connected. On logout it is disconnected, and at the end, it is returning the socketFactory wrapper.

The problem here is that socketFactory.$get is only ever called once in the provider, which means that the very first time Socket is injected into a controller it is valid and connected. But if a user logs out and logs in again, angular re-injects the same instance into a controller, and the internal ioSocket has since been disconnected and is no longer good. (note the use of socket.io forceNew option which abandons the socket wholesale and initiates a brand new connection).

What is needed is something like the angular service recipe pattern which news up the object each time, so we don't re-use the same instance of the wrapper...either that or else an API exposed on socketFactory itself that allows one to replace the inner ioSocket reference.

Any ideas on the best way to accomplish this?

[davisford]

Here's a quick hack that fixes this, by exposing a setter on socketFactory:

// socket.js

var wrappedSocket = {
   on: addListener,
   addListener: addListener,
   once: addOnceListener,
   socket: function (s) {
     if (s) { socket = s; }
     return socket;
   },
   // etc.

And the factory above re-worked as such:

  var socket, token = /* get token */ 
  var wrapper = socketFactory({ ioSocket: socket });

  function connect (token) {
    socket = io.connect('', { query: 'token=' + token, forceNew: true });
    socket.on('connect', function () {
       wrapper.socket(socket); // replace with new socket instance
    }
  }

  // other code
  return wrapper;

So, on a new connect event, I'm just replacing the socketFactory's inner socket reference. I just tested this out and it resolves the issue, but if there's a cleaner way, I'd like to hear it?

[nchudleigh]

Beauty, this is exactly what I am looking for. I'll let you know if I come up with any efficiency/cleanliness changes.

[ronaldjeremy]

this doesn't seem to work, apparently because the forwarding events aren't getting rebound to the socket? In socket.js, the "forward: function(events, scope)" binds events to the socket so a controller can receive them. If you are just replacing the inner socket reference, those bindings get thrown away...

[davisford]

@ronaldjeremy you are right, it isn't robust. i have run into cases where the the old socket is discarded, and controllers try to forward and they attach to the old discarded socket, then a new socket comes in and you lost the events.

i have forked this repo and resolved these issues myself. it didn't appear that brian was super receptive to PRs, and i had to get things done and move forward, so i have a branch here that i am using and it is in production for several months now with no issues. hope it helps.

[btford]

it didn't appear that brian was super receptive to PRs

I'm receptive to PRs, but I don't have enough time to maintain this library these days. @davisford (and others) – if you're interested I'd be happy to give you push access to this repo if you're interested in helping me maintain it.

[davisford]

Thanks @btford -- I didn't really mean that as a dig -- poor choice of words. I did send a PR earlier that you dinged for not having a test, and I'm ok with that -- I appreciate you putting this repo out there with an open license, so I don't want to make it seem like I'm complaining.

Right now, I'm trying to bootstrap a startup, so my time is pretty limited too; if things change, I'd be happy to try to help maintain.

[coommark]

Hi @davisford! Your branch works form me, except it connects twice to the server! In the debug is says "Forward has 2 deffered calls b/c socket was not ready yet"

I tripple checked thar the connection function is actually only called once. Pls is this the default behavior? What help can you offer? Because now socket.on('disconnect' on the server is getting called twice.

[davisford]

@coommark is your code on github or somewhere i can take a look at it? also, what branch are you using of the fork? calls made by the controller will be deferred until the socket is ready. that was the goal of the change so that message is not unusual nor does it warrant a problem if you are disconnecting and connecting on a regular basis. the problem initially was that on a disconnect -> reconnect the socket was not ready and clients were trying to use it -- so calls to socket.forward are queued up and executed after the connection is stable. i've been using it in prod for several months now w/o issue, but i think @mritzco sent a revised pr that does similar with tests -- you could try applying that.

[coommark]

Hi, here is the factory responsible for the connection:

''' app.factory('realtimeService', ['$rootScope', 'socketFactory', 'localStorageService', function($rootScope, socketFactory, localStorageService) { var socket;

var wrapper = socketFactory({ioSocket : socket})

$rootScope.$on('authenticated', function() { alert('This is called!') var authData = localStorageService.get('authorizationData'); if (authData) { var token = authData.token; socket = io.connect('', { query: "Bearer=" + token, forceNew : true }); socket.on('connect', function(){ alert("the fuck!") wrapper.socket(socket); }) } });

return wrapper; }]);

'''

[coommark]

Kindly disregard the second alert. Creeped into the code after two sleepless nights.

[davisford]

@coommark i think we should take this off this thread. can you post a gist of your code gist.github.com

on a superficial look it looks like you are doing too much in the controller, which will probably get re-initialized on any view change (depends on how you setup angular app, but that is standard way to do things). i think you've got a few things backwards, but it is hard to say without more codebase. post a gist with a few files for me to look at or point me to an open repo, and i'll try to assist.

[coommark]

Thank you. Been waiting all day. I will do just that. You're very kind.

[coommark]

So I stripped down the project so that I can git it. When I ran the stripped down version, voila! Connection established only once. Thank you @davisford! I am sure it is a conrib angular module causing the factory to run twice.

@btford is there a chance you'll merge @davisford's pull? Because it's awesome to be able to support authentication right out of the box. Thank you guys!

[davisford]

I did not send a pull request for him to merge, but @mritzco did. Glad you got it worked out.

[sam2x]

@davisford @btford , has @mritzco PR been merged to the project ? And has @mritzco PR solved the forward rebound issue ? thx for answer guys.

[mritzco]

@sam2x Not merged as far as I know, I'm checking the code and the forward event is untouched by my code (socket-deferred line 86), the reason was that the forward function actually belongs to the wrapper not the socket.(line 79)

There's a test with forward that's working fine (socket-deferred.spec.js line 226) If you could send me a test showing the problem I think it wouldn't be hard to fix just queue all the forward events and call wrappedSocket.forward on 'bootstrap' event