jessetane / queue

Asynchronous function queue with adjustable concurrency
MIT License
764 stars 66 forks source link

Unexpected behavior when using bluebird Promise #50

Closed alolis closed 6 years ago

alolis commented 6 years ago

Hello,

I have noticed some weird behavior whenever I am using bluebird and async job handlers.

The following sample code will output a warning on the console:

import queue from 'queue';
import Promise from 'bluebird';

function createJobHandler(jobId) {
  return async function() {
    console.log(`Running handler for ${jobId}`);

    return jobId;
  }
}

function generateHandlers(numHandlers) {
  let counter = 1;
  let handlers = [];

  while (counter <= numHandlers) {
    handlers.push(createJobHandler(counter));
    counter++;
  }

  return handlers;
}

const jobsQueue = queue({concurrency: 1});

jobsQueue.on('error', (error) => {
  console.log("Error", error);
});

jobsQueue.on('success', (jobId, jobHandler) => {
  console.log(`Job ${jobId} completed!`);
});

jobsQueue.on('end', () => {
  console.log("All jobs completed");
});

const handlers = generateHandlers(2);
jobsQueue.push(...handlers);

jobsQueue.start();

The above will produce the following warning:

(node:36404) Warning: a promise was created in a handler at /myproject/node_modules/queue/index.js:145:7 but was not returned from it, see http://goo.gl/rRqMUw
    at new Promise (/myproject//node_modules/bluebird/js/release/promise.js:79:10)

An explanation of the above warning can be found here.

And the line from queue which triggers the warning is the following:

if (promise && promise.then && typeof promise.then === 'function') {
    promise.then(function (result) { // LINE 145  is here
      next(null, result)
    }).catch(function (err) {
      next(err || true)
    })
  }

If I remove the import Promise from 'bluebird' line however, everything will work normally.

Thank you for your time.

jessetane commented 6 years ago

I don't know Promises well, maybe @kwolfy can help?