Search code examples
node.jsexpressws

is it ok to pass asynchronous function as Websocket(ws) listerner ? (node.js and ws library)


the most of the actions that my websockets have to do are asycnrhonous, so I'm passing an asynchronous function as callback to the websocket and calling await inside it, is it ok to do so ? if not what alternative do I have ?
EDIT : here is my message listener :

ws.on('message', async function (message) {
        if (message instanceof Buffer) {
          if (currentMode == prot.dataMode) {
            await appendData(sessionData,message);
          }
          else {
            console.log("error : unexpected message received");
          }
        }
        // if the message is for defining the mode of communication
        //This message is a configuration message
        else {
          message = JSON.parse(message);
          if (message[prot.modeField] == prot.confMode) {
            console.log("conf mode");
            currentMode = prot.confMode;
            await configure(sessionData,message);
          }
          //The following messages will be data
          else if (message[prot.modeField] == prot.dataMode) {
            console.log("data mode");
            currentMode = prot.dataMode;
          }

          else{
            console.log("unknown message structure : ");
            console.log(message);
          }
        }

      });

Solution

  • If there is no chance of any of the Promises rejecting, then yes, using an async function as the handler is OK. Your current approach will work.

    But this is unrealistic. The callback handler doesn't expect an async function, and if the Promise generated by it rejects, then you'll get an UnhandledPromiseRejectionWarning - a rejected Promise was created, which wasn't handled anywhere. This is ugly - unhandled Promise rejections are deprecated, and in the future they will crash your process, which you definitely don't want.

    If you use an async function, then make sure to try/catch everything you await inside, because otherwise, if anything awaited rejects, the whole function will return a rejected Promise, resulting in the above problems. For example:

    ws.on('message', async function (message) {
      if (message instanceof Buffer) {
        if (currentMode == prot.dataMode) {
          try {
            await appendData(sessionData,message);
          } catch(e) {
            console.log('Error', e);
          }
        }
        else {
          console.log("error : unexpected message received");
        }
      }
      // if the message is for defining the mode of communication
      //This message is a configuration message
      else {
        message = JSON.parse(message);
        if (message[prot.modeField] == prot.confMode) {
          console.log("conf mode");
          currentMode = prot.confMode;
          try  {
            await configure(sessionData,message);
          } catch(e) {
            console.log('Error', e);
          }
        }
        //The following messages will be data
        else if (message[prot.modeField] == prot.dataMode) {
          console.log("data mode");
          currentMode = prot.dataMode;
        }
    
        else{
          console.log("unknown message structure : ");
          console.log(message);
        }
      }
    });
    

    All that said, while the above code technically fulfills your requirements, it's a bit odd, because you're waiting for Promises, but not doing anything after waiting for them. await is syntax sugar for .then, but if you don't have any logic you need to perform in the .then (such as after an await, or at the consumer of the async function here), it's strange to have await at all, you may as well just .catch the Promises, and leave out the async and try parts entirely, eg:

    if (currentMode == prot.dataMode) {
      appendData(sessionData,message)
        .catch((e) => {
          console.log('Error', e);
        });
    

    If you also want to make sure appendData will only run after the last appendData call is completed, use a Promise queue, perhaps something like:

    const appendDataQueue = [];
    function queueAppend(sessionData, message) {
      appendDataQueue.push({ sessionData, message });
      tryRunNext();
    }
    let active = false;
    function tryRunNext() {
      if (active || !appendDataQueue.length) {
        return;
      }
      const { sessionData, message } = appendDataQueue.unshift();
      active = true;
      appendData(sessionData, message)
        .then(() => {
          active = false;
          tryRunNext();
        })
        .catch((err) => {
          // handle errors
          active = false;
          tryRunNext();
        });
    }
    

    And then, in the .on handler, call queueAppend instead of calling appendData (and, again, no need for an async function):

    if (currentMode == prot.dataMode) {
      queueAppend(sessionData,message);
    }