mqttjs / async-mqtt

Promise wrapper over MQTT.js
MIT License
271 stars 49 forks source link

Properly close the client before rejecting on error #57

Open myasul opened 1 year ago

myasul commented 1 year ago

I created a class wrapper to use the async-mqtt to test connecting to our broker. I created a test where the user does not have proper permissions to connect to the broker, so the expectation is for the asyncConnect to throw an error.

It does indeed throw an error, but Jest didn't terminate properly after running all the tests, which means there are asynchronous operations that weren't stopped. I debugged where this asynchronous operation is coming from and found out that it is client.end in the error listener.

connectAsync (brokerURL, opts, allowRetries=true) {
  const client = mqtt.connect(brokerURL, opts);
  const asyncClient = new AsyncClient(client);

  return new Promise((resolve, reject) => {
    // Listeners added to client to trigger promise resolution
    const promiseResolutionListeners = {
      // ....
      error: (err) => {
        removePromiseResolutionListeners();
        client.end(); // <--- This is the asynchronous operation still running after throwing.
        reject(err);
      }
    };
    // ....
  }
}

The fix is to wait until the client is properly closed before rejecting with an error.

image