Failed-method may cause serialization issues

[Urbanproof]

In a nutshell, defining a failed method in the action may cause serialization issues like Serialization of 'Closure' is not allowed..

Minimal reproduction would look something like this (this assumes that PSR-17 & PSR-18 factories / clients are bound to DI container, in this case I was using Guzzle):

// the action
<?php

namespace App\Actions;

use Illuminate\Support\Facades\Log;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestFactoryInterface;
use Psr\Http\Message\StreamFactoryInterface;
use Psr\Http\Message\UriInterface;
use Spatie\QueueableAction\QueueableAction;
use Throwable;

final class TestAction
{
    use QueueableAction;

    public function __construct(
        private ClientInterface $httpClient,
        private RequestFactoryInterface $requestFactory,
        private StreamFactoryInterface $streamFactory,
    ) {
    }

    public function execute(UriInterface $uri, array $payload): void
    {
        $request = $this->requestFactory->createRequest('POST', $uri)
            ->withHeader('Content-Type', 'application/json')
            ->withBody($this->streamFactory->createStream(json_encode($payload)));
        $this->httpClient->sendRequest($request);
    }

    public function failed(Throwable $exception): void
    {
        // Send user notification of failure, etc...
        Log::debug('TestAction failed', ['exception' => $exception->getMessage()]);
    }
}


// Calling code; tinker works
$uri = resolve(Psr\Http\Message\UriFactoryInterface::class)->createUri('http://example.com/');
$payload = ['foo' => 'bar'];
resolve(App\Actions\TestAction::class)->onQueue()->execute($uri, $payload);
// which throws;  Exception  Serialization of 'Closure' is not allowed.

ActionJob sets the failure handler in the constructor like this;

if (method_exists($action, 'failed')) {
    $this->onFailCallback = [$action, 'failed'];
}

This will cause the serialization of the whole action-instance, which in turn causes the actual error if any of the instance properties are not serializable.

I'm not sure if this constitutes as an issue of the library code as the error is caused by the user-defined action, but this is something that could potentially be handled in the library as well. I understand that serializing the actual instance has some benefits like preserving the actual instance properties, but given that those are more or less "static" (you are not really supposed to use constructor to do that much besides DI I think) the execute method parameters are probably the more important ones anyway. In that case the failure handler could actually be refactored slightly.

1. Don't set the onFailCallback property at all in the ActionJob
2. Change the failed-handler in the ActionJob;

    public function failed(Throwable $exception)
    {
        if (method_exists($this->actionClass, 'failed')) {
           // `execute` parameters here could be handy when debugging
            return resolve($this->actionClass)->failed($exception, $this->parameters);
        }
    }
   

As stated above I'm not that sure if this is an issue or not, but at least this may give some ideas to anyone else experiencing this.

Ps. thanks for your great work on open-source packages. 🙌

Pps. In case this is not deemed to be a library-issue (feature?), this should be fixed in the action code by gracefully handling the serialization / deserialaization with __sleep & __wakeup handlers.