hotmeteor / receiver

Receiver is a drop-in webhook handling library for Laravel.
MIT License
384 stars 21 forks source link

Breaking change in v0.1.15, classes not constructed the same? #23

Closed ChrisHardie closed 1 year ago

ChrisHardie commented 1 year ago

The change in v0.1.15 introduced in #18 created a fatal error for me when deployed to production. It may be my code, but here's what I saw:

The webhook event handling stopped happening synchronously, which was succeeding, and was moved to a queued job, which failed.

The code that ran to handle the webhook event is

<?php

namespace App\Http\Handlers\Woocommerce;

class UserCreated extends AbstractUserEvent
{
    public function handle()
    {
        $this->createContactFromUser();
    }
}

and the parent class has this construction:

<?php

namespace App\Http\Handlers\Woocommerce;

use App\Data\WoocommerceCustomer;
use App\Data\WordpressUser;
use App\Helpers\WoocommerceHelpers;
use App\Models\Contact;
use Illuminate\Support\Facades\Log;

abstract class AbstractUserEvent extends AbstractWoocommerceEvent
{
    private WoocommerceHelpers $wooHelper;

    public function __construct(public string $event, public array $data)
    {
        parent::__construct($event, $data);

        // Create a Woo customer object we can work with.
        $this->wooHelper = new WoocommerceHelpers();

        ...
    }

    protected function createContactFromUser(): Contact|null
    {
        $newContact = $this->wooHelper->createLocalFromWpUser($this->user);
        return $newContact;
    }

    ...
}

The error I started getting was:

local.ERROR: Typed property App\Http\Handlers\Woocommerce\AbstractCustomerEvent::$wooHelper must not be accessed before initialization {"exception":"[object] (Error(code: 0): Typed property App\\Http\\Handlers\\Woocommerce\\AbstractCustomerEvent::$wooHelper must not be accessed before initialization at <path>/app/Http/Handlers/Woocommerce/AbstractCustomerEvent.php:51)

So it appears that the __construct method in AbstractUserEvent is not run following this release. Downgrading to 0.1.14 fixed it.

Please let me know if there's any other information that would be helpful to understand or advise on what's happening. Thank you.

hotmeteor commented 1 year ago

The change was made from dispatchAfterResponse to dispatch because the former cannot go to a queue, which is what is required for some handlers.

As for your examples, I'm not sure what code is yours or another package's, but I think you just need to make your object nullable? Seems like more of a PHP 8 thing.

abstract class AbstractUserEvent extends AbstractWoocommerceEvent
{
    private WoocommerceHelpers|null $wooHelper = null;

    public function __construct(public string $event, public array $data)
    {

...
ChrisHardie commented 1 year ago

(Closing this until/unless I can provide more details showing it's actually an issue and not my misunderstanding.)