Bug fix in chat application

I received a bug report from someone who has downloaded & installed my node/socket.io based chat app that I have posted about a fewtimesbefore. The bug occured in a very particular scenario and I had some trouble getting my head around it. The scenario was the following:

1. user1 joins the server and creates a room
2. user2 joins the server and joins the room created by user1
3. user1 then disconnects from the server which destroys the room and kicks out everyone from the room
4. user2 was able to carry on sending messages and this has crashed the server

The reason why I'm sharing the resolution on my blog is that I believe some people may benefit from this as well.

The code is structed in a way that a people object is created that stores information about every user that connects to the server. Here's an example:

{
Pdd0QA1bYnH2nJZQPT7c:
    { name: 'Dave',
      owns: '905c3f49-d7d1-4403-aa8a-20d71f92600c',
      inroom: '905c3f49-d7d1-4403-aa8a-20d71f92600c',
      device: 'desktop',
      country: 'gb' },
RhszumhtYGTpy5uwPT7d:
  { name: 'Adam',
    owns: null,
    inroom: '905c3f49-d7d1-4403-aa8a-20d71f92600c',
    device: 'desktop',
    country: 'it' }
}

The keys are the socket identifiers (socket.id). From this data structure I can conclude a few things:

• I can see who owns a room by checking the owns property. (the ID that you see there is a node-uuid generated value that happens at room creation
• I can also see which room a particular user has joined. By comparing the owns and inroom properties, I can also easily deduct the ownership.
• Furthermore if the inroom property has a value but the owns property is empty it's safe to say that the particular user is not a room owner.
• In the example above, Dave is the room owner and Adam has joined the room Dave created.

I also place information into collection called rooms:

{
  '7fe96889-e898-4935-b87f-22d5c0338b0e':
    { name: 'FormulaOne',
      id: '7fe96889-e898-4935-b87f-22d5c0338b0e',
      owner: 'KWusKywHePS1JVdtRqKd',
      people: [ 'KWusKywHePS1JVdtRqKd', 'LlDMvpHsjHnLkyknRqKe' ],
      peopleLimit: 4,
      status: 'available',
      private: false }
  }

The main key on this collection is the previously mentioned randomly generated node-uuid. I also place the owner's id as well as all the people connected to the room (all of these values are again unique IDs coming from socket.id. (The code is currently not using the peopleLimit, status and private variables - those are for future enhancements.

There is quite a complex logic when it comes to capturing and handling how to remove users when a room is removed. First of all a room can only be removed by its owner. Then, if the room's owner decides to disconnect from the server or from the room or remove the room itself there are different kind of actions that need to be initiated. If a room owner disconnects, the following has to be done:

• Send out an advise to all connected users stating that the room's owner has left the server therefore they will be disconnected from the room
• Remove the user from the people collection
• delete the room from the room collection
• delete the chat history belonging to the room
• disconnect all users from the room

Everything is pretty straight forward here. The last point is a bit tricky. I have to set the inroom property to the connected users to be null because if I don't do that they won't be able to connect to a room. Why? Because on the joinRoom method I do the following check:

if (people[socket.id].inroom !== null) {
  socket.emit(
    'update',
    'You are already in a room (' +
      rooms[people[socket.id].inroom].name +
      '), please leave it first to join another room.'
  );
}

This is unfortunately not enough. As far as my code was concerned, setting the properties to null was satisfactiry since it let users join other rooms, but from a socket.io perspective it was still incorrect.

Upon room creation and join I utilise socket.io's Room functionality.

Rooms allow simple partitioning of the connected clients. This allows events to be emitted to subsets of the connected client list, and gives a simple method of managing them.

The following snippet shows how this works:

socket.room = room.name;
socket.join(socket.room);

This allows the emmited messages to be only seem by people who are in a room.

We are getting closer to fixing the bug I promise.

The error message that appeared on the server was the following (trimmed output):

chatHistory[socket.room].push(people[soc
ket.id].name + ": " + msg);
TypeError: Cannot call method 'push' of undefined

This particular push statment is called at the send method - everytime a user who is in a room sends a message - the messages are also stored in the chatHistory array (at least the last 10 messages are stored.)

But of course this method fails as chatHistory[socket.room] fails as the objects belonging to the room have all been wiped out when the owner has left the server/disconnected from the room or removed the room (s)he created.

So the issue is in this block inside the send function:

if (
  io.sockets.manager.roomClients[socket.id]['/' + socket.room] !== undefined
) {
  io.sockets.in(socket.room).emit('chat', people[socket.id], msg);
  socket.emit('isTyping', false);
  if (_.size(chatHistory[socket.room]) > 10) {
    chatHistory[socket.room].splice(0, 1);
  } else {
    chatHistory[socket.room].push(people[socket.id].name + ': ' + msg);
  }
} else {
  socket.emit('update', 'Please connect to a room.');
}

Going back to the example where user1 creates a room, user2 joins it then user1 disconnects from the server user2 should be not only removed from the server but also disconnected from the socket.io room. For whatever reason that fails henceforth the first if statement above evaluates to true and later on causes problems as the rest of the code is referencing a value that has been removed when user1 has left the server.

To see this in action I added the following log statement to the send function:

console.log(io.sockets.manager.roomClients[socket.id]);

It has revealed that even though the room no longer exists the user is still joined to it.

The solution? Has to be done in two steps. First, loop through all the connected sockets (stored in a sockets collection), and check if its IDs match any of the IDs in the room.people array. If there's a match, force that socket connection to leave the room.

Second, 'nullify' the inroom property for the users who were in that room.

var socketids = [];
for (var i = 0; i < sockets.length; i++) {
  socketids.push(sockets[i].id);
  if ((_.contains(socketids), room.people)) {
    sockets[i].leave(room.name);
  }
}

if ((_.contains(room.people), s.id)) {
  for (var i = 0; i < room.people.length; i++) {
    people[room.people[i]].inroom = null;
  }
}

This has finally solved the problem. It was quite an interesting challenge. I also made a massive change in my code. The above code had to be reused in three different places so I wrote a funciton instead and that helped me clean up the logic a bit. I have commited the latest changes to GitHub as well of course.

On a side note - I am going to place a working demo of this chat application online to AppFog but I'm currently they do not support socket.io or WebSockets but according to their Support team, it's coming soon. In a next post I will also give you a walkthrough on how to deploy an app using AppFog. Ciao!