hidden dependency on webprofiler crashes EcaDataCollector service

Thanks @joachim, we've had this issue reported already twice. But since the classes, that are using the trait, are webprofiler plugins, they are not supposed to be used/called by anything else other than webprofiler. So, as long as webprofiler isn't available, this code should never be touched, executed or initialized.

Looks like module builder is touching that code anyways. Is there a way to prevent that from happening there? Because I've no idea how we should re-architect these plugins, that are clearly valid webprofiler plugins.

EcaDataCollector isn't a plugin, it's a service.

Module Builder looks at all services in a codebase to perform code analysis on them.

I think the way to do this would be to use a service provider, and only declare EcaDataCollector if webprofiler is present. Or make eca_ui submodule depend on eca_ui.

EcaDataCollector isn't a plugin, it's a service.

My bad, thanks for correcting this.

That service implementation has been hard already, since it needs to support 2 different versions of webprofiler in Drupal 9 and for Drupal 10 and beyond. Making webprofiler a dependency isn't an option either, as people are using ECA UI without webprofiler. We would have to create another sub-module or ask webprofiler maintainers to do the integration at their end.

Isn't their an option, to tag a service such that module builder would ignore it? That would be the easiest way for now, not sure if at all possible and wanted,

> it needs to support 2 different versions of webprofiler in Drupal 9 and for Drupal 10 and beyond

Ouch! I thought I saw some classes with version suffixes! I've had that pain too :/

> Isn't their an option, to tag a service such that module builder would ignore it? That would be the easiest way for now, not sure if at all possible and wanted,

What I'll try first is running services though DCB's class safety checker -- I wrote a tool which checks whether PHP can safely instantiate a class without crashing. It's currently only used on plugin classes, because some contrib modules had plugins which were broken.

I'll see how much that slows analysis down by!

Hmm that's weird, services are already being checked for PHP safety:

      // Skip if the class isn't loadable by PHP without causing a fatal, as we
      // can't work with just an ID to generate things like injection.
      // (The case of a service class whose parent does not exist happens if
      // a module Foo provides a service for module Bar's collection with a
      // service tag. Metatag module is one such example.)
      if (!$this->codeAnalyser->classIsUsable($service_class)) {
        continue;
      }

I've fixed this in latest version -- turns out it wasn't checking class safety of services when looking for plugin managers.

This is great news. Thanks a lot @joachim

Marking this as fixed upstream. Thanks a lot @joachim for getting this done.

Automatically closed - issue fixed for 2 weeks with no activity.