Failed() method not called on failing jobs. [5.0] [5.1]

[Shkeats]

I and a number of others have noticed that the failed method does not seem to get called on jobs that fail when being executed by workers off the queue. The failure is detected by laravel because the jobs are added to the failed_jobs table however the failed() method on the job is not called and the Queue::failing() event doesn't seem to be triggered.

Seems to be an issue in both 5.0 and 5.1.

Possibly related to #9473 which was closed by @GrahamCampbell

https://laracasts.com/discuss/channels/general-discussion/queuefailing-laravel-5

http://stackoverflow.com/questions/31305571/laravel-5-1-failed-queued-jobs-fails-on-failed-method-prevents-queue-failure

[GrahamCampbell]

This is the expected behaviour. We only fire that event if firing the job failed.

[GrahamCampbell]

If we crashed before we started firing the job, no event will be fired.

[Shkeats]

@GrahamCampbell I don't quite understand what you mean by firing the job failed. Is this if no handler is found? Could you give an example?

In my problem an uncaught exception is thrown inside the handler, the worker detects the exception, logs it and adds the job to the failed_jobs table. So far, as expected. However the failed event doesn't happen. Surely that job counts as failed and should trigger it's failed() method?

If there is some distinction between my scenario and some other sort of failure it didn't come accross from the docs.

[GrahamCampbell]

Firing the job occurs when laravel actually calls the handle/fire method on your job. If it doesn't get that far, then no events are fired.

[Shkeats]

@GrahamCampbell OK but my job doesn't have a handle() method it has a handler that Laravel finds using the naming convention. The failure occurs within the handler for the job.

So in my scenario the Handler class is found, the handle() method on my handler is called successfully, there is an exception thrown somewhere inside the handle() method and the job fails.

Does the failed event only support self handling jobs?

[Shkeats]

To further clarify. I'm on 5.0 so I have app/Commands with MyCommand.php. It implements ShouldBeQueued and uses InteractsWithQueue, SerializesModels.

I new it up and dispatch it from a controller with $this->dispatch($myCommand).

The queue::listen daemon picks it up from the queue and finds the MyCommandHandler class in Handlers/Commands, calls the handle method passing the job into it. The job is then executed as expected. Unless there is an exception. In which case, see above.

[GrahamCampbell]

The event should be fired, and it does for me. I can't replicate. Perhaps you have a typo?

[Shkeats]

@GrahamCampbell OK thanks for responding. I'll investigate a bit more and see if I can replicate in a new project for you.

I would suspect user error too but the job/queue/handler system is functioning normally as long as there is no exception thrown so I can't think where I could introduce a typo without otherwise breaking it. I've checked the spelling of my failed() method.

I'm using Redis for a queue driver and queue::listen on daemon mode, but it does the same without the daemon flag.

[Shkeats]

@GrahamCampbell OK I've worked out what was happening. My failed() method was on the job class as per the docs, but in this use case the failed() method on the handler class is what gets called. By implementing the method there I managed to acheive the expected functionality.

http://laravel.com/docs/5.1/queues#dealing-with-failed-jobs

Assuming this is expected behaviour but could be stated more clearly?

[GrahamCampbell]

I think the 5.0 docs probably explain it better. 5.1 pushes self handling more.

[Shkeats]

I don't want to do the pull request for 5.1 as I'm not familiar with the new folder structure.

[Shkeats]

@GrahamCampbell Before I open a seperate issue, is there anyway to access the job instance from inside the failed() method on a handler? It seems like the handler class has been reinstantiated when failed() is called so I can't access the class variables that I set when handle() was called.

Since (correct me if I'm wrong) the job isn't passed back into the failed() method I can't actually do anything as I have no information about the job that failed.

Maybe conceptually the failed() method should be called on the job class even if handlers are being used?

[GrahamCampbell]

If you want access, you could set it as a property on the handler from within your handle method.

[GrahamCampbell]

I personally don't like the idea of a failed method though. I'd recommend implementing this using command middleware.

[Shkeats]

@GrahamCampbell Unfortunately the property route doesn't work. Like I said, it seems like the handler is getting reinstantiated at some point causing that variable to be lost.

 <?php
 class MyHandler extends CommandHandler
{
 protected $myJob;

 public function handle(MyJob $myJob)
 {
     //Set in handle method.
    $this->myJob = $myJob;
 }

 public function failed(){
     //Can't retrieve here.
     dd($this->myJob);
 }
}

[GrahamCampbell]

You need to bind your handler as a singleton to the container.

[Shkeats]

In the end the most elegant solution I could find to get the the failed() method to trigger on the job class itself was to add to the below to the boot() method of EventServiceProvider.php. Catching the full fail event that is fired and then digging out the command/job and unserializing it to call the failed() method.

Queue::failing(function($connection, $job, $data)
        {
            $command = (unserialize($data['data']['command']));
            $command->failed();
        });

There goes my afternoon but I hope this helps someone!

Edit: This of course gives you access to all the data that was set in the intial job/command so you can carry out the necessary logic for your job failure.

[GrahamCampbell]

What was wrong with using middleware?

[Shkeats]

@GrahamCampbell I'm sure nothing! I'm just not familiar with middleware in Laravel so the above was the shortest way around the problem for now. If I get a middleware solution put together I'll post an example back here too. Thanks for the help.

[nosun]

mark here, the same issue

[etiennemarais]

Seems like this has changed a little in 5.2.

Queue::failing(function (JobFailed $event) {
            // $event->connection
            // $event->$job
            // $event->$data
        });

[vafrcor]

Still have this issue on Laravel 5.2.x (latest) Condition:

• Queue Worker Daemon triggered by Supervisord
• Each Job has been triggered with long time runtime (heavy task using loop)
• Failed Job recorded on failed_jobs table, but no entry on Laravel Log file.

[stdclass]

Still apparrent in Lumen 5.4.7

[hadifarnoud]

In the end the most elegant solution I could find to get the the failed() method to trigger on the job class itself was to add to the below to the boot() method of EventServiceProvider.php. Catching the full fail event that is fired and then digging out the command/job and unserializing it to call the failed() method.

Queue::failing(function($connection, $job, $data)
        {
            $command = (unserialize($data['data']['command']));
            $command->failed();
        });

There goes my afternoon but I hope this helps someone!

Edit: This of course gives you access to all the data that was set in the intial job/command so you can carry out the necessary logic for your job failure.

does this give me job data in logs? I want to have the data of failed jobs with Laravel 5.1 so I put this in EventServiceProvider

public function boot(DispatcherContract $events)
    {
        parent::boot($events);
        Queue::failing(function($connection, $job, $data)
        {
            $command = (unserialize($data['data']['command']));
            $command->failed();
        });
        //
    }

[hadifarnoud]

it should be like this.

use Illuminate\Support\Facades\Queue;

    public function boot(DispatcherContract $events)
    {
        parent::boot($events);
        Queue::failing(function($connection, $job, $data)
        {
            $command = (unserialize($data['data']['command']));
            $command->failed();
        });
        //
    }
}

[xwiz]

In my case I forgot to import Exception type hinted in failed method.

[chadgrigsby]

In my case, I need to restart the queue worker.. ugh!