Fix PayPal checkout loop and improve error handling #305
Conversation
## Problem When PayPal returns an error (e.g., error 10002 "Security header is not valid" due to invalid API credentials), the checkout would silently loop back to the /register page instead of showing an error message to the user. ## Root Causes Fixed 1. **get_checkout_details() not checking ACK=Failure**: The function only checked HTTP status code (200 OK) but not the PayPal ACK field. When API credentials were invalid, PayPal would return HTTP 200 with ACK=Failure, which was being treated as a valid response. 2. **confirmation_form() returned errors instead of displaying them**: When an error occurred, the function returned an error string but the calling code used output buffering and ignored return values, so errors were never shown. 3. **Missing validation for required data**: No validation for pending_payment, customer, or membership objects before trying to render the confirmation form. ## Changes ### get_checkout_details() - Added ACK=Failure check to detect PayPal API errors - Returns WP_Error on API failures instead of partial data - Added comprehensive logging for all error conditions ### confirmation_form() - Changed return type from string to void - Uses wp_die() to display errors instead of returning strings - Added validation for pending_payment, customer, and membership - Added error logging ### process_confirmation() - Added proper WP_Error handling for get_checkout_details() response - Improved all error messages with better context - Added logging for all error conditions - Uses proper wp_die() calls with back_link option ### create_recurring_profile() & complete_single_payment() - Added logging for API call start, success, and failure - Added ACK=FailureWithWarning check (not just Failure) - Improved error messages to include PayPal error code and message - Fixed HTTP response code from 401 to 200 for proper back links ## Result Users now see clear error messages instead of being silently redirected. All PayPal API errors are logged for debugging. Fixes #193 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe PR enhances the PayPal gateway error handling and logging throughout the payment confirmation flow. It adds comprehensive error detection for API failures, improves user-facing error messages, integrates debugging logs across confirmation and payment processing, updates the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
inc/gateways/class-paypal-gateway.php (1)
848-852: Consider using integer for HTTP response code.The
responseparameter inwp_die()expects an integer. While the string'200'works (WordPress casts it), using an integer200would be more conventional and explicit.This pattern appears in multiple
wp_die()calls throughout the file (lines 848, 862, 885, 906, 927, 1351, 1381, 1477, 1558, 1580, 1658, 1687, 1705, 1724).🔎 Example fix (apply to all occurrences)
- 'response' => '200', + 'response' => 200,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
inc/gateways/class-paypal-gateway.php
🧰 Additional context used
🪛 PHPMD (2.15.0)
inc/gateways/class-paypal-gateway.php
1802-1802: Avoid unused local variables such as '$custom'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: PHP 8.4
- GitHub Check: Build Plugin for Testing
- GitHub Check: PHP 8.1
🔇 Additional comments (7)
inc/gateways/class-paypal-gateway.php (7)
833-866: LGTM!Good addition of WP_Error handling. The error detection for both
WP_Errorobjects and empty/invalid responses covers the main failure scenarios. Logging withLogLevel::ERRORand usingwp_die()withback_linkprovides clear user feedback and debugging capability.
878-931: LGTM!Consistent error handling for missing required objects (payment, cart, membership, customer). Each failure path now logs the error and provides a user-friendly message with a back link, addressing the original issue where errors were silently swallowed.
1333-1397: LGTM!Comprehensive error handling for
CreateRecurringPaymentsProfile. The changes properly:
- Log before initiating the request
- Handle
WP_Errorfromwp_remote_post- Check for both
failureandfailurewithwarningACK values- Extract and display PayPal error codes/messages
- Log success with transaction details for debugging
1469-1480: LGTM!Good addition of logging for unexpected HTTP responses before the
wp_die()call. This will help diagnose edge cases where PayPal returns non-200 status codes.
1532-1662: LGTM!The
complete_single_paymentmethod now has consistent error handling that mirrorscreate_recurring_profile. The addition of logging at entry, for failures, and on success provides good observability for debugging payment issues.
1668-1738: LGTM!The
confirmation_formmethod now properly validates all required objects before rendering the template:
- Validates checkout details response
- Checks for pending payment existence
- Verifies customer and membership data
The return type change to
voidis appropriate since the method now useswp_die()for errors instead of returning error strings.
1779-1790: LGTM - Core fix for the checkout loop issue.This is the critical fix addressing the root cause from Issue #193. The method now properly checks for
ACK=FailureandACK=FailureWithWarningresponses and returns aWP_Errorinstead of treating the response as valid. This prevents the silent loop back to/register.
| $body['pending_payment'] = $pending_payment; | ||
|
|
||
| $custom = explode('|', (string) $body['PAYMENTREQUEST_0_CUSTOM']); | ||
| $custom = explode('|', (string) wu_get_isset($body, 'PAYMENTREQUEST_0_CUSTOM', '')); |
There was a problem hiding this comment.
Remove unused variable.
The $custom variable is assigned but never used. This appears to be dead code.
🔎 Proposed fix
- $custom = explode('|', (string) wu_get_isset($body, 'PAYMENTREQUEST_0_CUSTOM', ''));
-
return $body;Based on static analysis hint from PHPMD.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $custom = explode('|', (string) wu_get_isset($body, 'PAYMENTREQUEST_0_CUSTOM', '')); | |
| return $body; |
🧰 Tools
🪛 PHPMD (2.15.0)
1802-1802: Avoid unused local variables such as '$custom'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In inc/gateways/class-paypal-gateway.php around line 1802, the variable $custom
is assigned from explode(...) but never used; remove that unused assignment line
to eliminate dead code and satisfy PHPMD static analysis.
Summary
Fixes the issue where PayPal checkout would silently loop back to the
/registerpage instead of showing an error message when PayPal API calls fail (e.g., error 10002 "Security header is not valid" due to invalid credentials).Root Causes Fixed
get_checkout_details()not checking ACK=Failure - The function only checked HTTP status code (200 OK) but not the PayPal ACK field. When API credentials were invalid, PayPal would return HTTP 200 with ACK=Failure, which was being treated as valid data.confirmation_form()returned errors instead of displaying them - When an error occurred, the function returned an error string, but the calling code used output buffering and ignored return values, so errors were never shown to users.Missing validation for required data - No validation for
pending_payment,customer, ormembershipobjects before trying to render the confirmation form.Changes
get_checkout_details(): Added ACK=Failure check to detect PayPal API errors, returns WP_Error on failures instead of partial data, added comprehensive loggingconfirmation_form(): Changed return type from string to void, useswp_die()to display errors, added validation for required objectsprocess_confirmation(): Added proper WP_Error handling, improved error messages with better context, added logging for all error conditionscreate_recurring_profile()&complete_single_payment(): Added logging for API call lifecycle, added ACK=FailureWithWarning check, improved error messages to include PayPal error code and messageResult
Users now see clear error messages instead of being silently redirected back to the checkout page. All PayPal API errors are logged for debugging.
Test plan
/wp-admin/network/admin.php?page=wp-ultimo-logs&tab=paypal) for proper loggingFixes #193
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.