socketio-auth icon indicating copy to clipboard operation
socketio-auth copied to clipboard

Client doesn't know it was timed out

Open ghost opened this issue 8 years ago • 6 comments

I have an Android client application that needs to be able to retry establishing the connection when it was timed out because of the Authentication event not being emitted or not reaching the server.

A common scenario where the timeout would be necessary to be treated by the client is when the network connection is slow.

My quick and a little bit tested "fix" was to add this socket.emit with the custom message Authentication timeout before calling the disconnect.

if (timeout !== 'none') {
    setTimeout(function() {
        // If the socket didn't authenticate after connection, disconnect it
        if (!socket.auth) {
            debug('Disconnecting socket %s', socket.id);
            socket.emit('unauthorized', {message: 'Authentication timeout'});
            socket.disconnect('unauthorized');
        }
    }, timeout);
}

This is my solution to avoid having the client constantly check if the socket is not connected because of a possible authentication timeout.

I would like to read your opinions on the "fix" that i have presented or any ideas to improve it.


It would also be nice to know on the server side, when the socket was disconnected because of the timeout. (I'm refering to socket.on('disconnect' ...)

This could easily be achieved by setting a socket.authTimeout = false; next to the socket.auth = false; and then before the previously mentioned socket.disconnect set socket.authTimeout = true;.

ghost avatar May 26 '16 18:05 ghost

Instead of emitting an unauthorized event in the server, can't you just listen for disconnect in the client and attempt a reconnection then? Haven't tried it, but that doesn't work for you, the unauthorized event seems ok.

Also have you considered increasing the timeout time since you're seeing cases where it is not enough?

facundoolano avatar May 27 '16 12:05 facundoolano

For me to listen for disconnect in the client, i would need to receive some information that could tell me that the socket was disconnected specifically because of an authentication timeout, otherwise every time I disconnected a socket for any other reason it would always attempt a reconnection.

The current information that the client is receiving on disconnect upon being timed out by socketio.auth is io server disconnect which is what the Java Socket.io Clienttells when it receives a disconnect (as it can be seen in the sample code below, taken from Socket.java):

private void ondisconnect() {
        logger.fine(String.format("server disconnect (%s)", this.nsp));
        this.destroy();
        this.onclose("io server disconnect");
    }

Increasing the timeout wouldn't eradicate the problem because at some point someone could get timed out because of a slower connection. So, the best approach is to treat this timeout in the client side.

I'm still experimenting to find a working solution to this problem as the one I presented in the first message is very unstable and doesn't work as intended. Any ideas are welcomed.

Questions:

  1. Is there anything preventing the code inside the config.authenticate(socket, data, function(err, success) { to run at the same time that the timeout finishes? Because like this, the server will eventually do a socket.emit('unauthorized' and then enter the function from the setTimeout.
  2. Why not clear the timeout or add some flags to prevent this situation from happening?

ghost avatar May 31 '16 22:05 ghost

  1. There's nothing preventing it. I don't think it's a big deal because if you initiate your authentication so close to the timeout then you kind of already have a problem, and probably should look at allowing a longer timeout. Then again, since the auth process already disconnects in case of failure, it would work to clear the timeout as soon as the authentication event comes in.
  2. I don't think preventing that scenario is that important, but clearing the timeout is actually a more elegant solution than the if (!socket.auth) currently used, I guess I didn't think of it back when I wrote it.

If you do find a solution that satisfies you, do send a PR. It looks like you're thoroughly evaluating this scenario, so I trust your judgment.

facundoolano avatar Jun 01 '16 14:06 facundoolano

PIPOKID is right. The client should know why it is disconnected.

therewillbecode avatar Mar 12 '17 11:03 therewillbecode

@facundoolano @therewillbecode This was the approach I used in my project to better manage the timeout situation.

I used a "semaphore" type flag (pathTaken) to block the timeout from firing when the callback from the config.authenticate comes back with the answer too close to the timeout time. So, once one of the paths (timeout function or config.authenticate callback) is chosen, it blocks the other one from continuing.

Also, to be able to identify in the server side that a socket was disconnected because of a timeout I decided to add the flag authTimeout.

As for the rest, the comments in the code should be self-explanatory but feel free to ask any questions.

P.S. This code was written many months ago but I forgot to show it here :/

io.on('connection', function (socket) {

    var pathTaken = false;
    var timeoutId;
    socket.auth = false;
    socket.authTimeout = true;

    socket.on('authentication', function (data) {

      if (timeout !== 'none') {
        timeoutId = setTimeout(function () {
          // Check if it already entered the authenticate callback
          if (pathTaken) {
            return;
          }
          // If not, then lock the path
          pathTaken = true;

          // Double check if the socket didn't authenticate after connection and disconnect it
          if (!socket.auth) {
            debug('Authentication timed out %s', socket.id);
            // Warning: The disconnect probably needs to go after the emit because if
            // the client fails to call the callback then the socket is not disconnected.
            // -> I chose to do it this way to give more freedom to the client so it could do
            // some tasks before calling the callback (which disconnects the socket).
            // -> Now that I'm revisiting this, I think the best approach would be to add a new
            // flag which could be set the same way as the timeout time is, so that here we could
            // had a couple of if statements to decide if it should be imediately disconnected
            // after the emit or inside its callback.
            socket.emit('unauthorized', {message: 'Authentication timeout'}, function () {
              socket.disconnect('unauthorized');
            });
          }
        }, timeout);
      }

      config.authenticate(socket, data, function (err, success) {
        // Check if it already entered the timeout
        if (pathTaken) {
          return;
        }
        // If not, then lock the path and clear the timeout
        pathTaken = true;
        clearTimeout(timeoutId);

        // It did not time out so, set the socket flag to false
        socket.authTimeout = false;

        if (success) {
          debug('Authenticated socket %s', socket.id);
          socket.auth = true;

          _.each(io.nsps, function (nsp) {
            restoreConnection(nsp, socket);
          });

          socket.emit('authenticated', success);
          return postAuthenticate(socket, data);
        } else if (err) {
          debug('Authentication error socket %s: %s', socket.id, err.message);
          socket.emit('unauthorized', {message: err.message}, function () {
            socket.disconnect();
          });
        } else {
          debug('Authentication failure socket %s', socket.id);
          socket.emit('unauthorized', {message: 'Authentication failure'}, function () {
            socket.disconnect();
          });
        }

      });

    });

    socket.on('disconnect', function(){
        return disconnect(socket);
    });

  });

ghost avatar Mar 12 '17 23:03 ghost

this looks like something I wouldn't be able to manage when an issue arises.

Is there a simpler solution/recommendation

radiumrasheed avatar Mar 13 '18 19:03 radiumrasheed