waitUntillFinished throws error for failed jobs

[bilalshaikh42]

Hello,
I have the following code. Assume there is some processor that throws an error when processing job

const queue = new Queue('Cars');
const events = new QueueEvents('Cars')
const job = await queue.add('paint', { colour: 'red' });
await job.waitUntillFinished(events) // error thrown here
const failed = await job.isFailed();
console.log(failed) // expect to print  "true" 

I would expect this code to print "true". However, the waitUntillFinished line instead throws the error, causing the process to end. Why is this? The description for the method states Returns a promise the resolves when the job has finished. (completed or failed). I interpreted this to mean that the promise will always resolve even if the job fails. Does the documentation need to be changed?

[bilalshaikh42]

The code that throws the error is here:

bullmq/src/classes/job.ts

Lines 741 to 806 in e55ff35

	async waitUntilFinished(
	queueEvents: QueueEvents,
	ttl?: number,
	): Promise<ReturnType> {
	await this.queue.waitUntilReady();
	const jobId = this.id;
	return new Promise<any>(async (resolve, reject) => {
	let timeout: NodeJS.Timeout;
	if (ttl) {
	timeout = setTimeout(
	() =>
	onFailed(
	/* eslint-disable max-len */
	`Job wait ${this.name} timed out before finishing, no finish notification arrived after ${ttl}ms (id=${jobId})`,
	/* eslint-enable max-len */
	),
	ttl,
	);
	}
	function onCompleted(args: any) {
	removeListeners();
	resolve(args.returnvalue);
	}
	function onFailed(args: any) {
	removeListeners();
	reject(new Error(args.failedReason || args));
	}
	const completedEvent = `completed:${jobId}`;
	const failedEvent = `failed:${jobId}`;
	queueEvents.on(completedEvent as any, onCompleted);
	queueEvents.on(failedEvent as any, onFailed);
	this.queue.on('closing', onFailed);
	const removeListeners = () => {
	clearInterval(timeout);
	queueEvents.removeListener(completedEvent, onCompleted);
	queueEvents.removeListener(failedEvent, onFailed);
	this.queue.removeListener('closing', onFailed);
	};
	// Poll once right now to see if the job has already finished. The job may have been completed before we were able
	// to register the event handlers on the QueueEvents, so we check here to make sure we're not waiting for an event
	// that has already happened. We block checking the job until the queue events object is actually listening to
	// Redis so there's no chance that it will miss events.
	await queueEvents.waitUntilReady();
	const [status, result] = (await Scripts.isFinished(
	this.queue,
	jobId,
	true,
	)) as [number, string];
	const finished = status != 0;
	if (finished) {
	if (status == -5 || status == 2) {
	onFailed({ failedReason: result });
	} else {
	onCompleted({ returnvalue: getReturnValue(result) });
	}
	}
	});
	}

Based on this, it seems clear that the intention is to reject the promise if the job has failed. IMO this is misleading since "finished" means either completed or failed. Since changing this would lead to breaking changes, perhaps just the documentation should be updated to clarify

[github-actions]

🎉 This issue has been resolved in version 1.68.2 🎉

The release is available on:

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

Your semantic-release bot 📦🚀