Problem/Motivation
On 1.0.x since #3618740: Add a dunning framework: past_due state, scheduled retries, grace period, and lifecycle notifications, any site that also runs Symfony Mailer fails to build its container:
Circular reference detected for service "subscription_manager", path: "subscription_manager -> Drupal\subscription_manager\Hook\SubscriptionManagerHooks".
Symfony Mailer registers a config override that, the first time any config is loaded, discovers legacy mail implementations by resolving every module's hook_mail; resolving an OOP hook instantiates its whole class. That behavior is a known source of circular references (#3564298: Circular Dependency error with eca_content in Drupal 11.3 caused by 1.6.2 in the Symfony Mailer queue, currently postponed with the expectation that affected modules adapt), so this module has to coexist with it. Two things here combine to trigger it: SubscriptionManagerService loads its settings in its constructor, and hook_mail — added with the optional dunning mails — was placed on SubscriptionManagerHooks, whose constructor depends on the subscription_manager service. The hook is therefore resolved while that service is mid-construction. Every request fails, including drush.
The kernel suite did not catch it because Symfony Mailer is not part of the test environment. 1.0.0-beta6 is unaffected; only 1.0.x since the dunning merge.
Steps to reproduce
Install Symfony Mailer (1.6.x) alongside subscription_manager 1.0.x and rebuild caches.
Proposed resolution
- Read the module settings on use rather than in
SubscriptionManagerService's constructor. This is a defect in its own right, independent of Symfony Mailer: constructor-time config loading is what makes the service a participant in any such cycle (any config override that touches services or hooks could trigger it), and it also let the settings go stale when changed mid-request. - Move
hook_mailto its own hook class with no service dependencies, documented as needing to stay that way. This part is an accommodation of Symfony Mailer's behavior, and sound hygiene under Drupal 11.3 hook resolution generally: a hook known to be resolved early should not pull in a large dependency graph. - Regression guard: a config override in the test module that resolves
hook_mailimplementations while config loads, exactly as Symfony Mailer does, so every kernel test using that module fails loudly if the combination ever returns. (It reproduces the failure in all 21DunningTesttests before the fix.)
Remaining tasks
Review and merge. No change record: no API change, and the affected code is unreleased.
User interface changes
None.
API changes
None. (SubscriptionManagerService's protected $config property is replaced by a protected settings() method.)
Data model changes
None.
Issue fork subscription_manager-3624585
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
Comment #5
colanFix in the MR, as one commit, exactly as proposed in the summary:
SubscriptionManagerServicereads its settings on use through a protectedsettings()method instead of loading config in its constructor (reusing the class's existing, previously unset$configFactoryproperty; the$configproperty is gone).hook_mailmoves fromSubscriptionManagerHooksto a newDunningMailHooksclass with no service dependencies, documented as needing to stay that way. It is picked up by core's automatic hook-class registration, so no service definition is needed.MailHookInvokingConfigOverridein the test module, taggedconfig.factory.override, resolves everyhook_mailimplementation while config loads — what Symfony Mailer's override does. With the guard in place and the fix absent, all 21DunningTesttests failed with the production error; with the fix, the suite is green (129 tests), and the affected live site builds its container again.Upstream context added as a data point on #3564298: Circular Dependency error with eca_content in Drupal 11.3 caused by 1.6.2 in the Symfony Mailer queue. No change record: no API change, and the affected code is unreleased (1.0.0-beta6 predates it).