Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly close the client before rejecting on error #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

myasul
Copy link

@myasul myasul commented Apr 30, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant