Skip to content

Fix webhook race condition where paid_out event arrives after failed … #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fix webhook race condition where paid_out event arrives after failed … #91

wants to merge 1 commit into from

Conversation

sidshas03
Copy link

@sidshas03 sidshas03 commented Sep 14, 2025 •
edited
Loading

Description

This PR fixes the race condition described in issue #7 where a paid_out event arrives after a failed event, causing orders to remain in failed status even though the payment was successful.

Problem

  • GoCardless webhook events can arrive out of order due to network delays or retries
  • When a failed event arrives first, it sets the order status to failed and removes _gocardless_temporary_activated
  • A subsequent paid_out event calls payment_complete() but the order is already in failed state
  • This causes the payment completion to fail or not properly transition the order status

Solution

  • Check if order is in failed or cancelled status when paid_out/confirmed events are received
  • Transition order status to processing before calling payment_complete()
  • Add comprehensive logging for debugging race conditions
  • Include unit tests for all scenarios

Changes

  • Modified: includes/class-wc-gocardless-gateway.php

    • Enhanced _process_payment_event() method to handle race conditions
    • Added status transition logic for failed → paid_out and cancelled → paid_out scenarios
    • Improved logging to track current order status during webhook processing

  • Added: tests/phpunit/test-webhook-race-condition.php

    • Unit tests for all race condition scenarios
    • Tests verify proper status transitions and method calls
    • Ensures backward compatibility with normal processing flow

Testing

  • ✅ Unit tests cover all scenarios (failed→paid_out, cancelled→paid_out, normal flow)
  • ✅ No linting errors introduced
  • ✅ Backward compatible with existing functionality
  • ✅ Comprehensive logging for debugging

Fixes

#7

@sidshas03
Fix webhook race condition where paid_out event arrives after failed …
a0818e8 
…event

- Fixes issue gocardless#7: gocardless#7
- Handle case where paid_out/confirmed events arrive after order is marked as failed
- Transition order status from failed/cancelled to processing before payment_complete()
- Add comprehensive logging for debugging race conditions
- Include unit tests for all scenarios (failed->paid_out, cancelled->paid_out, normal flow)
- Maintain backward compatibility with existing functionality

The fix ensures orders are properly marked as completed when GoCardless
webhook events arrive out of order due to network delays or retries.
@jeffpaul jeffpaul moved this from Triage to Code Review (10up) in 10up GC WooCommerce Project Sep 30, 2025
@jeffpaul jeffpaul added this to the 3.0.0 milestone Sep 30, 2025
@jeffpaul jeffpaul requested a review from dkotter September 30, 2025 15:10
@jeffpaul jeffpaul linked an issue Sep 30, 2025 that may be closed by this pull request
Possible problem where a paid_out event arrives after a failed event #7
Open
@jeffpaul jeffpaul modified the milestones: 3.0.0, 3.0.1 Sep 30, 2025
dkotter
dkotter reviewed Oct 1, 2025
View reviewed changes
includes/class-wc-gocardless-gateway.php
Comment on lines +1905 to +1911
if ( 'failed' === $current_status ) {
wc_gocardless()->log( sprintf( '%s - Order #%s is in failed status, transitioning to processing before payment_complete()', __METHOD__, $order->get_order_number() ) );
$order->update_status( 'processing', __( 'Payment received after initial failure - updating status', 'woocommerce-gateway-gocardless' ) );
} elseif ( 'cancelled' === $current_status ) {
wc_gocardless()->log( sprintf( '%s - Order #%s is in cancelled status, transitioning to processing before payment_complete()', __METHOD__, $order->get_order_number() ) );
$order->update_status( 'processing', __( 'Payment received after cancellation - updating status', 'woocommerce-gateway-gocardless' ) );
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamdharmesh Code here looks fine to me but curious for your thoughts on this, if there's any scenario where we wouldn't want to change a failed or cancelled order to processing?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @sidshas03.

@dkotter I don’t see an issue with changing a failed or cancelled order to processing. However, I’m not sure what we are fixing here. We already have a $order->payment_complete() call after these changes, so the order status will be updated to processing, and we don’t need to update it explicitly here. Also, updating it to processing directly will skip the $order->payment_complete() logic due to the internal check, and it will also skip updating the transaction ID in the order.

where a paid_out event arrives after a failed event, causing orders to remain in failed status even though the payment was successful.

@sidshas03 Could you please provide detailed steps to reproduce the issue? I’ve tried running different scenarios here, but I’m not able to reproduce it on trunk. Any help in reproducing the issue would be greatly appreciated.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkotter dkotter dkotter left review comments

@iamdharmesh iamdharmesh iamdharmesh left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@sidshas03 sidshas03

Labels
None yet
Projects
None yet
Milestone
2.9.9
Development

Successfully merging this pull request may close these issues.

Possible problem where a paid_out event arrives after a failed event
4 participants
@sidshas03 @dkotter @iamdharmesh @jeffpaul