Problem/Motivation

After upgrading to 8.8, our tests started throwing exceptions in various places that weren't checking if $route_match->getRouteObject() returned NULL. One of those places is in layout_builder_system_breadcrumb_alter

I'm not entirely sure what changed between 8.7 and 8.8, but it seems to be to do with rendering content controlled by layout_builder outside of a route context, for example during search indexing.

See the full stack trace below:

1) Drupal\Tests\apra_publications_search\Functional\PublicationsSearchTest::testPublicationsSearch
Error: Call to a member function hasOption() on null

/data/app/core/modules/layout_builder/layout_builder.module:336
/data/app/core/lib/Drupal/Core/Extension/ModuleHandler.php:539
/data/app/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php:94
/data/app/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php:72
/data/app/core/modules/layout_builder/src/EventSubscriber/BlockComponentRenderArray.php:104
/data/app/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111
/data/app/core/modules/layout_builder/src/SectionComponent.php:90
/data/app/core/modules/layout_builder/src/Section.php:86
/data/app/core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplay.php:317
/data/app/core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplay.php:276
/data/app/core/lib/Drupal/Core/Entity/EntityViewBuilder.php:351
/data/app/core/modules/node/src/NodeViewBuilder.php:24
/data/app/core/lib/Drupal/Core/Entity/EntityViewBuilder.php:293
/data/app/core/lib/Drupal/Core/Entity/EntityViewBuilder.php:250
/data/app/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php:100
/data/app/core/lib/Drupal/Core/Render/Renderer.php:781
/data/app/core/lib/Drupal/Core/Render/Renderer.php:372
/data/app/core/lib/Drupal/Core/Render/Renderer.php:200
/data/app/core/lib/Drupal/Core/Render/Renderer.php:156
/data/app/core/lib/Drupal/Core/Render/Renderer.php:573
/data/app/core/lib/Drupal/Core/Render/Renderer.php:157
/data/app/modules/contrib/search_api/src/Plugin/search_api/processor/RenderedItem.php:276
/data/app/modules/contrib/search_api/src/Item/Item.php:272
/data/app/modules/contrib/search_api/src/Entity/Index.php:975
/data/app/modules/contrib/search_api/src/Utility/PostRequestIndexing.php:84
/data/app/modules/custom/apra_tests/tests/src/Functional/ApraTestBase.php:90
/data/app/modules/custom/apra_publications_search/tests/src/Functional/PublicationsSearchTest.php:33

Proposed resolution

Check $route_match->getRouteObject() is not NULL.

CommentFileSizeAuthor
#19 3104980--19.patch2.19 KBdanflanagan8
#19 3104980--19--FAIL.patch1.12 KBdanflanagan8
#18 3104980--18.patch2.19 KBdanflanagan8
#18 3104980--18--FAIL.patch1.12 KBdanflanagan8
#5 3104980-4-test-only.patch1.55 KBacbramley
#2 3104980-2.patch1.13 KBacbramley

Comments

acbramley created an issue. See original summary.

acbramley’s picture

acbramley commented
Status: Active » Needs review
StatusFileSize
new 3104980-2.patch1.13 KB

I'm not sold that this is the right fix, but it gets around the issue for us for now.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Looks exactly correct to me.

Whoever wrote that (me) should have checked the return value on \Drupal\Core\Routing\RouteMatchInterface::getRouteObject() which clearly indicates it can be NULL.

This may be tricky to test, but we should try! Marking NW for that.

acbramley’s picture

acbramley commented

@tim.plunkett awesome! I'll have a bit of a poke around and try and come up with a test.

acbramley’s picture

acbramley commented
Status: Needs work » Needs review
StatusFileSize
new 3104980-4-test-only.patch1.55 KB

I've been trying to work on this on and off, I think a Kernel/Unit test would probably be best but I was playing around with this Functional test too. I couldn't get it to fail as expected but here it is so far.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work
Issue tags: +Blocks-Layouts
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

This appears to happen, for instance, when indexing search_api items via drush that might be rendering a layout-builder-based page.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented 
index daf54a4f78..bf6f4fe18b 100644
--- a/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php

I think this test instead of being functional (which on request will always have a route object), if it were to instead be a kernel test and attempt to render a node that is layout-builder-enabled, it would fail as expected.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

josephdpurcell’s picture

josephdpurcell commented
Status: Needs work » Needs review
Related issues: +#3089913: null route object in big_pipe requests after 8.7.8 upgrade, +#3116623: Error: Call to a member function hasRequirement() on null, +#3158327: InvalidArgumentException while trying to render item (entityidhere) with view mode full for search index Acquia Search Solr Index: Route required in Drupal\Core\Url::fromRouteMatch(), +#3101533: Indexing a rendered entity which contains a breadcrumb using Search API results in a fatal error, +#3158332: InvalidArgumentException while trying to render item (entityidhere) with view mode full for search index Acquia Search Solr Index: Route required in Drupal\Core\Url::fromRouteMatch()

I ran into this issue after upgrading from 8.7.7 to 8.8.8. I am using Search API (with SOLR) to index the rendered content.

Thank you for this patch. After applying #2 I can confirm the error went away during indexing.

I'm linking other tickets that I also ran into errors with, or found to describe similar symptoms / resolution. It would be convenient if Core had a way to return an object still instead of null, but no idea if thats possible -- I didn't see a ticket in core about this.

It's unclear why this is "needs work", so setting back to "needs review" just in case that was done on accident.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work

The patch needs automated tests. #5 didn't fail, so that was insufficient.

AmiOta’s picture

AmiOta commented

Hi,

I have the same problem on my drupal 8.9.x with the module Sarch API installed, the patch #2 solved the issue for me.

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

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

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.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Issue tags: +#pnx-sprint
thomaswalther’s picture

thomaswalther commented

I have drupal 8.9.13 and search_api 8.x-1.19

patch #2 do not worked for me still got the error.

Error: Call to a member function id() on string in /webroot/modules/contrib/easy_breadcrumb/src/EasyBreadcrumbBuilder.php on line 561 #0 /webroot/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php(83): Drupal\easy_breadcrumb\EasyBreadcrumbBuilder->build(Object(Drupal\Core\Routing\RouteMatch))
#1 /webroot/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php(72): Drupal\Core\Breadcrumb\BreadcrumbManager->build(Object(Drupal\Core\Routing\CurrentRouteMatch))
#2 /webroot/core/modules/block/src/BlockViewBuilder.php(171): Drupal\system\Plugin\Block\SystemBreadcrumbBlock->build()
#3 [internal function]: Drupal\block\BlockViewBuilder::preRender(Array)
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

That error message points to the easy_breadcrumb contrib module

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Status: Needs work » Needs review
StatusFileSize
new 3104980--18--FAIL.patch1.12 KB
new 3104980--18.patch2.19 KB

Here's a go at test coverage. I stumbled upon the rarely used NullRouteMatch class, which looks perfect for us. Lots of false starts before I found that, though.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
StatusFileSize
new 3104980--19--FAIL.patch1.12 KB
new 3104980--19.patch2.19 KB

Let's try again...

The last submitted patch, 19: 3104980--19--FAIL.patch, failed testing. View results

acbramley’s picture

acbramley commented
Issue tags: -Needs tests

Nice find! Looks good to me!

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Test is great, looks ready

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented

It looks to me like the FAIL patch is set to "re-tests every 2 days". Is there a simple way to indicate that it's the other patch that should instead be tested?

andypost’s picture

andypost
he/him
Primary language Russian
commented

Fixed, just set retest to required patch

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +Bug Smash Initiative

Committed ef493a7 and pushed to 9.3.x. Thanks!

As there is little risk of disruption here, backported to 9.2.x

  • larowlan committed e13e559 on 9.2.x 
    Issue #3104980 by danflanagan8, acbramley:...
  • larowlan committed ef493a7 on 9.3.x
    Issue #3104980 by danflanagan8, acbramley:...

Status: Fixed » Closed (fixed)

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