Comments

nickvanboven created an issue. See original summary.

hanoii’s picture

I also looked into this while looking at the related issue:

I tried to look into this. It's reproducible. The error comes from the webprofiler module when its attempting to do the reflection of the class. If the serialization module is enabled, that error doesn't happen. I tried jsonapi out of curiosity as I would have expected them to have the same issue, but they have serialization as a dependency, thus preventing this.

I am not sure what's the fix for this, if any, but likely beyond this module unless adding serialization as a dependency is viable, but I think not.

There could potentially be a fix to be done on the profiler to prevent a fatalerror on this or to reflect all classes, not only the ones of the enabled modules. But it's a bit beyond me or what I can look into this right now.

hanoii’s picture

Well, maybe adding serialization as a dependency is not THAT bad. Another option would be to create a metatag_serialization submodule with the dependency most of the code added by the patch on the parent issue.

damienmckenna’s picture

That's weird, the tests run ok so it isn't a general problem when WebProfier is enabled. I wonder if the tests don't show the problem because it's using the "testing" profile which doesn't have a full site to test against?

hanoii’s picture

But what tests? The webprofiler tests? I don't think this use case is covered by tests. Metatag+Devel+Webprofiler module without serialization, or is it?

damienmckenna’s picture

It's like there's a problem with WebProfiler that it's loading classes it shouldn't be trying to load, breaking the DI workflow..

Incidentally, changing the test's profile to "standard" causes the tests to never finish. So there's a problem there somewhere X-)

damienmckenna’s picture

Check out Drupal\Tests\metatag\Functional\EnsureDevelWebProfilerWorks.

Maybe because it doesn't create any entities?

hanoii’s picture

Ah, I didn't know you were already testing for the module.

I do feel that there's something odd on the webprofiler module.

FWIW I tried a manual install with no entities and the error is still there.

damienmckenna’s picture

Try updating Devel to the latest -dev snapshot, does that make a difference?

damienmckenna’s picture

Status: Active » Needs review
StatusFileSize
new1.92 KB

This expands the tests to create a content type and node, and then load a node.

damienmckenna’s picture

StatusFileSize
new2.93 KB

Refactored NodeJsonOutput() to use this too.

  • DamienMcKenna committed 8cfebaa on 8.x-1.x
    Issue #2899234 by DamienMcKenna: Expand Devel/WebProfiler tests to make...
damienmckenna’s picture

Those tests work locally so I've committed them.

hanoii’s picture

Devel-1.x does the have the same issue.

hanoii’s picture

Status: Needs review » Needs work

And the patch I guess doesn't really fix or add a proper test to this issue.

EDIT: As it was passing before anyway.

damienmckenna’s picture

The patch was so that we could have an entity existing while WebProfiler was installed, rather than just a custom route, so that WebProfiler would try to do its entity shuffling stuff.

What other modules do you have installed?

hanoii’s picture

Me, I tried it with a bare install, nothing other than metatag and webprofiler.

damienmckenna’s picture

Out of interest are you using 8.3.x or 8.4.x?

damienmckenna’s picture

I've been working on fixing the tests for 8.4.x (#2898805: Fix tests for 8.4.x) and have discovered that EnsureDevelWebProfilerWorks does indeed still fail on 8.4.x on drupalci, though it does work locally. Gah.

hanoii’s picture

8.3.x currently

damienmckenna’s picture

What version of PHP?

hanoii’s picture

The bare install I tried this is on PHP 7.0.20.

I haven't tried it, but I think this error should happen with any non-core plugin if the module is not enable or even there.

Like say I have a module that defines a webform plugin to be used if webform is available but I don't even have webform, the same should happen.

hanoii’s picture

Also, out of curiosity as I haven't worked so much with tests.. do you have anything special or recommendation to go about it, or just a normal durpal install? I once tried to setup the database on a ram drive or something on D7 to speed things up, but not sure if that's still necessary/worth doing.

Anyway, if you have any advise to share to get speed up getting into testing, appreciated!

Thanks!

damienmckenna’s picture

Title: Missing dependency - drupal:serialization » Missing dependency (drupal:serialization) / Fatal error: Class NormalizerBase not found
damienmckenna’s picture

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

Can you please confirm if this is still a problem? Devel 1.0 was released and should fix this problem.

edurenye’s picture

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

Well, it does not. Sadly, the error still happens.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new314 bytes

Adding the missing dependency fixes the problem.

paladyn’s picture

@DamienMcKenna
Problem still exists.
I am using Drupal 8.3.7, MetaTag 8.x-1.2, Devel 8.x-1.0 (with WebProfiler enabled).

Error:
Error: Class 'Drupal\serialization\Normalizer\NormalizerBase' not found in .../metatag/src/Normalizer/FieldItemNormalizer.php, line 10

