Problem/Motivation

_subscription_manager_sync_remote_subscriptions() exists to self-heal the case where a remote subscription contract exists but the local subscription entity is missing (for example a webhook that never fired). It runs on hook_user_insert() and when cron drains the subscription_manager_remote_sync queue populated at login.

The function only ever queries the connector named in default_connector. On a site with more than one connector, a missed webhook on any non-default connector is never reconciled: the user's remote contract is active, but no local entity is created, no roles are granted, and every login re-enqueues a sync job that checks the wrong service. This contradicts the multi-connector design of the rest of the module, where syncPlans() already iterates all installed connector definitions.

Steps to reproduce

  1. Install connectors A and B; set default_connector to A.
  2. Create a remote subscription on connector B for an email address with no local subscription entity (simulating a missed webhook).
  3. Register or log in as that user and run cron.
  4. No local subscription is created; connector B was never consulted.

Proposed resolution

Iterate all installed connector definitions, mirroring SubscriptionManagerService::syncPlans():

  • Keep the cheap local short-circuit (an active local subscription skips all remote calls).
  • For each installed connector, call loadRemoteSubscriptionsByUser() and sync any results via syncRemoteSubscriptionToLocal(), wrapping each connector in its own try/catch so one provider's outage doesn't block reconciliation against the others.
  • Optionally stop after the first connector that yields results. This short-circuit is only valid if the single-active-subscription invariant from #3616778: Define and enforce single-active-subscription semantics (loadLocalSubscription() returns an arbitrary first match) is adopted (its Option A); if concurrent subscriptions are allowed there, every connector must always be consulted.

Remaining tasks

User interface changes

None.

API changes

None if implemented in the internal helper. If #3616880: Move _subscription_manager_sync_remote_subscriptions() into SubscriptionManagerService and convert the cron queue drain to a QueueWorker plugin lands first, the change lands in the public SubscriptionManagerService::syncRemoteSubscriptions() method instead, still with no signature change.

Data model changes

None.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

colan created an issue. See original summary.

colan’s picture

Priority: Normal » Major
colan’s picture

colan’s picture

Status: Active » Needs review

MR opened, implementing the proposed resolution on top of the #3616880: Move _subscription_manager_sync_remote_subscriptions() into SubscriptionManagerService and convert the cron queue drain to a QueueWorker plugin refactor (the change lands in SubscriptionManagerService::syncRemoteSubscriptions(), no signature change):

  • Every installed connector is consulted — the default connector first (the most likely home of the contract), then the rest in sorted order for determinism. The cheap active-local short-circuit is unchanged.
  • Stop-after-first-yield is adopted, per the condition in the summary: #3616778: Define and enforce single-active-subscription semantics (loadLocalSubscription() returns an arbitrary first match) landed with Option A, so the first connector yielding remote subscriptions is authoritative and the remaining connectors are skipped.
  • Per-connector isolation: each connector runs in its own try/catch, so one provider's outage cannot block reconciliation against the others. Failure semantics compose with the #3616880: Move _subscription_manager_sync_remote_subscriptions() into SubscriptionManagerService and convert the cron queue drain to a QueueWorker plugin contract: if a later connector heals, the earlier failure is only logged (a retry would no-op against the now-active local subscription anyway); if nothing was synced anywhere and a connector failed, the first failure is rethrown so the QueueWorker path keeps core's retry handling while hook_user_insert() stays fail-open.
  • Kernel tests (MultiConnectorSyncTest): the reproduce case — a contract on a NON-default connector is now healed with the correct connector_plugin_id recorded, with both connectors provably consulted (RED before the fix); iteration stops at the first yielding connector; an outage on the default connector does not block healing via another; and a failure with nothing synced rethrows after still consulting the remaining connectors. The test connectors gained a consultation counter and a simulated-outage flag to drive these.

Change record for the stop-after-first behavior drafted alongside, as the summary requested.

  • colan committed 9a49509d on 1.0.x
    Issue #3616770: Consult every installed connector in the self-healing...
colan’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.