Skip to content

Fix WC Memberships: null guard and retry queue cleanup #838#896

Merged
zahardev merged 2 commits intodevelopfrom
version/3.16.0-fix-wc-subscription-renewals
May 4, 2026
Merged

Fix WC Memberships: null guard and retry queue cleanup #838#896
zahardev merged 2 commits intodevelopfrom
version/3.16.0-fix-wc-subscription-renewals

Conversation

@zahardev
Copy link
Copy Markdown
Collaborator

@zahardev zahardev commented May 4, 2026

Summary

  • Guard sync_membership() against null membership lookups to prevent fatal errors when get_user_membership() returns null
  • Clear the SINGLE_SYNC_DATA_OPTION queue when the 10-attempt retry cap is reached, so stranded users don't block future syncs

Test plan

  • Verify WC Memberships single sync still works for active/revoked memberships
  • Verify that after 10 failed sync attempts the queue option is cleaned up
  • Run WPUnit WCMembershipsIntegratorTest suite

Summary by CodeRabbit

  • Bug Fixes
    • Prevented errors when a membership lookup fails by stopping the sync and logging the failure.
    • Reconciled capped/queued membership sync snapshots with current state to avoid losing pending changes.
    • Reset retry counters and reschedule single-item syncs when needed to preserve concurrent updates and avoid indefinite retry loops.

sync_membership() guards against null membership lookups.
process_single_sync() clears stale queue at retry cap instead of
silently abandoning stranded users.

Co-Authored-By: Claude via AIContext
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f8ec9e76-98e1-418d-805d-55b56c7d5944

📥 Commits

Reviewing files that changed from the base of the PR and between 0bd7656 and 103d139.

📒 Files selected for processing (1)
  • php/classes/integrations/woocommerce/class-wc-memberships-integrator.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • php/classes/integrations/woocommerce/class-wc-memberships-integrator.php

📝 Walkthrough

Walkthrough

Adds defensive handling in the WooCommerce membership synchronizer: sync_membership() returns early on failed membership lookup; process_single_sync() reconciles the capped snapshot with the DB when attempts reach the cap and only deletes or reschedules the queue based on reconciliation results.

Changes

Membership Sync Error Handling & Reconciliation

Layer / File(s) Summary
Lookup Validation
php/classes/integrations/woocommerce/class-wc-memberships-integrator.php
sync_membership() now checks for a falsy membership result, logs "Membership lookup failed", and returns early to avoid dereferencing an invalid $membership.
Retry-cap Reconciliation (core)
php/classes/integrations/woocommerce/class-wc-memberships-integrator.php
process_single_sync() replaces the previous unconditional deletion on attempts >= 10 with a call to reconcile_sync_data($snapshot, $empty_pending) to reconcile the capped snapshot against current DB state.
Retry-cap Persistence / Reschedule (wiring)
php/classes/integrations/woocommerce/class-wc-memberships-integrator.php
If reconciliation yields no remaining users, it deletes SINGLE_SYNC_DATA_OPTION; otherwise it resets attempts to 0, updates the option, and calls schedule_single_sync(0) to continue processing concurrent additions.
Tests / Docs
(no files changed)
No test or documentation files were modified in this PR.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A careful hop, a watchful eye,
I check each membership lest it lie.
When retries pile up to ten and more,
I reconcile — then clean or schedule for more.
A tidy burrow, syncs kept bright,
Hooray for order, and code done right.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly summarizes the main changes: a null guard added to sync_membership() and improvements to retry queue cleanup logic when the 10-attempt cap is reached.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch version/3.16.0-fix-wc-subscription-renewals

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PHPStan (2.1.51)

PHPStan was skipped because the sandbox runner could not parse its output.


Review rate limit: 2/5 reviews remaining, refill in 27 minutes and 18 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@php/classes/integrations/woocommerce/class-wc-memberships-integrator.php`:
- Around line 287-290: When the retry cap is reached in
class-wc-memberships-integrator (the block using self::SINGLE_SYNC_DATA_OPTION),
do not unconditionally call delete_option which can remove concurrent entries
added by prepare_single_sync(); instead load the current option with
get_option(self::SINGLE_SYNC_DATA_OPTION, []), compute the difference between
the current stored users and the stranded $snapshot['users'] (remove only the
users that belong to the capped snapshot), update the option with the remaining
users (and preserve other metadata), and only delete the option if no users
remain; keep the logger->log call but remove the unconditional delete_option
call and replace it with update_option when needed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b18532c5-af76-4846-801b-b0aebfb38883

📥 Commits

Reviewing files that changed from the base of the PR and between df6baa0 and 0bd7656.

📒 Files selected for processing (1)
  • php/classes/integrations/woocommerce/class-wc-memberships-integrator.php

Reuse reconcile_sync_data() to drop only capped snapshot users
instead of unconditionally deleting the queue option.

Co-Authored-By: Claude via AIContext
@zahardev zahardev merged commit 11ea64e into develop May 4, 2026
2 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request May 4, 2026
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.

1 participant