I`ve added drupal:serialization dependency to metatag.info.yml.

edurenye’s picture

You must reinstall the module or install serialization manually for this to take effect, probably we should add and update path.
But I'm not sure if adding a module with an update path could break any current site.

damienmckenna’s picture

Status: Needs review » Needs work

Adding a dependency is the wrong approach to take and isn't going to happen. The problem is with Devel or WebProfiler, it shouldn't be loading classes unnecessarily.

norman.lol’s picture

Thanks #29! Enabling Drupal's Serialization module and re-installing Metatag fixed the issue for me.

vvs’s picture

I'm from #2913860: Error after enable module config_translation. Site cruched and drush not working… How to reanimate it?

musa.thomas’s picture

Agree with #30
Pending new patch if you can't use drush / ui to disable enble module you can pro tempore delete all content in all src/Normalizer of metatag, then enable your module, patch metatag, then put back the code inside.

jeffm2001’s picture

Dumb question, but the FieldItemNormalizer class extends Drupal\serialization\Normalizer\NormalizerBase. Does this not mean that metatag depends on serialization? Not trying to be obnoxious here, I honestly don't know the answer.

damienmckenna’s picture

@JeffM2001: due to how the DI system works, I don't believe it should try loading those classes until a module requests them. The problem is that something is requesting them when it shouldn't be.

We could try wrapping the class in a class_exists() call, but it's a bit unhygienic.

jeffm2001’s picture

From what I can tell — and I admit I am a bit out of my depth here — webprofiler is attempting to gather info on every service defined in the container (see Drupal\webprofiler\Compiler\ServicePass class). That's obviously not a typical scenario, but also doesn't seem strictly out of bounds.

Should it be up to webprofiler to determine if another module's service has an undeclared dependency? (again an honest question)

Perhaps these services should be defined in a submodule that has serialization as a dependency.

jeffm2001’s picture

Status: Needs work » Needs review
StatusFileSize
new2.53 KB

I looked into this a bit more, and I think this might be the best way to go. Rather than define the normalizers in metatag.services.yml, it uses a ServiceProvider class to add the definitions conditionally. This patch is based on what's in the entity_reference_revisions module.

damienmckenna’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

@JeffM2001: Awesome, thank you.

Anyone have any suggestions on how to reproduce the problem using tests?

twang’s picture

Status: Needs work » Reviewed & tested by the community
StatusFileSize
new2.53 KB

We downloaded the patch #37 and tested it. It threw warnings related to blank lines with spaces. We fixed the warnings, tested, and verified that the patch works.

twang’s picture

StatusFileSize
new274.43 KB

We tried to fix the blank line warning issues in #39 as shown by the attached screen shot, but apparently the issue is still here.

jcisio’s picture

Status: Reviewed & tested by the community » Needs work

In #38 the maintainer asked for test, which there is not now.

BTW it seems that the cause is webprofiler module. You need to enable that module to have this bug.

damienmckenna’s picture

@jcisio: But that's the thing - there's already a test that enables WebProfiler and it doesn't hit this error.

kumkum29’s picture

Hello,

I have broken my site after install metatag & Webprofiler.
I get always this error in my server logs.
But I don't see how to uninstall metatag / or webprofiler without the Drupal UI. I tried several Drush commands without success.

Can you give a solution to uninstall manualy metatag module?

thanks.

I'm on D8.4

PS:
When I use a drush command, i get this error:

PHP Fatal error:  Class 'Drupal\serialization\Normalizer\NormalizerBase' not found in /srv/data/myserver/htdocs/modules/metatag/src/Normalizer/FieldItemNormalizer.php on line 10
Drush command terminated abnormally due to an unrecoverable error.
kumkum29’s picture

Priority: Normal » Critical
damienmckenna’s picture

As mentioned above, a temporary workaround is to enable the Serialization module.

damienmckenna’s picture

Can anyone please work out the steps necessary to trigger this problem? Given that the module already has tests which confirm the error doesn't happen with a bare installation, there has to be some extra piece that causes it to fail.

hanoii’s picture

I don't think it doesn't happen with a bare drupal.

I just run a simpletest.me with this and devel, enabled metatag and webprofiler and I got:

Fatal error: Class 'Drupal\serialization\Normalizer\NormalizerBase' not found in /home/du1jz/www/sites/default/modules/metatag/src/Normalizer/FieldItemNormalizer.php on line 10

And it became broken: https://du1jz.ply.st/ (up for the next 23h, not that is any useful like this).

I think the test, for some reason, is not really triggering this.

damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new682 bytes

@hanoii: Ok then, lets try this.

damienmckenna’s picture

The tests still pass and they pass locally. That said, my local D8 build *does* fail.

kumkum29’s picture

@DamienMcKenna

Unfornately, this patch don't resolved my broken site.
I haven't access to Drupal admin UI, and drush commands don't work.

Is there another method to uninstall metatag module ?

damienmckenna’s picture

StatusFileSize
new1.51 KB

Ok. Does this work? It checks to make sure that the NormalizerBase class is available.

Status: Needs review » Needs work

The last submitted patch, 51: metatag-n2899234-51.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new1.55 KB

Ok, try it a different way.

kumkum29’s picture

Yes !!

my site is back !!!
Now, I can uninstall the webprofiler module.

Thanks ;)

(with patch 51)

Status: Needs review » Needs work

The last submitted patch, 53: metatag-n2899234-53.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jeffm2001’s picture

Status: Needs work » Needs review
StatusFileSize
new2.52 KB

@DamienMcKenna: I still think the approach in my patch (#37) is the way to go.

The fundamental problem is defining a service that is extending a class which may or may not exist. Your patch in #53 does seem to fix the error, but it looks strange to me to have a condition just floating there outside the class. I think it's better to not have the service registered at all if the module it depends on isn't installed. This is also the approach used in entity_reference_revisions and file_entity.

As for tests, it's not clear to me why the existing test doesn't get triggered, but I don't have a ton of experience in that area. Wondering if an explicit test for webprofiler is really necessary. Obviously it would be nice, but if the patch is an improvement, solves the specific issue folks are seeing, and doesn't break any other tests, might be worth just committing.

Here's the patch again with a fix for the whitespace issue that was flagged by twang.

damienmckenna’s picture

StatusFileSize
new1.86 KB

@JeffM2001: You are correct, especially when the conditional logic I was trying was causing the tests to fail. Your patch works great.

I'm still trying to reproduce the problem via tests.

Status: Needs review » Needs work

The last submitted patch, 57: metatag-n2899234-57.patch, failed testing. View results

damienmckenna’s picture

And the test failure was useless:

1) Drupal\Tests\metatag\Functional\EnsureDevelWebProfilerWorks::testManual
Exception: Warning: uasort(): Array was modified by the user comparison function
Drupal\shortcut\Entity\ShortcutSet->getShortcuts()() (Line: 120)

*sigh*

So I'm planning on committing #56 but I really would prefer to be able to cause a proper test failure first.

damienmckenna’s picture

Reran the test and it still fails again. The backtrace ends with this, i.e. this is where we find the last function call before it fails:
/var/www/html/modules/contrib/metatag/tests/src/Functional/EnsureDevelWebProfilerWorks.php:42
That is this line:

    $this->drupalGet('/admin/modules/uninstall');

pullmyhairout.gif

jeffm2001’s picture

I did some playing around with this, and from what I can tell, it might actually be impossible to trigger this error through tests. You can assertTrue(class_exists('Drupal\serialization\Normalizer\NormalizerBase')) or any other class in the system and it will always pass. Take a look at core/tests/bootstrap.php, it appears to be finding the paths for every core/contrib extension and adding them all to the autoloader.

heddn’s picture

Closed out #2923446: webprofiler depends on serialization as duplicate.

+++ b/src/MetatagServiceProvider.php
@@ -0,0 +1,37 @@
+      $metatag_field = new Definition('Drupal\metatag\Normalizer\FieldItemNormalizer');
...
+      $metatag = new Definition('Drupal\metatag\Normalizer\MetatagNormalizer');
...
+      $metatag_hal = new Definition('Drupal\metatag\Normalizer\MetatagHalNormalizer');

Drive-by feedback: can this use ClassName::class syntax?

mile23’s picture

Yes, bootstrap.php finds all existing extensions and adds them to the autoloader. This is left over from when there were only Drupal unit tests in PHPUnit, so it made sense.

This means that when you run a PHPUnit-based test you'll have autoloadable access to all the classes in serialization, which isn't so great for functional or kernel test purposes.

During run-time, this won't happen, so you could break anything that needs serialization module (or anything that isn't declared a dependency in info.yml).

Basically: Since the core module is always present in the file system, you won't be able to test the behavior of when it's unavailable.

It also seems that since FieldItemNormalizer::normalize() says return t('Metatags are normalized in the metatag field.');, #56 needs to remove FieldItemNormalizer as well. Or at least deprecate it.

heddn’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests +Needs manual testing
StatusFileSize
new2.3 KB
new1.98 KB

Here's an attempt to respond to the feedback since #56. I've removed the needs test tag, since it doesn't seem possible. And add manual testing instead.

shaundychko’s picture

StatusFileSize
new2.59 KB

The patch takes the comment in #63 that the third normalizer service needs to be moved from metatag.service.yml into the service provider class, and also edits #64 to use short array syntax.

dqd’s picture

ok, last both comments are confusing. Which one should be reviewed? Is #65 a follow up with additions or is it another approach? Oh I think I missed the last part of the last comment.

dqd’s picture

 $ git apply -v 2899234-65-metatag-normalizer.patch
Checking patch metatag.services.yml...
Checking patch src/MetatagServiceProvider.php...
Applied patch metatag.services.yml cleanly.
Applied patch src/MetatagServiceProvider.php cleanly.
dqd’s picture

And I can confirm finally that the Error 500 (WSOD) disappears. So you can reach the site again. Any drawbacks or flaws aren't investigated yet. At least you are able to deactivate the respective modules now.

EDIT: no drawbacks yet with both modules still active (1 1/2 days)

landsman’s picture

Hello,

I have problem on multisite site in Drush. On default site it works fine, look:

 drush status
Drush command terminated abnormally due to an unrecoverable error.                                                                                                [error]
Error: Class 'Drupal\serialization\Normalizer\NormalizerBase' not found in
/Users/michal.landsman/localhost/marianne-thunder/docroot/modules/contrib/metatag/src/Normalizer/FieldItemNormalizer.php, line 10

Fatal error: Class 'Drupal\serialization\Normalizer\NormalizerBase' not found in /Users/michal.landsman/localhost/marianne-thunder/docroot/modules/contrib/metatag/src/Normalizer/FieldItemNormalizer.php on line 10
landsman2-mbp:docroot michal.landsman$ drush site-set default
Site set to default
landsman2-mbp:docroot michal.landsman$ drush status
 Drupal version                  :  8.4.3                                                     
 Site URI                        :  default                                                   
 Database driver                 :  mysql                                                     
 Database hostname               :  127.0.0.1                                                 
 Database port                   :  3306                                                      
 Database username               :  root                                                      
 Database name                   :  marianne-thunder                                          
 Database                        :  Connected                                                 
 Drupal bootstrap                :  Successful                                                
 Drupal user                     :                                                            
 Default theme                   :  marianne                                                  
 Administration theme            :  thunder_admin                                             
 PHP configuration               :  /Applications/MAMP/bin/php/php7.1.8/conf/php.ini          
 PHP OS                          :  Darwin                                                    
 Drush script                    :  /usr/local/bin/drush                                      
 Drush version                   :  8.1.15                                                    
 Drush temp directory            :  /tmp                                                      
 Drush configuration             :                                                            
 Drush alias files               :                                                            
 Install profile                 :  thunder                                                   
 Drupal root                     :  /Users/michal.landsman/localhost/marianne-thunder/docroot 
 Drupal Settings File            :  sites/default/settings.php                                
 Site path                       :  sites/default                                             
 File directory path             :  sites/default/files/public                                
 Private file directory path     :  sites/default/files/private                               
 Temporary file directory path   :  ../tmp/private                                            
 Sync config path                :  ../config/default/sync                                    

landsman2-mbp:docroot michal.landsman$ 

Can you say where is a problem please?
EDIT: This patch working for me now: https://www.drupal.org/files/issues/2899234-65-metatag-normalizer.patch

danielnolde’s picture

2899234-65-metatag-normalizer.patch did _not_ solve the error

> Fatal error: Class 'Drupal\serialization\Normalizer\NormalizerBase' not found in /htdocs/modules/contrib/metatag/src/Normalizer/FieldItemNormalizer.php on line 10

when having metatag.module enabled with devel.module's webprofiler module.

jonasdk’s picture

Any chance this will get into the release soon?
I have tested the patch in https://www.drupal.org/project/metatag/issues/2899234#comment-12357622 and it seems to work as promissed.

I did this after applying the patch to ensure that no other problems got stuck in there.

drush pm-uninstall devel -y
drush en webprofiler
mmbk’s picture

I had the same issue on my development system, metag did not install while webprofiler was activated.
Applying the patch #65 solved the problem, the module is working now.

Georgii’s picture

+1 for the patch #65.

In reply to #70. Did you confirm that the patch was applied? In my case I had the same story after executing git apply. And then I found that the patch just was not successfully applied until patch -p1 < 2899234-65-metatag-normalizer.patch

Georgii’s picture

Status: Needs review » Reviewed & tested by the community

With all the confirmation above may I move it forward?

damienmckenna’s picture

Committed. Thanks everyone!

damienmckenna’s picture

Status: Reviewed & tested by the community » Fixed

This was actually committed, so it can be closed.

It will be included in the next stable release, 8.x-1.5.

Status: Fixed » Closed (fixed)

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

dqd’s picture

Thanks @all! Great work done here. Thanks @heddn & @Shaun for the final patch and thanks to @Damien for supporting, guiding and letting it in so quickly.