-
Notifications
You must be signed in to change notification settings - Fork 153
chore: paypal unnecessary log remove #1635
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.
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
chore: paypal unnecessary log remove #1635
Conversation
WalkthroughAll diagnostic logging statements invoking Changes
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
Lib/Gateway/Paypal.php (2)
441-475: Webhook re-throw breaks mandatory 200-OK acknowledgement
handle_webhook_request()promises to always acknowledge PayPal (http_response_code( 200 ) …) even on failure.
The re-throw inside thecatch(l. 468) short-circuits that guarantee—execution jumps out of the method and the 200 never reaches PayPal, causing repeated retries.- } catch ( \Exception $e ) { - throw new \Exception( 'Webhook processing failed: ' . $e->getMessage() ); + } catch ( \Exception $e ) { + // Do not re-throw – we still need to ACK PayPal. + $acknowledged = false; + // Optional: error_log( 'PayPal webhook failed: ' . $e->getMessage() ); }
731-805: Local subscription state is not updated when remote cancellation failsInside
cancel_subscription()the innertry { … } catch(l. 761-793) re-throws an exception.
As a consequence, the “Update local subscription status regardless of PayPal API result” block (l. 797-819) is skipped whenever the remote call errors—contradicting the comment and leaving user meta/table rows stale.- } catch ( \Exception $e ) { - throw new \Exception( 'PayPal cancellation failed: ' . $e->getMessage() ); + } catch ( \Exception $e ) { + // Remote call failed – record locally and surface the error later. + $remote_error = $e; } ... - return true; - } catch ( \Exception $e ) { - throw $e; + return true; + } catch ( \Exception $e ) { + // Preserve previous behaviour of surfacing errors if desired + throw $e; }Alternatively, move the update logic into a
finally { … }block.
Failing to fix this will leave users with an active PayPal subscription marked as “active” in WPUF after PayPal already cancelled billing.
🧹 Nitpick comments (2)
Lib/Gateway/Paypal.php (2)
458-463: Stale comment references removed loggingThe line
// Log the event type(l. 461) is now misleading—there is no log anymore. Remove or re-phrase.- // Log the event type + // Event type verified – proceed
1473-1477: Clean up leftover “debug logging” commentsThere are still comments signalling debug logging at:
prepare_to_send()– l. 1475process_subscription_payment()– l. 951They add noise now that all
WP_User_Frontend::log()calls are gone.- // Add debug logging - // Create order + // Create PayPal order(and remove the similar comment around l. 951)
Also applies to: 950-952
Summary
Removed all 54 instances of
\WP_User_Frontend::log()calls from the PayPal gateway file while ensuring:Key Areas Where Log Statements Were Removed:
The file now operates exactly the same way functionally, but without any logging output to
\WP_User_Frontend::log(). All error handling, try-catch blocks, and business logic remain intact.Summary by CodeRabbit