Problem/Motivation

Under certain circumstances the module files are not yet loaded.

This happens for example when invoking a hook inside the constructor of a http_middleware service; theses services are constructed very early as a dependency of http_kernel service.

A more concrete example is a middleware service using the entity_type.manager. Most of the times the entity type information is retrieved from cache (stored in the discovery cache bin). When this cache however is missing, hooks like hook_entity_type_build() and hook_entity_type_alter() need to be invoked at this early stage. The modules files however haven't been loaded yet, making ModuleHanlder::verifyImplementations() skip all the hook implementations (silently!). The entity definition cache is then incomplete resulting in various hard to debug (next to impossible) exceptions like #3031598: The "paragraph" entity type did not specify a translation handler. .

Steps to reproduce

Taken from this comment.

  1. Just uninstalled the dblog module from UI here /admin/modules/uninstall
  2. Then open any menu /admin/structure/menu/manage/[menu-name] and I see an error:
    Uncaught PHP Exception Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException: "The "menu" entity type did not specify a "edit" form class." at /core/lib/Drupal/Core/Entity/EntityTypeManager.php line 211
  3. Then click to Flush views cache on another admin page (Flush all caches > Flush views cache via admin menu).
  4. After #3 I can see the error:
    Uncaught PHP Exception Symfony\Component\Routing\Exception\RouteNotFoundException: "Route "entity.path_alias.collection" does not exist." at /core/lib/Drupal/Core/Routing/RouteProvider.php line 206

Why does it happen?

After #3, a lot of items were removed from router table from the database. You have to launch drush cr twice to fix such error.

Proposed resolution

If a hook is invoked before the modules being loaded, load all modules.

Remaining tasks

Update issue summary
Address #12 and #17

Issue fork drupal-3207813

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

casey created an issue. See original summary.

casey’s picture

Status: Active » Needs review
StatusFileSize
new1.28 KB
bhumikavarshney’s picture

StatusFileSize
new44.68 KB

Hi @casey ,
Patch applies clearly and works fine for me.
Now the modules files loaded easily.
Thanks.

sajiniantony’s picture

Hi Casey,

I have applied the patch, the site worked fine. But at a later stage we are getting the same error The "paragraph" entity type did not specify a translation handler. in Drupal\Core\Entity\EntityTypeManager->getHandler().We are using Drupal 8.9.10 and paragraph 8.x-1.12. Also it is a multilingual site. We do have contents with paragraph but translation is not enabled as it does not support translations
Do you have any suggestions? Thanks

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

joel_osc’s picture

Thank-you everyone for your work on this issue! I saw both the paragraph error and a problem where tokens were not working because the token hooks were skipped and token_module_implements_alter then would fail to add the core tokens that it provides like book, field, etc. Banged my head on it for a few hours until I found this issue and patch.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

dmitriy.trt’s picture

StatusFileSize
new3.73 KB
new1.85 KB

We experienced a very similar issue even with #2 patch applied. It turned out, the patch doesn't cover all the cases.

All of the following conditions must be met in order to trigger it with #2 patch applied:

  • hook_info cache entry exists,
  • module_implements cache entry is missing,
  • a call to ModuleHandler::getImplementations() happens before all the modules are loaded.

In this case the ModuleHandler::buildImplementationInfo() starts the discovery without loading the module files. In our case it also resulted in empty list of implementations for the hook_entity_type_build() and with External Entity module it produced "The ... entity type does not exist." error. And since the list of hook implementations is then written to cache, this breaks the website with that error for everyone until the next cache rebuild.

Worth mentioning that it was reproducible on a high traffic website during a cache rebuild executed from drush. Drush commands don't call the ModuleHandler::writeCache(), since they don't dispatch the kernel.terminate event. That seems to increase the chances and may be a separate bug, but it's minor. In this scenario it happened in 1 out of 3-5 cache rebuilds leaving the website broken. But even without that, it could be reproducible via multiple parallel requests, since the hook_info cache is written immediately after generation, while the module_implements cache entry is only written at the end of the request and there is plenty of time for another request to get into that trap.

Attaching patches with tests only and with the fix + tests. Hope that's fine to move the code of #2 to a different method.

I should explain the line removed from existing test. There is no actual reason why the test must load the module here. Actually, it looks like a workaround that hides the bug fixed by #2. Please let me know if that's not correct.

Status: Needs review » Needs work

The last submitted patch, 8: 3207813-8-tests-only.patch, failed testing. View results

dmitriy.trt’s picture

Status: Needs work » Needs review

That's a test-only failure as expected, so setting "Needs review" back.

anybody’s picture

Thank you @casey, this looks important and interesting as, as you wrote, it potentially might also have an impact on other issues like #3056633: It is not possible to uninstall a module that provides a filter plugin via config import.

This happens for example when invoking a hook inside the constructor of a http_middleware service; these services are constructed very early as a dependency of http_kernel service.

Are there other scenarios you could imagine to also cause this?

Could you list some examples of modules which do this? Looking through our stack I found the following modules with a http_middleware service, unsure if they invoke hooks:

  • devel webprofiler
  • remove_http_headers
  • shield
catch’s picture

This is a very old issue. The short answer is that middlewares shouldn't be invoking hooks, there might be a later kernel event listener that would allow you to achieve the same thing.

This might be a duplicate of #1905334: Only load all modules when a hook gets invoked which was trying to load hooks when a hook is invoked instead of at a particular point in bootstrap.

The other is to do #2237831: Allow module services to specify hooks, and deprecate procedural hooks (in Drupal 11 or 12 though).

catch’s picture

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

peterwcm’s picture

StatusFileSize
new3.74 KB

Thank you @dmitriytrt for your patch. It was working fine with 9.3.x but it couldn't apply on 9.4.0 due to changes in ModuleHandlerTest I believe. getImplementations is also deprecated since 9.4.x so I have made some changes to the test.

dieterholvoet’s picture

Status: Needs review » Reviewed & tested by the community

I also encountered the issue after clearing caches through the admin interface. Some hook_entity_type_build implementations were not called, causing errors indicating missing entity types. I found this issue after reporting my findings at #3107252: Missing plugin notice after every merge and cache clear, cache_discovery table. The latest patch fixes the issue for me.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

I think that #12 implies that this solution is not really the correct solution.

I think the issues around the caches getting written at different times is a real one and one that we should look to fix in a separate issue. The hook_info cache write should be written in \Drupal\Core\Extension\ModuleHandler::writeCache() and not in \Drupal\Core\Extension\ModuleHandler::getHookInfo().

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

thejimbirch’s picture

Just wanted to comment that I searched my code for the hooks mentioned in the initial description and the ctools_entity_mask implements hook_entity_type_alter. After uninstalling ctools_entity_mask my error went away.

heddn’s picture

We're seeing this on a handful of sites and I was pointed here by an associate. I'll apply the patch in #15 for now and report back if our random cache poison issues stop appearing.

heddn’s picture

Also of note, the 2 sites facing the issue are using chained fast cache (redis and memory) and are hosted on Pantheon.

heddn’s picture

Status: Needs work » Needs review

Random cache poisoning is now decidedly fixed with the patch applied. I'm not sure what are the next steps here, so moving back to NR to get some feedback. As the patch fixes a very large problem on 2 active and large websites.

mrconnerton’s picture

Understanding that via #12 / #17 this patch might not be the final / "right way" to solve this issue, I wanted to add my feedback that the patch in #15 did resolve our eck issue @DieterHolvoet describes in #3107252-18: Missing plugin notice after every merge and cache clear, cache_discovery table and echo Lucas's comment in #22 that this could use eyes/feedback again to see what the next steps are.

joseph.olstad’s picture

Patch 15 has test,
tests only patch fails as expected (#8),
patch with tests passes
patch reported to fix the issue
patch 15 applies cleanly on the latest HEAD of:

  • 10.1.x (with fuzz clean)
  • 10.0.x (with fuzz clean)
  • 9.5.x (clean)
  • 9.4.x (clean)

@TODO: figure out how to write a version of the patch to satisfy #17 and #12

smustgrave’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs issue summary update

Issue summary should be updated with steps to reproduce.

Added addressing #12 and #17 to remaining tasks.

solideogloria’s picture

joseph.olstad’s picture

StatusFileSize
new3.74 KB

patch 15 had fuzz, removing fuzz to be able to try testing with newer versions.

straight up reroll, no interdiff, just fuzz removal.

joco_sp’s picture

#27 worked for me also on core 9.5.5.
Thank you all for making that patch, because it solved a big issue :D If I see the error reappearing, I'll report it here.

solideogloria’s picture

Was unable to add a new view suddenly for some reason.

"view" entity type did not specify a "add" form

Patch #27 fixed the issue.

alexpott’s picture

This is a important patch that makes the ModuleHandler more robust. Looking at the code I'm pretty sure that we do not need to call loadAll() from buildImplementationInfo() because we already do a reload() in buildHookInfo(). I'm pretty use we only need the call in verifyImplementations(). I think we should be adding a middleware test that calls a hook prior to the regular loadAll() call from \Drupal\Core\DrupalKernel::preHandle().

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

adamps’s picture

FYI I believe this caused #3367047: Caching bug enabling/disabling overrides. I found a fix which is to check ModuleHandlerInterface::isLoaded(), and skip any further processing if FALSE.

So yes this would be really useful to fix as a priority. As already noted, the resulting problems are pretty tricky to debug. The symptoms (to "randomly" skip calling hooks) are quite severe, potentially even security related.

berdir’s picture

I'm unsure if we should support this. Yes, it makes it more robust, but at the cost of possibly unpredictable performance issues? Forcing modules to load in a middleware will basically eliminate the reason middlewares exist in the first place.. to have very fast pre-full-bootstrap logic, like page cache.

What about throwing an exception instead if we detect this (trying to invoke a hook before a full bootstrap), so it fails consistently and reliably when adding such a hook? If you do that then you know that you have to implement things differently. And if you really really want to do this, you could still call loadAll manually? For BC, we could add a deprecation message with the loadAll() fix as an intermediate step?

adamps’s picture

I'm unsure if we should support this.

Good point. The currently behaviour is: run no hooks and cache the result. This seems clearly undesirable, however it's a good idea to think carefully about what is right.

What about throwing an exception instead if we detect this

We could try it, however It's pretty easy to trigger a hook - the call to invoke the hook could be buried within a function that we want to call. There might be places in existing middleware code that trigger a hook as a side-effect of their function. Or, like #32, places in ordinary module code that can "unexpectedly" run early in the bootstrap cycle.

We might need a way to disable the exception, i.e. run no hooks and don't cache the result.

berdir’s picture

Not running the hooks does not work IMHO. We don't have a way to pass that information back, and caching happens elsewhere. One of the most common issues related to this is incomplete entity type definitions because build/alter hooks weren't invoked. Things will be completely unpredictable then. For example, information about bundle classes might be missing and then it's not the class you'd expect to get back in your code.

And yes, hooks are typically called several steps further down, for example the entity type thing, if you load a node in a middleware for example. But an exception will fail right there and then and the stacktrace will tell you exactly which middleware and line of code is the problem. Unlike now, where it will suddenly blow up in a completely different place, for example when editing a node with paragraphs.

adamps’s picture

@Berdir, yes thanks it makes sense in general. However it seems to imply that middleware cannot do anything that triggers hooks (such as to load a node) because that would trigger the exception. Are you suggesting that this becomes a strict limitation of middleware? It could imply even that adding a new hook anytime in the future would be a BC-break because middleware might be using the code where the hook is being added. And when we say middleware, we actually mean anything that might run "early", for example implementations of ConfigFactoryOverrideInterface.

I expect that we would want a function that code can use to check if it is yet safe to use hooks.

So I was wondering if this might be too strict, and we could allow middleware to call a function to disable all hook exceptions (run no hooks and don't cache the result), or even just for a specific hook. However I can see that could be problematic because disabling some hooks could even have security implications.

berdir’s picture

There's no such thing as only loading one/some hooks, what's this about is loading the .module files of all modules so that hook discovery can figure out if a certain function exists or not.

And yes, middlewares should absolutely be as fast and slim as possible. They are specifically designed to run in very-early-bootstrap and use the decorator pattern, so they're always invoked including their dependencies. You should only use a middleware if there is a benefit to running before modules are loaded and the general bootstrap happens. For example page cache and the IP ban module, both benefit from being able to return a response as quickly as possible to achieve fast response times and minimal overhead if a certain IP attempts to send a massive amount of requests. If it doesn't need to be that fast or if you have quite a few dependencies, use a request event subscriber.

It's like hook_boot() in D7, that also only supported a limited set of API's.

For example, instead of using the config system in a middleware, you could dynamically set a parameter on it with a service provider, or only support configuration through container parameters or settings in the first place instead of config with a UI.

FleetAdmiralButter’s picture

We had been running into an issue for some time on one of our Drupal instances where performing a cache-rebuild (via Drush) while the site was serving web traffic would result in a corrupt module_implements cache.

I managed to trace the problem down to something weird happening in Shield and chanced across https://www.drupal.org/project/shield/issues/3277210. Although the patch in that issue did resolve the cache corruption, we ultimately ended up going with https://www.drupal.org/files/issues/2023-03-10/3207813-27.patch which addresses the root cause of the issue rather than patching Shield.

andysipple’s picture

I was directed to this issue from https://www.drupal.org/project/pathauto/issues/3254939

Symfony\Component\Routing\Exception\RouteNotFoundException: An exception of type 'Symfony\Component\Routing\Exception\RouteNotFoundException' with the message 'Route "entity.path_alias.collection" does not exist.' was noticed in /code/web/core/lib/Drupal/Core/Routing/RouteProvider.php on line 206
in Drupal\Core\Routing\RouteProvider::getRouteByName called at /code/web/core/lib/Drupal/Core/Routing/UrlGenerator.php (line 432)
in Drupal\Core\Routing\UrlGenerator::getRoute called at /code/web/core/lib/Drupal/Core/Routing/UrlGenerator.php (line 129)

We've been grappling with this error for some time now. It would trigger the White Screen of Death (WSOD) after deploying changes to the Pantheon live site using Redis or clearing the cache. Although it seems random, this error occurs most consistently in our high-traffic live site. We would fix it temporarily by uninstalling and installing a module or just clearing the cache several times until the WSOD clears up.

We applied the patch mentioned in #27.
No longer get the WSOD.

Fernando Iglesias made their first commit to this issue’s fork.

jeffreysmattson’s picture

After three weeks of banging our heads against the wall. With seemingly random site crashes that occurred only in production. With errors like this:

notice Uncaught PHP Exception Drupal\Core\Field\FieldException: "Attempted to create an instance 
of field with name field_last_password_reset on entity type user when the field storage does not exist." 
at {redacted}docroot/core/modules/field/src/Entity/FieldConfig.php line 315 

These errors were not only for the field_last_password_reset. They would error on all kinds of different fields that didn't have field storage, but actually did have field storage.

The site would become unavailable and a simple cache clear would resolve it... until a few hours later it would go down again.

This fixed it. Thank you!

I used #15 because we are still on Drupal 9.

joseph.olstad’s picture

re: #41,
I haven't seen "field storage doesn't exist" in relation to password_policy, or if I did see it I may have massaged the configs to resolve it. I've seen other issues such hook password policy updates that break on anything other than mysql.

***EDIT***

ah, actually we're using a version of this patch so that's probably why I didn't see the "field storage doesn't exist" error.

joseph.olstad’s picture

Still needing steps to reproduce. So far I see mention of the password_policy module in relation to this issue.

It would be nice to simplify the steps to reproduce in the wild.

"missing cache entry for hook implementations"

It would be nice to see what the performance cost of this solution is also. Looking at the code it seems like it would only be affecting a cold cache (rebuild).

catch’s picture

@joseph.olstad password policy is just an example in #41, not the cause of the issue or the only symptom.

This bug gets triggered (usually) when a middleware (which runs before modules are loaded) ends up invoking hooks, this is pretty well documented as the issue and there are lots of code paths that can trigger it. I still think #1905334: Only load all modules when a hook gets invoked is closer to the correct solution here - i.e. always load modules when hooks are invoked and don't load them until they are.

joseph.olstad’s picture

ok thanks for that, ya the other approach to this issue at
#1905334: Only load all modules when a hook gets invoked
does sound better!

berdir’s picture

> I still think #1905334: Only load all modules when a hook gets invoked is closer to the correct solution here - i.e. always load modules when hooks are invoked and don't load them until they are.

The concern I have with that issue is that it becomes very unpredictable. Yes, triggering a hook in a middleware will work, but it will also cause a considerable performance regression for sites that happen to use such a module, without any indication about that unless they do some profiling.

The current cases likely only trigger such hooks in edge cases or they would consistently fail and not work at all. But with this, you could for example load an entity in a middleware. Middlewares should be as slim as possible with as few dependencies as possible otherwise you're better off doing a request subscriber.

catch’s picture

But with this, you could for example load an entity in a middleware.

I think that might be what's happening with some of these reports - loading an entity, or especially config entity, that's usually cached, then suddenly it's not cached and the middleware loads it, triggering the bug.

casey’s picture

I think I agree with Berdir that the right solution is to throw an exception (when hooks are invoked before all modules are loaded).

Maybe we should only do this for D11, and fix it for D10 by loading the modules and logging a warning. This way we don't break existing sites that do trigger hooks in middlewares. But we will get some telemetry on which modules do need changes.

rakshith.thotada’s picture

StatusFileSize
new1.52 KB

I see if this is cache issue during middleware service request -

  • I am trying to throw an exception in that case and in exception handler -> Get the active definition which has the last successful event response.
  • Further if still it does not respond - throw the usual InvalidPluginDefinitionException
rakshith.thotada’s picture

Version: 11.x-dev » 10.1.x-dev
Status: Needs work » Needs review
StatusFileSize
new1.33 KB

Adding patch for 10.1.x version.

smustgrave’s picture

Version: 10.1.x-dev » 11.x-dev
Status: Needs review » Needs work

Was previously tagged for issue summary which still needs to happen.

Current development branch is 11.x

If you are going to upload patches please include interdiffs

nitesh624’s picture

Is there any patch available for drupal 10.0.x release? We are also running into same problem

nitesh624’s picture

I tried patch from #27 but did not resolve issue

solideogloria’s picture

I'm using patch #27 on Drupal 10.1. Everything seems to work.

nitesh624’s picture

For me this failing during blt sync command. Which actually pull down the production database to local. and it runs couple of drush command
drush sql-sync-> this ran fine without any issue
drush cr -> Here the error came "The "paragraph" entity type did not specify a translation handler."

We have the multilingual enable on site and paragraph field also available on node, which allowed to be translated via content_translation module

bobthebuilder’s picture

Drupal 10.2.3. Paragraphs 8.x-1.17. Paragraphs Library 8.x-1.17.
I'm still receiving the error message when trying to create a Paragraph. Patch #27 does not fix the issue and Patch #50 can no longer be applied with Drupal 10.2.3. Patch #50 did work for me pre-Drupal 10.2.3.

solideogloria’s picture

This issue needs a MR to be created instead of using patch files.

joseph.olstad’s picture

I re-read #44 through 47, continue on.
As discussed in #44 through #47, prioritize effort to 1905334 instead.
#1905334: Only load all modules when a hook gets invoked

solideogloria’s picture

I created a MR from #27, since that's what most people said is working. #50 and #51 seems to do something entirely different, and it also doesn't have any tests.

quimic’s picture

Patch #27 can no longer be applied on Drupal 10.2.5

jsst’s picture

For us too, patch #15 brought an end to a long running terror bug, causing spontaneous crashes due to this issue. Our site was running fine for five years, but somewhere during the upgrade to 10.2 the random cache corruption started to hit. Our http middlewares don't do anything special, something in Drupal Core made this bug worse. The bug is hard to debug because it seems to depend on concurrent usage of the site and clearing the cache can fix the bug (after running it up to 10 times) but also cause it.

So we have a major bug that is hard to debug and causes a site to go completely offline, with an unreliable fix (drush cr until it works). Seems to me we should be merging #15 in Drupal 10.2 as soon as possible.

solideogloria’s picture

Well, we shouldn't be merging a patch at all. We should merge the merge request.

The MR targets 11.x, and it could also be cherry-picked to earlier branches.

solideogloria’s picture

Issue summary: View changes
solideogloria’s picture

Status: Needs work » Needs review
Issue tags: -Needs issue summary update

I added a steps to reproduce in the issue summary that I copied from a comment. The comment said that the patches here fixed the issue, so it's at least one example.

However, there may be more ways to reproduce the problem, and the example I added requires another module. It'd be helpful if anyone can reliably reproduce the problem in another way, such that a test can be written for it.

nicxvan’s picture

Should we be using the patch in 27 as the base? I think it seems the suggestion was to throw an exception if invoking hooks in middleware?

solideogloria’s picture

An exception is already thrown and is the cause of the issue in the first place. How would throwing an exception fix the issue? Wouldn't it just break in a different place?

The patch the MR is based on fixes the issue because the information needed is already there, and it is helpful for anyone patching their code, like me.

If you want to change the approach, you'll need to open a new MR.

nicxvan’s picture

I'm not sure to be honest, I was reading comments 33+ which came after the patch in 27 saying that the solution in 27 could have unpredictable performance issues and the exception description.

I know the later patches don't have tests, but I think the concerns around comment 33 still need to be addressed.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

joegl’s picture

We cut a patch from the merge request and applied it to a Drupal 10.2.7 multisite stack with about 100+ sites. During deployment about 20% of sites were losing home page content and content on a few other pages inconsistently with no determinable pattern. We do not have Memcache enabled. The patch resolved the issues we were having.

In logs, we noticed a few errors pop up during these deployments and perusing the issues queues they all seemed to point back to this issue, including:

Route "entity.path_alias.collection" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName()
Uncaught PHP Exception Error: "Call to undefined method Drupal\Core\Entity\Entity\EntityViewDisplay::isLayoutBuilderEnabled()

I understand the methodology here isn't ideal, but it was a critical issue to resolve for us.

joseph.olstad’s picture

@joegl, please try the solution in #1905334: Only load all modules when a hook gets invoked
It is more likely to go forward than this one.

k.wiktorowicz’s picture

StatusFileSize
new1.48 KB

Patch from the comment https://www.drupal.org/project/drupal/issues/3207813#comment-15332466 working with 10.3.x version.

nicasso’s picture

StatusFileSize
new1.91 KB

Hi,

I updated patch #27 or at least the essence of it, so it applies for 10.3.5.

nicxvan’s picture

Can someone confirm if this still happens on 11.1, that method no longer even exists.

akoepke’s picture

Have applied the patch from #73 in 10.3.10 and it has resolved this issue for us.

I noticed this after enabling the IP Login module which uses Entity Type Manager in a middleware service: #3490122: Exceptions caused by middleware service using the entity_type.manager

hanan alasari’s picture

We had the same issue on drupal 10.4.3 , but could not figure out which module was causing the issue, patch #73 worked for me, thx.

berdir’s picture

Version: 11.x-dev » 10.5.x-dev

I'm fairly certain this is resolved in 11.1+, yes. Moving back to 10.5.x, there's still an option to commit there as a workaround. Instead of rerolling patches, please createa merge request, specifically against 10.5.x.

liam morland made their first commit to this issue’s fork.

liam morland’s picture

Status: Needs work » Needs review

I have created a new merge request targeting 10.5.x which is a cherry-pick of the commit in the existing merge request. All tests are passing.

liam morland’s picture

Current state of !12355.

liam morland changed the visibility of the branch 10.5.x to hidden.

nicxvan changed the visibility of the branch 3207813-modulehandler-skips-all to hidden.

nicxvan’s picture

Issue tags: +Needs issue summary update

Hiding the files and other MR. Can't review right now though.

Can someone confirm the next steps comments have been addressed and update the issue summary please?

vlyalko’s picture

#72 worked and applied perfectly for 10.2.10 Core version. Thank you for working on this issue!

igorgoncalves’s picture

Just tried to reproduce the error following the steps at summary but didnt work.

With simplytest, and Drupal 10.5.1-dev

Needs a summary update indeed.

If i find the way, i will edit the summary.

nicxvan’s picture

Component: bootstrap system » extension system
Status: Needs review » Postponed (maintainer needs more info)

I'm not sure what would have changed between 10.5 and 10.2 to fix this.

I would expect this to be fixed by 11.1

Postponing for steps to reproduce.

gregn0r’s picture

Running a site split using Domain, utilising Memcache we were digging though a lot of random errors and services to try and resolve caching issues with a large live site.

Digging deeper into what was happening we understood the combination of modules / configuration we had were invoking Drupal hooks early in the page load and that modules had not been loaded. The module_implements cache gets built incorrectly and you end up with an incomplete site cache state which can result in a multitude of errors largely revolving around missing entitles or field definitions

The issue was repeatable when the cache clear was particular slow i.e. on a large live site or done while using xDebug talking to an IDE locally, potentially triggering race conditions on the rebuild of the cache

Applying the MR patch on our 10.5 site resolved the issue, many thanks

berdir’s picture

Status: Postponed (maintainer needs more info) » Needs review

Yes, I'm certain this is still an issue.

smustgrave’s picture

Status: Needs review » Needs work

Seems there are some remaining tasks in the summary

himynameisseb’s picture

We were experiencing many different errors following cache clearing on a site under load, with the most common errors being:

- RouteNotFoundException: Route "entity.node.latest_version" does not exist.
- Error: Call to a member function getFieldStorageDefinition() on null in scheduler_content_moderation_integration_scheduler_hide_publish_date()
- InputBag::get(): Argument #1 ($key) must be of type string, null given

Putting the site in maintenance mode and doing a couple more cache clears kicked the site back in to life.

Upon digging, discovered that it was caused by the same issue that this patch fixes.

Tested and released on a site running Drupal 10.4 (10.4.8 specifically).

We have experienced this sort of "caching strangeness" in the past. I cannot say for sure if this was the cause in the past, but it could well be. It would be great to see this merged in to a future release if others experiences matches ours!

Many thanks!

kevinquillen’s picture

This seems like it needs a reroll for 11.2.

berdir’s picture

> This seems like it needs a reroll for 11.2.

No, it does not, because this problem does not happen anymore on 11.1+, this is a D10 only issue at this point.

nicxvan’s picture

Title: ModuleHandler skips all hook implementations when invoked before the module files have been loaded » [10.x] ModuleHandler skips all hook implementations when invoked before the module files have been loaded
eric_a’s picture

No, it does not, because this problem does not happen anymore on 11.1+, this is a D10 only issue at this point.

On Drupal 11.1 Rest API & JSON API 500 errors on authenticated requests with : Error: Call to undefined function \ckeditor5_module_implements_alter() in Drupal\Core\Extension\ModuleHandler->alter()

Issue went away when hacking in this:

     if (!isset($this->alterEventListeners[$cid])) {
       $this->alterEventListeners[$cid] = [];
+      $this->loadAll();
       $hook = $type . '_alter';
       $hook_listeners = $this->getHookListeners($hook);

So maybe same underlying issue?

nicxvan’s picture

Interesting, this might still be an issue then! 11.2 won't have this, but only because we removed all hook_module_implements_alter implementations.

joseph.olstad’s picture

*** EDIT ***
Please see my next comment. modules_implements_alter is indeed still included in 11.2.x.
*** END EDIT ***
@nicxvan, the API documentation for 11.x mentions that this hook will be removed in 12.0. It's not very clear what 11.2.x has to do with this. What exactly is changing in D11.2.x that would mean "won't have this?".

Example I just pulled up from one of my clients that has search_api and facets along with a custom bit of helper code in a module designed to combine two facet options as one.

My commit message for this module was squashed so I'm not even quite sure why I had the module_implements_alter however it's there and the module works as designed currently. The commit message was: "Combine article and landing_page into one single Type selection checkbox in the Content Type facet." , of course the module code is more than just the module_implements_alter.

/**
 * Ensure our alter runs last.
 */
function xyz_facets_module_implements_alter(array &$implementations, $hook) {
  if ($hook === 'search_api_query_alter' && isset($implementations['xyz_facets'])) {
    $self = $implementations['xyz_facets'];
    unset($implementations['xyz_facets']);
    $implementations['xyz_facets'] = $self;
  }
}

@nicxvan, since you mentioned 11.2 won't have this, can you please explain why the change to 11.2.x that you just mentioned is not mentioned in the api documentation? Where's the change record for this?

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...

This hook will be removed in 12.0.0. It is not deprecated in order to support the "#[LegacyModuleImplementsAlter]" attribute, used for compatibility with versions prior to Drupal 11.2.0.

This is a very muddy explanation. Versions prior to Drupal 11.2.0. What exactly happens in Drupal 11.2.0? Not deprecated. "won't have this"?

Also, is there some bit of reassurance that can be added to the api documentation saying that "if you change your approach to xyz that abc will continue functioning"? in this case the hook we're dealing with "search_api_query_alter" has not been changed and is a contrib hook.

This particular client code I reference is being used in D11.1.8. D11.2.x is right around the corner and since you "just" mentioned "won't have this", maybe when my client upgrades to D11.2.x, this client won't have combined facet options for content type either?

joseph.olstad’s picture

@nicxvan , I just looked in the 11.2.x branch, 64 lines of code or comments with "modules_implements_alter"

In 11.1.x 46 lines of code mention modules_implements_alter

What exactly do you mean by 11.2.x "won't have this"?

joseph.olstad’s picture

@nicxvan, I found your document, a reference to this should probably be included in all of the hooks documented in the 11.x api

https://nlighteneddevelopment.com/hook-timeline/

nicxvan’s picture

hook_module_implements_alter is still supported in 11.2 but will be removed in 12.

However drupal core has no implementations of hook_module_implements_alter after 11.2 so ckeditor5_module_implements_alter won't throw an error. That is all I meant.

The article you found links to the change record deprecating hook_module_implements_alter.

steven jones’s picture

We're seeing this issue too, in our case, it was a combination of using the shield module, and key module (with key overrides) triggering this issue for us.

Bizarrely enabling the dblog module sorted the issue, as did the patch from #81.

mkdok’s picture

I can also confirm that the issue still occurs on 11.2.4 and shield module enabled.

catch’s picture

Version: 10.5.x-dev » 11.x-dev

Moving this back to 11.x. However hook_module_implements_alter() won't be in 12.x at all (although most other procedural hooks will be eventually deprecated for removal in 13.x, not 12.x).

nicxvan’s picture

Title: [10.x] ModuleHandler skips all hook implementations when invoked before the module files have been loaded » ModuleHandler skips all hook implementations when invoked before the module files have been loaded

@mkdok can you add some more steps to reproduce this based on your comment in the issue summary so we can work on this?

herved’s picture

I believe I am facing this bug on my project.
This happens during deployment in the config:import phase, when there are both modules to be uninstalled and installed at the same time.
I did manage to reproduce on clean 11.x with demo_umami and devel

Steps (I use ddev):
- ddev drush si demo_umami -y
- ddev drush cex -y
- Now go to sites/default/files/sync/core.extension.yml:40 and replace page_cache: 0 with devel: 0
- ddev composer require --dev drupal/devel
- ddev drush deploy -y

$ ddev drush deploy -y
 [notice] Database updates start.
>  [success] No pending updates.
 [success] Config import start.
+------------+----------------+-----------+
| Collection | Config         | Operation |
+------------+----------------+-----------+
|            | core.extension | Update    |
+------------+----------------+-----------+

 // Import the listed configuration changes?: yes.                                                                      

>  [notice] Synchronized extensions: uninstalled page_cache.
>  [error]  Error: Call to undefined function \devel_entity_type_alter() in Drupal\Core\Extension\ModuleHandler->alter() (line 479 of /var/www/html/core/lib/Drupal/Core/Extension/ModuleHandler.php) #0 /var/www/html/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php(389): Drupal\Core\Extension\ModuleHandler->alter()
...stracktrace...

Stacktrace: https://gist.github.com/vever001/a80414b43d317d84e1d1aacc91cbb895

It seems to involve \Drupal\language\EventSubscriber\LanguageRequestSubscriber::onContainerInitializeSubrequestFinished.
Maybe there should be a check to \Drupal::isConfigSyncing() somewhere?
But why is it attempting to run devel_entity_type_alter while devel is not even installed yet.

berdir’s picture

@herved: I'm pretty sure that's a new and different issue than what this is about. Can you create a new issue? It would be helpful to know if this happens in 11.1 or only 11.2. It very likely doesn't happen on 11.0.

And no, this doesn't have anything to do with config syncing, this isn't about changing the container. This happens in the middle of installing the devel module, when the container is built, it finds the hooks, but the module file is only loaded afterwards. And yes, invoking hooks as part of a container rebuild is more than just a little bit scary but I don't think that's new.

herved’s picture

Thank you @Bedir, indeed it looks like a different issue but possibly with the same root cause?
I created #3549397: [regression] Uninstalling and installing modules during config:import can lead to fatal errors. Would it make sense to add a check to is_callable($callable) in ModuleHandler::getFlatHookListeners ?

eric_a’s picture

On Drupal 11.1 Rest API & JSON API 500 errors on authenticated requests with : Error: Call to undefined function \ckeditor5_module_implements_alter() in Drupal\Core\Extension\ModuleHandler->alter()

This specific error went away after updating to Drupal 11.2, as expected. But the underlying problem is still there. It just manifests itself at another point:

Error: Call to undefined function \field_group_module_implements_alter() in Drupal\Core\Extension\ModuleHandler->alter()

eric_a’s picture

So the common thing that I see is the ordering of implementations of hook_form_alter() with hook_module_implements_alter(). Some specifically move the implementation to the end.
With ckeditor5 not implementing this anymore, the problem surfaces with field_group. I wonder: if field_group would move to OO, could then webform < 6.3.0-beta5 hit this...

berdir’s picture

Can you provide a full backtrace of how you hit this error?

anybody’s picture

@eric_a feel free to create an issue and a MR over at field_group. I'd be happy to merge it, once RTBC'd. Still I'm not sure if it will be the "final" fix. Often the module shown in the error isn't really causing it, just affected by it. At least we had that experience.

Thanks!

eric_a’s picture

Can you provide a full backtrace of how you hit this error?

Error: Call to undefined function \field_group_module_implements_alter() in Drupal\Core\Extension\ModuleHandler->alter() (regel 474 van /data/www/html/test.site/web/core/lib/Drupal/Core/Extension/ModuleHandler.php)
#0 /data/www/html/test.site/web/core/lib/Drupal/Core/Extension/ModuleHandler.php(646): Drupal\Core\Extension\ModuleHandler->alter()
#1 /data/www/html/test.site/web/core/lib/Drupal/Core/Extension/ModuleHandler.php(532): Drupal\Core\Extension\ModuleHandler->reOrderModulesForAlter()
#2 /data/www/html/test.site/web/core/lib/Drupal/Core/Extension/ModuleHandler.php(471): Drupal\Core\Extension\ModuleHandler->getCombinedListeners()
#3 /data/www/html/test.site/web/core/lib/Drupal/Core/Database/Query/Select.php(491): Drupal\Core\Extension\ModuleHandler->alter()
#4 /data/www/html/test.site/web/core/lib/Drupal/Core/Database/Query/Select.php(516): Drupal\Core\Database\Query\Select->preExecute()
#5 /data/www/html/test.site/web/core/lib/Drupal/Core/Entity/Query/Sql/Query.php(288): Drupal\Core\Database\Query\Select->execute()
#6 /data/www/html/test.site/web/core/lib/Drupal/Core/Entity/Query/Sql/Query.php(85): Drupal\Core\Entity\Query\Sql\Query->result()
#7 /data/www/html/test.site/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php(608): Drupal\Core\Entity\Query\Sql\Query->execute()
#8 /data/www/html/test.site/web/core/modules/user/src/UserAuthentication.php(64): Drupal\Core\Entity\EntityStorageBase->loadByProperties()
#9 /data/www/html/test.site/web/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php(99): Drupal\user\UserAuthentication->lookupAccount()
#10 /data/www/html/test.site/web/modules/contrib/shield/src/ShieldMiddleware.php(336): Drupal\basic_auth\Authentication\Provider\BasicAuth->authenticate()
#11 /data/www/html/test.site/web/modules/contrib/shield/src/ShieldMiddleware.php(256): Drupal\shield\ShieldMiddleware->basicAuthRequestAuthenticate()
#12 /data/www/html/test.site/web/modules/contrib/shield/src/ShieldMiddleware.php(130): Drupal\shield\ShieldMiddleware->bypass()
#13 /data/www/html/test.site/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\shield\ShieldMiddleware->handle()
#14 /data/www/html/test.site/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
#15 /data/www/html/test.site/web/core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php(53): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
#16 /data/www/html/test.site/web/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\AjaxPageState->handle()
#17 /data/www/html/test.site/web/core/lib/Drupal/Core/DrupalKernel.php(715): Drupal\Core\StackMiddleware\StackedHttpKernel->handle()
#18 /data/www/html/test.site/web/index.php(19): Drupal\Core\DrupalKernel->handle()
#19 {main}
berdir’s picture

Ok, shield module again, as usual.

Loading entities in a middleware will *maybe* work reliable in D13 with the concept of loading modules gone, but even then it might have side effects. https://git.drupalcode.org/project/shield/-/blob/8.x-1.x/src/ShieldMiddl... is also really bad in regards of performance. It loads a ton of dependencies and basically undoes all the improvements that went into https://www.drupal.org/node/3538740 in 11.3.

If does stuff like then it should be responsible to properly bootstrap Drupal.

I understand it needs to do that to run before page cache, but that comes at a serious hit to performance, you should at least make sure to not have that enabled on production and I'd recommend using an alternative way outside of Drupal to protect dev environments.

nicxvan’s picture

luke.leber’s picture

Are there other enabling criteria other than middleware layers that may cause this condition?

Our hosting provider (Acquia) refers to this issue as a root cause of the symptom outlined in #3277728: Call to undefined method isLayoutBuilderEnabled(), but I've audited our entire code-base numerous times and have confirmed there isn't any code at play that does anything remotely related to invoking inappropriate things in constructors.

If it helps at all, it appears that an out of memory error occurred just prior to the last time we've recorded this behavior.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

marttir’s picture

If this is indeed still an issue in 11.2.x, perhaps it is still an issue in 11.3.x as well?

In any case, this needs a reroll for 11.x.

luke.leber’s picture

Hello all,

This comment is one of the results from making S.D.O. issue #184413 public.

I've added a functional test case that should prove this is an issue on Drupal 10. The chain of event goes...

1. The discovery cache is empty
2. In the next request, an exception is thrown from the early kernel bootstrapping
3. A logger object is instantiated by errors.inc, which invokes discovery before all modules are loaded

In the wild, this can happen with a combination of core modules and certain popular contributed modules: syslog and key.

1. Layout builder is enabled
2. The discovery cache is empty (which on Acquia hosting, can happen randomly as the platform cycles memcached k8s pods out!)
3. In the next request, an exception is thrown from the early kernel bootstrapping
4. A syslog object is instantiated by errors.inc, which calls the config factory service
5. The config factory service dispatches the appropriate API events to discover and apply config overrides
6. The key module implements a config override which invokes entity type discovery via the entity type manager
7. The entity type cache item is corrupted and has the \Drupal\Core\Entity\EntityViewDisplay class set and not the layout builder flavor
8. Subsequent requests show no layout builder content on pages until the discovery cache is flushed and can rebuild appropriately!

hctom changed the visibility of the branch 10.5.x to active.

hctom changed the visibility of the branch 10.5.x to hidden.

joegl’s picture

We recently upgraded from 10.5.x to 11.2.x and are using the Shield module. I wanted to quickly provide an update.

We had previously used a patch from the merge request for 10.5.x to resolve the issue. However, there is no longer a merge request or patch for 11.2.x, and we removed it. Unfortunately, the error popped up again:

TypeError: Drupal\Core\Entity\EntityTypeManager::Drupal\Core\Entity\{closure}(): Argument #1 ($hook) must be of type callable, string given, called in /var/www/html/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php on line 357 in /var/www/html/docroot/core/lib/Drupal/Core/Entity/EntityTypeManager.php on line 117

Based on the comments and guidance in related issues, while it seems a core fix would be desirable, the preference now is to improve the middleware in modules to not make these calls.

We applied a patch from the most recent merge request for the Shield issue: https://www.drupal.org/project/shield/issues/3277210. This is a one-liner that adds the following to the middleware:

+    // @see https://www.drupal.org/project/shield/issues/3277210
+    // Prevent corrupting the module_implements cache.
+    $this->moduleHandler->loadAll();
+

It's clearly not a true fix and is non-optimal but it does clean up the error on our end.