Issue returning a rejected promise in worker

[dusty]

Thanks for writing this.

I'm running into an issue when I return a rejected promise inside a worker. The job gets stuck in busy until timeout. It works fine if I return a resolved promise.

Any ideas?

Here is my worker

const faktory = require('faktory-worker');

const doWork = async (id, size) => {
  return new Promise((resolve, reject) => {
    reject('Test')
  })
}

faktory.register('MyDoWorkJob', doWork);

faktory.work();

Pushing into the queue

const faktory = require('faktory-worker');

async function run() {
  const client = await faktory.connect();
  client.push({
    queue: 'default',
    jobtype: 'MyDoWorkJob',
    args: []
  });

}

run().then(process.exit)

Here is the error message

> node worker.js
(node:90083) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): TypeError: Cannot read property 'split' of undefined
(node:90083) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

[jbielick]

What version of faktory-worker are you using?

[jbielick]

This is an interesting issue. It boils down to the processor expecting that if your job returns a rejected promise, that there's an error that it is rejecting with—rather than undefined or a string. In your example, reject('Test') rejects the promise with a string, not an error. When the processor encounters this, it attempts to FAIL your job in the faktory server (a rejected promise will always be FAILed). FAIL requires an error message and stack trace when sent to Faktory Server, which is why this lib is attempting to call e.message, e.stack.split(...) and so on.

I think this article "A String is not an Error" may articulate the nuance here. It is important to always use an error where one is expected, though node itself doesn't enforce this.

I think I'll probably make an update to to accept a string (and maybe worse, undefined) as an error so that things still work, but in the mean time using reject(new Error('Test')) would suffice for your case.

[jbielick]

fix in [email protected]

[dusty]

Thanks for the explanation, that makes sense. I wouldn't have even seen it had I actually been using real data that would have passed a proper error.

👍