When a block plugin declares a non-existing deriver in its annotations and you run drush cr, Drupal suggests that the deriver plugin was found, but invalid:

Plugin (modulename) deriver "Drupal\modulename\Plugin\Deriver\NonExisting" must implement \Drupal\Component\Plugin\Derivative\DeriverInterface

In my situation, this message made me search for the problem in the wrong place. Let's have a different message in case the deriver class does not exist.

CommentFileSizeAuthor
#6 deriver_does_not_exist_exception-2333465-6.patch3.26 KBmarcvangend
#4 deriver_does_not_exist_exception-2333465-4.patch2.57 KBmarcvangend
#1 deriver_does_not_exist_exception-2333465-1.patch1.13 KBmarcvangend

Comments

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented
Assigned: marcvangend » Unassigned
Status: Active » Needs review
StatusFileSize
new deriver_does_not_exist_exception-2333465-1.patch1.13 KB

...and the patch.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs review » Reviewed & tested by the community

This actually applies to any plugin type of which a plugin has a non-existing class defined as its deriver. This simple check can save a lot of time.

RTBC if the tests pass.

dawehner’s picture

dawehner
Primary language German
commented
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs tests

This looks great, but yeah let's ensure that we expand the test coverage, see DerivativeDiscoveryDecoratorTest

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented
StatusFileSize
new deriver_does_not_exist_exception-2333465-4.patch2.57 KB

Thanks dawehner.

Here's a new patch which adds a test. That said: I'm not quite sure how useful it actually is. The initial patch merely switches between error messages. The tests now check twice for the same exception, but it doesn't test if the right message is returned in each situation.

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/tests/Drupal/Tests/Core/Plugin/Discovery/DerivativeDiscoveryDecoratorTest.php
@@ -89,6 +89,28 @@ public function testGetDerivativeFetcherWithAnnotationObjects() {
+   * @expectedException \Drupal\Component\Plugin\Exception\InvalidDeriverException
+   */

Afaik it would be great to also use @expectedExceptionMessage just to be sure.

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented
StatusFileSize
new deriver_does_not_exist_exception-2333465-6.patch3.26 KB

New patch. I added @expectedExceptionMessage to both derivative discovery exception tests.

While I was at it, I also:
- made the exception messages more consistent (both end with a period now);
- changed "non existing" to "non existent" because that seems a more common phrase in both Drupal core and the internet as a whole.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Thank you!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 2c35927 and pushed to 8.0.x. Thanks!

  • alexpott committed 2c35927 on 8.0.x 
    Issue #2333465 by marcvangend: Fixed Confusing exception message when...

Status: Fixed » Closed (fixed)

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