Errors thrown inside of queue "completed" handler result in "Missing lock" errors

[sysrun]

This one gave me hours of pain...

Description

If an error is throw inside the queue "completed" handler, the queues "error" event will be triggered with "Error: Missing lock for job failed" and errorMessage "Error processing job".

Minimal, Working Test code to reproduce the issue.

After the second iteration the described error will be triggered

const bull = require('bull');

const queue = bull('testqueue', 'redis://localhost:6380');

queue.process(async (job) => {
  console.log('>> processing', job.id);
  return Promise.resolve(true);
});

queue.on('completed', (job, result) => {
  console.log('>>> completed', job.id);
  if (job.data.counter >= 2) {
    console.log('>>> counter >= 2, throwing inside "completed" handler');
    throw Error();
  }
});

queue.on('error', (error, errorMessage) => {
  console.error('error', error);
  console.error('errorMessage', errorMessage);
});

let counter = 0;
setInterval(() => {
  const job = queue.add({counter: counter++});
  console.log('> added job', job.id);
}, 1000);

Bull version

4.1.0

[sysrun]

Looks like thats the problem

bull/lib/queue.js

Lines 1092 to 1097 in 019d612

	const handleCompleted = result => {
	return job.moveToCompleted(result, undefined, notFetch).then(jobData => {
	this.emit('completed', job, result, 'active');
	return jobData ? this.nextJobFromJobData(jobData[0], jobData[1]) : null;
	});
	};

I modified the code; error not happening anymore

const handleCompleted = result => {
   return job.moveToCompleted(result, undefined, notFetch).then(jobData => {
     try {
       this.emit('completed', job, result, 'active');
     } catch (err) {
       // ignore
     }
     return jobData ? this.nextJobFromJobData(jobData[0], jobData[1]) : null;
   });
 };

[manast]

Isn't it insane that emit can throw exceptions? Actually now that I read this issue I have a weak recall about this problem in another context. This is really insane if you think about it, and not the way you expect events should work, which its main feature is to decouple the event emitter from the event listeners... anyway, thanks for reporting.

[sysrun]

Yeah, i don't get it too. Thats why it took me hours to find the problem...

[sysrun]

https://nodejs.org/api/events.html#events_capture_rejections_of_promises

[manast]

@sysrun not sure that would solve the problem actually. The case at hand is when a non async listener throws an exception, as your code above demonstrates. Best would be to wrap emit so that it catches all possible exceptions that can be generated inside the listener. If the listener would have been async the throw will just be ignored.

[sysrun]

So an emit wrapper would catch possible exceptions and "redirect" them to the default "error" event?

[manast]

Yeah, thats what I am planing to do to fix this issue anyway.

[manast]

ahh, actually I fixed this on BullMQ already, I just forgot mostly about it: https://github.com/taskforcesh/bullmq/blob/master/src/classes/queue-base.ts#L51

[manast]

🎉 This issue has been resolved in version 4.1.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