[BUG] Job.moveToFailed does not emit 'failed' event

[alolis]

Description

Just wanted to report in case it's not known already that when I use job.moveToFailed no failed event is emitted. It would be nice if it did.

From what I checked only queue.js will emit the event after the method is called but I think the job object should do that instead.

Minimal, Working Test code to reproduce the issue.

import Queue from 'bull';

const queue = new Queue('myQueue');

queue.process('/path/to/my/processor.js');
await queue.add({foo: 'bar'}, {uuid: 1});

const job = await queue.getJob(1);
await job.moveToFailed(new Error('some error'), true); // will not emit failed event

Bull version

I am using Node.js v6.9.1 with bull v3.4.8 and redis-server v3.0.7

Thank you for your time.

[manast]

Can you provide a test code that reproduces the issue?

[alolis]

@manast sorry about that, I have updated the original post

[manast]

The problem here is that you are both defining a process function and moving to fail manually. What probably happens is that it gets processed before the call to moveToFailed.

[alolis]

Processing is definitely not finished in my actual code when I do this; job still running.

Also I checked the code and here is where the event is emitted. if you check Job.moveToFailed there is no emission of the failed event, unless I missed something.

[manast]

If you remove this code it will work:

queue.process('/path/to/my/processor.js');

[manast]

Also I checked the code and here is where the event is emitted. if you check Job.moveToFailed there is
no emission of the failed event, unless I missed something.

Yes, you are missing that the failed event is emitted from the LUA script moveToFinished :)

[alolis]

@manast , ah I see, although i checked here and I still do not see the event emission! Can you please point out where exactly it happens?

So, just to clarify here, If the job is in process, can I job.moveToFailed(new Error('somereason'), true) (while it's still in process) and then wait for the failed event? or this is only for waiting jobs? Also i want to mention here that, after the call to job.moveToFailed i do see the job inside redis with failedReason "somereason" so the call must be doing something!

[manast]

If the job is in process you cannot call moveToFailed. You should never call that method if you have defined a process function.

[alolis]

Thanks for your patience. The whole flow is not very clear to me yet but I have been doing the following in sandbox.js in order to enable cancellation and it seems to be working fine:

 return done.finally(function() {
        if (done.isCancelled()) {
          job.discard();
          job
            .moveToFailed(new Error('cancelled'), true)
            .then(() => child.kill());
        }

        childPool.release(child);
      });

Are you saying that calling moveToFailed there is bad and it might ruin the bull flow?

The result of the above is that the job (even while in progress) is indeed moved to failed state with failed reason cancelled (I checked redis as well) and the process is killed after the moveToFailed promise resolves. No failed events get emitted though.

[manast]

I don't know, it must be debugged in order to be fully understood. But basically, you are calling a method that the queue is also calling, which is not good.

[manast]

btw, you can use redis command MONITOR in redis-cli to see what is going in redis.

[alolis]

I will run a few debug tests with monitor and get back to you.

[alolis]

@manast , after plenty of runs, i still don't think that the event is emitted anywhere else other than in queue. Just wanted to let you know.

[manast]

https://github.com/OptimalBits/bull/blob/develop/lib/commands/moveToFinished-6.lua#L56

[tommyc38]

So is the correct way to move a job to failed to return an error from the job process function? I wasn't clear either on exactly how to move jobs to the failed state and was trying to do it by calling moveToFailed.

[SqueezedLight]

@tommyc38 i think yes. Something like that:

try {
  // code that is run by your job
} catch (e) {
  return Promise.reject(new Error(`${e}`));
}

This will move the job to the failed queue.

[aditodkar]

@tommyc38 i think yes. Something like that:

try {
  // code that is run by your job
} catch (e) {
  return Promise.reject(new Error(`${e}`));
}

This will move the job to the failed queue.

Thanks a lot. This worked for me :)

[iflairritesh]

Hey @tommyc38
This is not a solution for the moveToFailed function, If I want to move the current running job to failed handler with its job id from somewhere else How would I do that?