laravel / cashier-mollie

MIT License
375 stars 63 forks source link

WebhookController throws exception when payment failed because of missing mandate. #105

Closed edcoreweb closed 5 years ago

edcoreweb commented 5 years ago

In case of a failed payment which doesn't have a mandate (user deleted card or something), the controller fails, which causes the webhook to fail, and the subscription is not canceled.

https://github.com/laravel/cashier-mollie/blob/develop/src/Http/Controllers/WebhookController.php#L30

https://github.com/laravel/cashier-mollie/blob/develop/src/Order/Order.php#L356

This line throws an exception which is not caught.

https://github.com/laravel/cashier-mollie/blob/develop/src/Billable.php#L390

Proposed fix is to add some error handling in the mollieMandate method.

/**
 * @return \Mollie\Api\Resources\Mandate|null
 */
public function mollieMandate()
{
    $id = $this->mollieMandateId();
    $mandate = null;

    if (! empty($id)) {
        try {
            $mandate = $this->asMollieCustomer()->getMandate($id);
        } catch (ApiException $e) {} // A revoked mandate may no longer exist, so throws an exception
    }

    return $mandate;
}

Can put together a pr if needed.

sandervanhooft commented 5 years ago

Hi @edcoreweb,

Good catch. I happen to be working on the mandate checks right now, so I'll include a fix there.

It will be a bit more specific, only catching status 404 or 410. And try will exclude the asMollieCustomer part.

edcoreweb commented 5 years ago

@sandervanhooft Great!

sandervanhooft commented 5 years ago

Hi @edcoreweb ,

Just checked with Mollie's support because I was unsure about the API behaviour. A revoked mandate will remain retrievable, but will have a status invalid. So I don't think this would break this code actually. Can you figure out what triggered your error?

sandervanhooft commented 5 years ago

Nonetheless, I've included a fix in develop

edcoreweb commented 5 years ago

@sandervanhooft Sorry for the late reply, not sure why this happened but it can happen. Maybe Mollie needs to investigate this.

The user payments: Screenshot_4

The missing mandate: Screenshot_5

This was retrieved with Postman using Mollie's api. Some sensitive data has been removed.

As you can see the first payment worked fine, has the mandate. Second payment a month later, with same mandate failed because of it missing.

Maybe this helps someone, but happy to hear it's handled nonetheless.

sandervanhooft commented 5 years ago

Big thanks for reporting this back in full detail. I'll notify Mollie support.

sandervanhooft commented 5 years ago

Cleared it up: exception ~404~ 410 is the proper response for fetching a revoked mandate.