Skip to content

[PIWOO-841] Prevent duplicate processing for orders with final status or already paid in Mollie WebhookHandler#1182

Open
Biont wants to merge 1 commit intodev/developfrom
dev/PIWOO-841-expiry-webhook-race-condition
Open

[PIWOO-841] Prevent duplicate processing for orders with final status or already paid in Mollie WebhookHandler#1182
Biont wants to merge 1 commit intodev/developfrom
dev/PIWOO-841-expiry-webhook-race-condition

Conversation

@Biont
Copy link
Copy Markdown
Collaborator

@Biont Biont commented Mar 31, 2026

Fixes #1159

Problem

When a customer makes multiple payment attempts on the same order (e.g. returning to the order-pay URL after abandoning a first attempt), an expired-payment webhook can cancel an order that has already been successfully paid.

onWebhookExpired has two guards meant to prevent this:

  • Guard 1!$order->needs_payment(): skips processing if WooCommerce considers the order paid
  • Guard 2$molliePaymentId !== $payment->id: skips processing if a newer transaction is on record

Both can be bypassed in practice:

Scenario A (primary — sequential): A new payment attempt resets the order to pending. The customer completes that payment and the order moves to processing. If Mollie subsequently delivers an expired webhook for that same transaction (reproducible via test-mode dashboard), Guard 1 can still return true and Guard 2 passes because the IDs match — the handler proceeds to cancel a paid order. Reported at ~5% frequency on affected installations.

Scenario B (secondary — race condition): Two webhooks for the same order arrive in close succession (paid + expired). If the expired webhook's PHP process reads the order before the paid webhook's process has committed payment_complete() and the _mollie_paid_and_processed flag to the database, both guards see a pending order and proceed to cancel it.

A related structural gap: onWebhookCanceled already has an isFinalOrderStatus guard; onWebhookExpired had no equivalent.


Solution

Two additions at the top of onWebhookExpired, before the existing guards:

1. isFinalOrderStatus check — mirrors the guard already present in onWebhookCanceled. Returns early for completed, refunded, and canceled orders. This was simply missing from the expired path.

2. Strengthened $alreadyPaid check — replaces the bare needs_payment() call with a three-way condition:

$alreadyPaid = !$order->needs_payment()
    || $order->get_status() === 'processing'
    || $order->get_meta('_mollie_paid_and_processed', true);
  • !$order->needs_payment() — the original guard, retained
  • === 'processing' — explicit status check independent of WC's needs_payment() filter chain; processing is not in FINAL_STATUSES so the first guard above doesn't cover it
  • _mollie_paid_and_processed — set by the paid-webhook handler immediately after payment_complete(), before the order note and subscription logic; checking it narrows the race window for Scenario B

Rationale & known limitations

The isFinalOrderStatus addition is a straightforward parity fix with onWebhookCanceled. The $alreadyPaid expansion is defensive layering: each condition catches a slightly different failure mode and together they make the "already paid" check robust against both sequential and concurrent webhook delivery.

Scenario B is not fully eliminated. The race condition is a TOCTOU problem across two PHP processes with no shared in-process state. WooCommerce has no public API for a guaranteed cache-busting DB re-read that is also safe under HPOS. The only complete fix would be either a transient-based per-order mutex (serialising concurrent webhooks) or a Mollie API call immediately before the destructive write to verify payment status on Mollie's authoritative side. Both are intentionally deferred: they touch a broader surface, carry their own failure modes, and belong in a separate focused change. The _mollie_paid_and_processed check meaningfully shrinks the race window and is a safe interim improvement.

@Biont Biont requested a review from danielhuesken March 31, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expired webhook for stale transaction cancels order despite subsequent payment attempts

2 participants