queue:retry is broken after recent commit

[chaoszcat]

• Laravel Version: 8.21.0
• PHP Version: 8.0.0
• Database Driver & Version: MySQL

Description:

A recent pull on the framework has broken the php artisan queue:retry. I submitted a comment on the commit but decided to create this issue since this is reproducible.

Steps To Reproduce:

1. Create a fresh project

composer create-project laravel/laravel test

2. Create database migrations

php artisan queue:table
php artisan migrate

3. Insert a new user into the users table with an email address.

INSERT INTO users SET name="test", email="test@example.com", password="";

4. Make sure .env is using database as the queue driver:

QUEUE_CONNECTION=database

5. Create a new notification that will fail (app/Notifications/TestNotification.php)

<?php

namespace App\Notifications;

use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Notifications\Notification;

class TestNotification extends Notification implements ShouldQueue
{
    use Queueable;
    
    public function via($notifiable)
    {
        return ['mail'];
    }

    public function toMail($notifiable)
    {
        throw new \RuntimeException('Purposely fail this notification to test queue:retry');
    }
}

6. Insert the notification into queue by calling this anywhere from the code:

(\App\Models\User::find(1))->notify(new \App\Notifications\TestNotification());

7. Run the queue worker. The worker should fail.

php artisan queue:work

8. Retry the queue, and the problem arise:

php artisan queue:retry all

ErrorException 

Attempt to read property "timestamp" on null

  at vendor/laravel/framework/src/Illuminate/Queue/Console/RetryCommand.php:132
    128▕ 
    129▕         $instance = unserialize($payload['data']['command']);
    130▕ 
    131▕         if (is_object($instance) && method_exists($instance, 'retryUntil')) {
  ➜ 132▕             $payload['retryUntil'] = $instance->retryUntil()->timestamp;
    133▕         }
    134▕ 
    135▕         return json_encode($payload);
    136▕     }

      +16 vendor frames 

Reasoning:

As per my comment on the commit, the retryUntil() method in the class SendQueuedNotifications does not always return a Carbon object as expected in the RetryCommand. Suggest a fix is to check also if the retryUntil() is returning a Carbon object before accessing the timestamp field.

[rodrigopedra]

I guess this is fixed by this commit: 4415b94

But it still needs to be tagged.

[chaoszcat]

@rodrigopedra no it's not. The if case in RetryCommand is checking if SendQueuedNotifications is indeed an object, and if the method retryUntil() is exist or not, and he simply call retryUntil()->timestamp without checking if the method is returning a Carbon object or not. The method defined in SendQueuedNotifications is returning null when the notification does not define a retryUntil().

[rodrigopedra]

I see now, thanks for the heads up.

[driesvints]

Thanks for the report. I've sent in a fix here: #35828