mateusjunges / trackable-jobs-for-laravel

This package allows you to easily track your laravel jobs!
https://mateusjunges.com
MIT License
280 stars 16 forks source link

Jobs are not getting retried, using Laravel Horizon #35

Closed DimaVIII closed 2 years ago

DimaVIII commented 2 years ago

Hello,

I'm running into an issue where Horizon is marking a job as failed without attempting any retries.

Just a simple job for debugging purpose. If I add the Trackable trait, the job goes into failed jobs without any retry attempt. Without the Trackable trait it works as expected.

class AssignIpaddress implements ShouldQueue
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels, Trackable;

   public $tries = 10;
   public $backoff = [3, 5, 10]; 

    public function handle()
    {

        $ip = Ip::inRandomOrder()->first();
        logger("Domain {$this->model->name} was assigned to IP: #{$ip->id} {$ip->ipaddress}");

        throw new \Exception('OHH A RANDOM ERROR.');

        // Safe output to tracked_jobs table
        return "Domain {$this->model->name} was assigned to IP: #{$ip->id} {$ip->ipaddress}";
    }
}
DimaVIII commented 2 years ago

Update: There is a issue with the middleware class: https://github.com/mateusjunges/trackable-jobs-for-laravel/blob/v1.5.x/src/Jobs/Middleware/TrackedJobMiddleware.php

Explanation can be found here: https://github.com/laravel/framework/issues/32266

Jobs getting not retried if the middleware is calling $job->fail(). Taylor Otwell suggested to call if ($job->job->hasFailed()) instead but there is also an issue with this method.

There is also some weird behavior due to the try catch block, after removing this as well everything started to work as expected.

My middleware looks now like this:

namespace App\Http\Middleware;

class NewTrackedJobMiddleware
{
    public function handle($job, $next)
    {
        $job->trackedJob->markAsStarted();
        $job->trackedJob->markAsFinished($next($job));
    }
}
mateusjunges commented 2 years ago

Hi @DimaVIII I'm returning from my vacation trip today, I'll look into this tomorrow. Thanks for reporting.

DimaVIII commented 2 years ago

After trying around with different jobs I noticed that jobs which get released back into the queue are getting marked as finished instead of queued. I added a check to mark the job as queued $job->job->isReleased().

Notes:

Middleware

<?php

namespace App\Jobs\Middleware;

class NewTrackedJobMiddleware
{
    public function handle($job, $next)
    {
        $job->trackedJob->markAsStarted();

        $response = $next($job);

        if ($job->job->isReleased())
        {
            $job->trackedJob->update([
                'status' => $job->trackedJob::STATUS_QUEUED,
            ]);
        }
        else {
            $job->trackedJob->markAsFinished($response);
        }
    }
}

Test job

<?php

namespace App\Jobs;

use App\Jobs\Middleware\NewTrackedJobMiddleware;
use App\Models\User;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Junges\TrackableJobs\Concerns\Trackable;

class DebugJob implements ShouldQueue
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels, Trackable {
        __construct as __baseConstruct;
    }

    public $tries = 4; 

    public function __construct(User $user)
    {
        $this->__baseConstruct($user);
    }

    public function handle()
    {
        logger("DebugJob ++++ STARTED... for User # {$this->model->id}");

        sleep(5);

//        if ( $this->attempts() == 3) {
//            logger("DebugJob ++++ FINISHED ... for User # {$this->model->id}");
//            return 'JOB Done';
//        }

        $this->release(
            now()->addSeconds(10)
        );

    }

    public function middleware(): array
    {
        return [new NewTrackedJobMiddleware()];
    }
}
mateusjunges commented 2 years ago

After trying around with different jobs I noticed that jobs which get released back into the queue are getting marked as finished instead of queued. I added a check to mark the job as queued $job->job->isReleased().

Hi @DimaVIII, yeah, nice catch. Can you submit a PR with this fix? If you don't have time for that then I'll work on it on weekend. Thaks for figuring it out!

mateusjunges commented 2 years ago

Hey @DimaVIII I've just released the fix on v1.5.2. Thanks for reporting and sending the PR :smile: