Problem/Motivation

We've been dealing with exception issues in Custom Breadcrumbs, specifically in cases where it's being used to generate token strings for metatags. Due to the fact that it might not necessarily involve a valid route match from the current entity's canonical route, this means the TitleResolver might not be able to resolve the arguments for methods like `NodeViewController::title`, resulting in a RuntimeException being thrown.

This is probably a very roundabout way of hitting this problem, but we routinely use breadcrumb data in Metatags, whether it be to render out the breadcrumb for metatags or determine the main section of the site that a given page is in. If it's throwing exceptions in JSON:API that means there's a whole class of Drupal core functionality unavailable to us.

I've discussed this with @larowlan and we think that broadening the exception handling around the TitleResolver that was introduced in #3184641 might handle these cases. I can understand any reticence to expand this type check, but returning a NULL for the appended title is probably more useful than passing an exception further up the call stack.

Steps to reproduce

This is a very simplified version of the flow of events that we think might reproduce this:

  1. Set up Custom Breadcrumbs with some rules
  2. Implement a token to invoke a breadcrumb builder for entity type node
  3. Set up node Metatags to use this token
  4. Request a node of the given type via JSON:API

Proposed resolution

Broaden the exception handling around TitleResolver to catch \Throwable instead of \InvalidArgumentException.

Remaining tasks

  • Implement patch
  • Implement additional tests if required

User interface changes

None.

API changes

None.

Data model changes

None.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

geoffreyr created an issue. See original summary.

geoffreyr’s picture

Status: Active » Needs review
geoffreyr’s picture

We've started running this patch in PRODUCTION and it has alleviated the issue described in the description above.

geoffreyr’s picture

Issue summary: View changes

lamp5 made their first commit to this issue’s fork.

lamp5’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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