Node.js ws server: global or local functions? [closed]

Posted on

Problem

I’m working on a Node.js chat app project using the ws library. I have it up and running but I’m still making improvements and adding stuff like authentication, etc. Before I get too far, I am wondering what the best functional approach to handling messages (and other events) is. For example, right now I have the following code to handle a message from the client (I cut out most of it, just keeping in one function to use as an example):

wss.on('connection', (ws, req) => {
    // Give them the funtions (uses local ws)
    function handleMessage(msgJSON) {
        let incMsg = {};
        try {
            incMsg = JSON.parse(msgJSON); // message from client
        } catch {
            console.error("Could not parse sent JSON");
            return;
        }
        switch (incMsg.type) {
            case "message":
                 ws.send("Message sent.");
                 // send the message to other clients:
                 wss.clients.forEach(client => {
                      // send message to all open clients but not this client
                      if (client !== ws && client.readyState === WebSocket.OPEN) {
                           client.send(`${client.username}: ${incMsg.message}`);
                      }
                 }
            // some more cases, like for "meta"
        }
    }

    // more functions ...

    ws.on('message', message => {
        handleMessage(message);
    });
}

This strategy takes advantage of having the ws variable inside the inner function, but this makes the wss.on('connection') handler full of a lot of functions. Is it better practice to use this approach or to make a global sort of function that you then pass the client into? My alternate idea looks like this:

function handleMessage(msgJSON, ws) { // accepts the current client
    let incMsg = {};
    try {
        incMsg = JSON.parse(msgJSON); // message from client
    } catch {
        console.error("Could not parse sent JSON");
        return;
    }
    switch (incMsg.type) {
        case "message":
             ws.send("Message sent.");
             // send the message to other clients:
             wss.clients.forEach(client => {
                  // send message to all open clients but not this client
                  if (client !== ws && client.readyState === WebSocket.OPEN) {
                       client.send(`${client.username}: ${incMsg.message}`);
                  }
             }
             break;
        // some more cases, like for "meta" ...
    }
}

// more functions ...

wss.on('connection', (ws, req) => {
    ws.on('message', message => {
        handleMessage(message, ws); // pass along the ws client
    });
}
```

Solution

“Before going too far” – is the tip of the iceberg as I see it 🙂

If it’s just a simple lab, nothing fancy, tutorial for the sake of testing some functionality and everything should remain small and in one file
then ignore the rest of what I’m saying.

Otherwise, if that is a long ongoing project, it should be production-ready and etc.., there are lots of aspects to consider so it still a hard question to answer.

To simplify and I’m sure many would not agree with me.

I consider a single source, a monolith chat application.

Try to disconnect the WebSocket from the business logic and create a clear cut boundary.

Because you want to unit test all parts of the system you should plan your code around testing – so business logic should be seeing as a black box that you can test as unit.

Imagine that from one side of the boundary you are passing a generic message package.
Inside the boundary, the message package is broken and the real action/parameters are extracted and a function is dispatched based on these values to handle the request.

This actually starts forming a request/response flow – that flow should have validation, use case handler, presentation handler along the way.

your action handler should be generic and allow adding more functionality into the communication protocol, so you should use a factory pattern to create action handlers by name.

I hope this helps

Leave a Reply

Your email address will not be published.