-
Notifications
You must be signed in to change notification settings - Fork 63
Issue with getOrder method in WebhookController #117
Comments
Thanks @EvgeniyHablak for reporting this. I have fixed the inconsistency in the Can you help me understand what's going on with the second part of your question? |
ah found it |
Dropped the 'temp_' prefix in the controller: |
We are almost there BUT |
Could you check:
|
I've tried to debug library a little bit. |
Mollie's webhook call is crazy fast. It's probably a concurrency issue. We'll have to look into it. |
What cashier-mollie version are you at? |
@sandervanhooft |
Can you give |
Maybe i'm testing incorrectly
|
Hi @EvgeniyHablak,
|
Hi @sandervanhooft ! Thanks! I'll check it out asap |
Closing this for now, let me know if it should be reopened @EvgeniyHablak . |
I don't want to hijack this issue, but I also have some issues with the default Webhook callback. The getOrder function returns always null (can't find by mollie_payment_id). Could it be that the Webhook is called before the Order actually exists in the database.....? In this case the webhook is called after swapping a plan. |
@Paulsky what version are you using? |
I'm using v1.10.3 |
Just to make things clear (maybe I'm doing something wrong): I'm using the swap() function on an existing subscription. The webhook is called correctly, and the data in the Mollie dashboard looks also correct. When I manually check the database, the mollie_payments_status is 'open' and the 'mollie_payment_id' is filled. The Order before the swap, created with the newSubscription, is 'paid'. The given payment ID with the request is set and looks correct. But as I said, maybe the Order doesn't exists at the time of the webhook call? Because Order::where('mollie_payment_id', $id)->first(); returns null. |
So I did a lot of debugging. The Order is found in the webhook function, if I remove the DB::transaction() wrapper in the Subscription restartCycleWithModifications() function. This way, the handlePaymentPaid() is called in the webhook and the OrderPaymentPaid event is fired. So that's great (for sending a new invoice). But, somehow, the order status is still 'open', even though Hope this helps you understand my issue a little bit! |
I know you are very busy @sandervanhooft , and I don't want to push you. But maybe you can re-open this issue...? |
I have been thinking about this... The transaction is there for a reason. It's good to disable it to understand the problem, but I think we should keep it there. As I've discussed here, there are essentially two alternatives we can try:
For simplicity and ease of debugging, I'm leaning towards option 1. What do you think, @Paulsky et al? |
Hmm, I'm not sure. I'm also leaning towards option 1. But, the problem I have is that I don't understand the reason why this is happening. Why is the Order not found (while it really seems to exists in the db)? Maybe if I understand the reason, I can give a better suggestion. Do you know what the problem is and could you explain it to me? Thank you @sandervanhooft 🙏 Edit: I thought I have read somewhere that you wrote that the Mollie webhook is too fast. But I'm not sure about that myself; why would the Order be found if the DB:transaction wrapper is removed? This shouldn't speed up the webhook code would it? Or am I wrong about this? |
It's a race condition issue. Mollie's webhook call is so crazy fast it calls the webhook before the transaction is finished (meaning the Order is not stored yet). This all happens in a fraction of a second. So to the human eye it seems like the Order already is in the database table. But in fact, it's not stored yet. |
Alright, incredible! If that's the case and if the DB::transaction wrapper can't be removed (I don't know if this is even the problem) I also recommend option 1; return a 404 because the Order really does not exists right? It feels less hacky and a lighter solution (instead of forcing users to use queues right?) |
Exactly @Paulsky , thanks! |
@Paulsky is this is issue occurring in test or live mode? |
This occurs in test mode. But live I'm not sure, because I just saw I have some 'paid' 'mollie_payment_status' in the live database. |
Ok, any of them not marked as |
No, it looks correct at live! |
I've got an error while testing the webhooks
Undefined property: stdClass::$temporary_payment_id {"exception":"[object] (ErrorException(code: 0): Undefined property: stdClass::$temporary_payment_id at /var/www/wordproof/vendor/laravel/cashier-mollie/src/Http/Controllers/WebhookController.php:52)
And it seems like this is because of different properties in this places
But when i've set the same properties the order is not found by findByPaymentId using uuid.
And thats why order status can't be updated.
This is how fixed version look but it works not as expected.
The text was updated successfully, but these errors were encountered: