Tagged services - misleading exception

If the exception is in Symfony it must be fixed there.

Sorry, you are right. I was confused by the exception's namespace, Symfony\Component\DependencyInjection\Exception\LogicException.

Hello guys, I am working on trying to extend the twig function from a custom module and I am also getting the same issue mentioned above, is this something still being worked on?

Or would anybody know a solution.

I am getting the following error:

The website encountered an unexpected error. Please try again later.
Symfony\Component\DependencyInjection\Exception\LogicException: Service 'bml_site.twig.bml_site_extension' for consumer 'twig' does not implement Twig_ExtensionInterface. in Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass->process() (line 125 of [I REMOVED MY PATH]/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php).
Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass->process(Object) (Line: 107)
Symfony\Component\DependencyInjection\Compiler\Compiler->compile(Object) (Line: 593)
Symfony\Component\DependencyInjection\ContainerBuilder->compile() (Line: 1254)
Drupal\Core\DrupalKernel->compileContainer() (Line: 866)
Drupal\Core\DrupalKernel->initializeContainer() (Line: 40)
Drupal\Core\Update\UpdateKernel->initializeContainer() (Line: 461)
Drupal\Core\DrupalKernel->boot() (Line: 62)
Drupal\Core\Update\UpdateKernel->handle(Object) (Line: 19)

My services file looks like this:

services:
  bml_site.twig.bml_site_extension:
    class: Drupal\bml_site\Twig\BMLSiteExtension
    tags:
      - { name: twig.extension }

Thanks

This issue is about a misleading error when the namespace can't be found. In your case, does Drupal\bml_site\Twig\BMLSiteExtension exist and does it implement Twig_ExtensionInterface or more likely, extend \Twig_Extension?

Just a quick note, when this was just occurring for me I found that the src file name is case sensitive
i.e. my class def is "class Images extends \Twig_Extension", with a filename of "images.php" it would fail compilation, but "Images.php" would work. (probably not related to the core issue but might help newcomers like me to get some progress).
Steve.

Maybe something which tells that either the class or the namespace is wrong would be IMHO more concrete. The current proposed error message seems to be really cryptic, at least for me.

There's a real bug here, most Symfony's compiler class will manually resolve class names before using it, for example you can see it there: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Eve...

Drupal never does this, and as soon as you rely upon parameters for class names, Drupal will go into havoc, and this is a seriously blocking issue for contrib.

I am preparing a complete explicative bug report for this and a preliminary proof a concept patch, will link it here as soon as it's done.

And done! https://www.drupal.org/node/2832018

As I lost some time on this I'd suggest putting it in while we wait for #2832018 to be completed. Tested.

I agree with @dawehner in #11; the current exception message doesn't go far enough to explain the problem. I agree with hotfixing this while #2832018: Compiler passes don't manually resolve class names of definition before using it is underway to provide better DX and debugging, and I think we could still backport it since exception messages are not translated and we're not changing what exceptions are thrown or where, but I wouldn't know what to do with "its namespace cannot be resolved". So let's add more detail that the class or namespace could be incorrect...

But actually, we should probably be throwing a different exception message earlier. Is this happening if $handler or $handler->getClass() are empty? If so, we could throw a different exception in that case rather than relying on an unclear "or".

Just got hit with this too. Drupal 8 is already extremely hard to use, why can't somebody just add this easy patch?

The reason is that the feedback in #17 hasn’t been seen to. Please upload a patch.

Here is a test coverage, anyone wants to try to implement it?

I would love to see this fixed.

The error message "consumer X does not implement Y" is so specific that it can completely get you off on the wrong foot. If your problem isn't exactly that, it only makes debugging more difficult.

Also, I'm cringing at the fact that this has been tagged as "Quick fix" for 6 years. This is a case of "perfect is the enemy of good". The patch from #5 would have been incredibly helpful. I'm marking this RTBC: let's commit #5 and do the gold-plating in another issue.

@dawehner provided a failing test in #21, this needs to be combined with the patch in #5 to solve this issue.

Addressed Comment #29.

Fix CS ERRORS.

The last patch doesn't pass commit checks, could you make sure to run ./core/scripts/dev/commit-code-check.sh before uploading a patch to make sure there are no issues with code formatting. see https://www.drupal.org/docs/develop/development-tools/running-core-devel...

Fixed deprecation issu.e

Fixed up test case.

The test from #21 was more specific; it should be possible to know which of the error cases is the actual issue by providing a different message for each, instead of leaving it to the developer to work out.

Updated for message from #21.

not sure what happened in #37

Random failure

IMO the change in the exception message from #37 to #38
- does not implement $interface, or its namespace cannot be resolved.
+ does not have an existing class $interface.
is restoring some confusion to the dev/reader, i.e. it does little to inform that the provided class: for the collected service does not exist. The $interface variable will be generated based on the service collector's definition and not the collected service.

Even more, the actual change in code could focus explicitly on this case, e.g.:

+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandl
ersPass.php
@@ -178,6 +178,9 @@ protected function processServiceCollectorPass(
array $pass, $consumer_id, Contai
     foreach ($this->tagCache[$tag] ?? [] as $id => $attributes) {
       // Validate the interface.
       $handler = $container->getDefinition($id);
+      if (class_exists($handler->getClass()) === FALSE) {
+        throw new LogicException("Service '$id' for consumer '$con
sumer_id' does not have a corresponding existing class " . $handler
->getClass());
+      }
       if (!is_subclass_of($handler->getClass(), $interface)) {
         throw new LogicException("Service '$id' for consumer '$con
sumer_id' does not implement $interface.");
       }

That would mean a small change in the first one of the new tests to capture the new exception for non-existing classes.
And a small change in the second test, if we stick to the original exception message (Service '$id' for consumer '$consumer_id' does not implement $interface.