# Bug fix in chat application

Source: https://tpiros.dev/blog/bug-fix-in-chat-application

I received a bug report from someone who downloaded and installed my node/socket.io chat app, which I've posted about a few times before. The bug showed up in a very specific scenario, and it took some head-scratching to track down:

1. user1 joins the server and creates a room
2. user2 joins the server and joins user1's room
3. user1 disconnects from the server, which destroys the room and kicks everyone out
4. user2 was still able to send messages, which crashed the server

I'm sharing the fix here because others may run into the same thing.

The code is structured so that a `people` object stores information about every connected user. 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 socket identifiers (`socket.id`). From this data structure a few things are clear:

- The `owns` property reveals who owns a room. (The ID is a `node-uuid` generated value created at room creation.)
- The `inroom` property shows which room a user has joined. Comparing `owns` and `inroom` reveals ownership.
- If `inroom` has a value but `owns` is empty, that user isn't a room owner.
- In the example above, Dave owns the room and Adam has joined it.

There's also a `rooms` collection:

```
{
  '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 is the randomly generated `node-uuid`. The owner's ID and all connected people are stored as unique `socket.id` values. (The `peopleLimit`, `status` and `private` variables aren't used yet; they're earmarked for future enhancements.)

The logic for removing users when a room is destroyed gets complicated. A room can only be removed by its owner. If the owner disconnects from the server, leaves the room, or removes the room, different actions need to fire. When a room owner disconnects, the following has to happen:

- Notify all connected users that the room's owner has left and they'll be disconnected
- 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

The last point is the tricky one. The `inroom` property for connected users must be set to `null`, otherwise they can't join another room. Why? Because the `joinRoom` method runs this check:

```js
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.'
  );
}
```

But that wasn't enough. Setting the properties to `null` let users join other rooms from my code's perspective, but from [socket.io](http://socket.io)'s perspective, things were still broken.

On room creation and join, I use [socket.io](http://socket.io)'s [Room](https://github.com/LearnBoost/socket.io/wiki/Rooms) 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.

Here's how that works:

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

This restricts emitted messages to people in the room.

Now we're getting to the actual bug.

The error on the server was (trimmed):

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

This `push` call fires in the `send` method every time a user in a room sends a message. The last 10 messages are stored in the `chatHistory` array.

It fails because `chatHistory[socket.room]` is undefined. All the objects belonging to the room were wiped out when the owner disconnected.

The problem lives in this block inside the `send` function:

```js
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.');
}
```

When user1 creates a room, user2 joins, then user1 disconnects, user2 should be both removed from the server and disconnected from the [socket.io](http://socket.io) room. That disconnection was failing, so the first `if` statement above evaluated to `true`, and the subsequent code tried to reference values that had already been destroyed.

Adding this log statement to the `send` function confirmed it:

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

Even though the room no longer existed, the user was still joined to it.

The fix requires two steps. First, loop through all connected sockets (stored in a `sockets` collection) and check if their IDs match any in the `room.people` array. If there's a match, force that socket to leave the room.

Second, set the `inroom` property to `null` for users who were in that room.

```js
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;
  }
}
```

That solved it. An interesting challenge. I also pulled the above code into a function since it was needed in three different places, which tidied up the logic considerably. The [latest changes](https://github.com/tpiros/advanced-chat/commit/8ce629d78116f1404db375f7919a56159a977678) are on [GitHub](https://github.com/tpiros/advanced-chat).