Circular reference error when used with "String" module from contrib

I feel it's not really a bug in this module. It comes from a combination of the 2 modules. Either one alone works fine.

AFAICS StringManager calls ThemeHandler which gets config which calls MailerConfigOverride which calls EmailBuilderManager which depends on entity_type.manager.

Where is the circle?? I guess that somehow StringManager has registered in a way that makes it called from entity_type.manager. If you can change that then the problem goes away.

I'm not sure there's much we can change in this module.

@adamps,

I feel it's not really a bug in this module. It comes from a combination of the 2 modules. Either one alone works fine.

Thanks for the response. The problem could be a bug in Drush or Core.

AFAICS StringManager calls ThemeHandler which gets config which calls MailerConfigOverride which calls EmailBuilderManager which depends on entity_type.manager.

Where is the circle?? I guess that somehow StringManager has registered in a way that makes it called from entity_type.manager. If you can change that then the problem goes away.

The circle is happening over here

string module includes theme handler https://git.drupalcode.org/project/string/-/blob/1.1.x/src/StringManager...

and when symfony_mailer loads plugin.manager.email_builder in MailerConfigOverride https://git.drupalcode.org/project/symfony_mailer/-/blob/1.x/src/Process...
.

I was investigating this issue and even though I don't understand 100%, I see that this circular dependency is being triggered due to config override depending on services that could have dependency on configuration.

However, what if we refactor the MailerConfigOverride to read from a Drupal State instead of calling `plugin.manager.email_builder` ?

So the idea for breaking the loop being,

- After Plugin Discovery, write the overrides into Drupal State.
- While overriding via Module, read the overrides from Drupal State.

This could lead to module override reading from a stale or empty state for some edge cases. But since the override is via module, the effect should be quite minimal.

This should allow you to use Dependency injection in your classes as well. What do you think?

I agree that the code in this module is dangerous for causing circles. However it doesn't cause any alone. I don't really understand why that code in "String" module is enough to trigger a circle. Perhaps the exception message about the circle isn't complete?

MailerConfigOverride already has code that tries to avoid a circle. It ignores any config requests before module hander is loaded:

if (!$this->builtCache && $this->moduleHandler->isLoaded()) {

Indeed I was trying to wait until "After Plugin Discovery" but I couldn't see how to test that. I believe we don't need to write anything to state. We can just change the check on module handler to a better one that happens later on in the bootstrap sequence.

@adamps,

Thanks for quick reply. Actually, the problem is not with the module handler call. As you can see in the fix that i pushed in string module to temporary unblock deployment, https://git.drupalcode.org/project/string/-/commit/f78038a7f26d82c56fbb7... , the circular dependency is caused when $theme_directories = $this->themeHandler->getThemeDirectories(); is executed. And the themeHandler doesn't have the equivalent isLoaded method like module handler does. So not sure how to introduce a similar check for theme handler.

I (maintainer of string module) would like to avoid catching that exception in string module as it can lead to un-expected consequence that plugins in theme folder would never work when symfony_mailer is present in the website.

Here's a patch for 2.x that worked for me.

It's not really systematic😃. We already waited for module handler to load. Now we also wait for EntityTypeManager. Maybe in the future we would have to add another one. However it works for now on my site at least.

I'm going to make a release soon, so I'll commit this now. Please do test and let me know when you have a chance.

My projects are running on 1.x. I had problem upgrading to 2.x and got distracted and ended up not able to test it. I will try to pick this up later this week.

Meanwhile, I tried to apply the changes to corresponding file in 1.x and it didn't resolve the issue though.

OK thanks