Canvas Doctor cron run crashes with "Call to a member function hasTranslation() on null" when an entity is in Trash, and cron never completes again

Overview

  • Project: Canvas
  • Component: Code (Health / Doctor)
  • Category: Bug report
  • Priority: Major (one entity in Trash stops cron on the whole site)
  • Version: 1.x-dev (found in 1.12.0)
  • Tested with: Drupal core 11.4.9, PHP 8.5.4, Trash 3.1.0 (drupal/trash)

Problem/Motivation

Drupal\canvas\Hook\HealthHooks::cron() runs Doctor::runCronWindow(), which validates, among other things, the past revisions of every content entity that has a Canvas component tree field (Inventory::pastRevisionItems() → Doctor::runContentPastRevisionsCheck()). When such an entity is soft-deleted with the Trash module, cron fails every time with a fatal Error:

Error: Call to a member function hasTranslation() on null in
Drupal\path\Plugin\Validation\Constraint\PathAliasConstraintValidator->validate()
(core/modules/path/src/Plugin/Validation/Constraint/PathAliasConstraintValidator.php, line 55)
#0 core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php(231): ...->validate()
...
#7 core/lib/Drupal/Core/Entity/ContentEntityBase.php(520): Drupal\Core\TypedData\TypedData->validate()
#8 modules/contrib/canvas/src/Health/Doctor.php(442): Drupal\Core\Entity\ContentEntityBase->validate()
#9 modules/contrib/canvas/src/Health/Doctor.php(520): Drupal\canvas\Health\Doctor::{closure:...runContentPastRevisionsCheck():433}()
#10 modules/contrib/canvas/src/Health/Doctor.php(433): Drupal\canvas\Health\Doctor->process()
#12 modules/contrib/canvas/src/Health/Doctor.php(330): iterator_count()
#13 modules/contrib/canvas/src/Hook/HealthHooks.php(37): Drupal\canvas\Health\Doctor->runCronWindow()
#14 core/lib/Drupal/Core/Cron.php(275): Drupal\canvas\Hook\HealthHooks->cron()

Why it happens, checked on a site with two nodes in Trash:

  • For an entity in Trash, getQuery()->currentRevision() and ->latestRevision() return nothing (Trash hides the entity), so the "default" and "latest" ID lists in Inventory::pastRevisionItems() do not contain it.
  • getQuery()->allRevisions() still returns its older revisions (those rows have no deleted timestamp, only the newest revision gets one). Example from the site: allRevisions for nodes 59 and 60 returns revisions 254, 255, 256 and 257, 258, while the default revisions 259 and 260 are hidden. Inventory therefore queues them as "past revisions".
  • $storage->loadRevision(254) loads them as non-default revisions. validate() runs the core PathAliasConstraint, which for a non-default revision calls loadUnchanged($entity->id()) to compare the alias. For an entity in Trash that returns NULL, and $original->hasTranslation() is called on it.
  • Error is not an Exception, so nothing in Cron::invokeCronHandlers() catches it. The whole cron run is aborted.

Consequences: no cron run completes any more (system.cron_last does not change; on the reporter's site it stood still for about 64 hours). Everything that depends on cron stops: search indexing, queue workers, update checks and more. With Automated Cron the crash is repeated on nearly every request, and it also aborts the dispatch of kernel.terminate, so listeners behind Automated Cron are never called.

The failing validator is core code and is not null-safe (see "Remaining tasks"), but Canvas is the one that hands it revisions of entities that no longer exist for the rest of the system.

Steps to reproduce

  1. Install Drupal core 11.4.x, Canvas 1.12.0 and Trash 3.1.0 and enable Trash for a revisionable entity type that has a Canvas component tree field (for example nodes of a content type with the Canvas field).
  2. Create an entity of that type and save it at least twice, so it has more than one revision.
  3. Delete the entity. With Trash it stays in the database and is hidden.
  4. Run cron (drush cron, or wait for Automated Cron).
  5. Cron aborts with Call to a member function hasTranslation() on null (see the log type php). system.cron_last is not updated.

(Not reproduced on a fresh site by the reporter: the steps follow what was analysed on the production site described above.)

Proposed resolution

In Inventory::pastRevisionItems(), only queue the past revisions of entities that still have a loadable default revision, that is, entities whose ID is in the currentRevision() result:

$live_entity_ids = \array_fill_keys(
  \array_map('strval', \array_values($default_rid_map)),
  TRUE,
);
...
if (!isset($excluded_rids[$rid]) && isset($live_entity_ids[$entity_id])) {
  $past[] = new CheckEntity($entity_type, $entity_id, revisionId: $rid);
}

An entity in Trash is not shown to visitors, and its default revision is not checked either, so skipping its past revisions is consistent. When the entity is restored from Trash, the past revisions are checked again in a later cron window.

The patch (applies to 1.12.0, src/Health/Inventory.php only, 12 lines added and one line removed) is below. After applying it on the reporter's site, cron completed again (Cron run completed., system.cron_last updated) with the two nodes still in Trash. Not tested against 1.x-dev.

canvas-doctor-skip-trashed-entities-past-revisions.patch
diff --git a/src/Health/Inventory.php b/src/Health/Inventory.php
index bcf50d8..b19d363 100644
--- a/src/Health/Inventory.php
+++ b/src/Health/Inventory.php
@@ -313,12 +313,23 @@ final readonly class Inventory {
         TRUE,
       );
 
+      // Entities without a loadable default revision (for example,
+      // soft-deleted entities sitting in Trash) cannot be validated: doing
+      // so requires loading the current entity via loadUnchanged(), which
+      // will return NULL for them. Skip their past revisions entirely,
+      // rather than letting validation crash on a NULL entity.
+      // @see \Drupal\path\Plugin\Validation\Constraint\PathAliasConstraintValidator::validate()
+      $live_entity_ids = \array_fill_keys(
+        \array_map('strval', \array_values($default_rid_map)),
+        TRUE,
+      );
+
       // Revision queries return revision_id => entity_id.
       $all_rid_map = $storage->getQuery()->accessCheck(FALSE)->allRevisions()->execute();
       foreach ($all_rid_map as $rid => $entity_id) {
         $rid = (string) $rid;
         $entity_id = (string) $entity_id;
-        if (!isset($excluded_rids[$rid])) {
+        if (!isset($excluded_rids[$rid]) && isset($live_entity_ids[$entity_id])) {
           $past[] = new CheckEntity($entity_type, $entity_id, revisionId: $rid);
         }
       }

Remaining tasks

  • Add a kernel test: an entity with two or more revisions that is soft-deleted by Trash must not end up in the past revision items (and Doctor must not throw). The reporter has not written or run such a test.
  • Decide whether Trash should be an optional test dependency, or whether Inventory should guard against missing default revisions in a way that does not mention Trash (for example by comparing with the IDs returned by the default revision query, as in the patch).
  • Related core problem: PathAliasConstraintValidator::validate() does not check the result of loadUnchanged() for NULL. A separate core issue could be filed; it does not remove the need for the Canvas change, because the revisions of such entities are not meant to be validated.
  • Check the other revision queries in Inventory (forwardRevisionItems()) for the same assumption.
  • Confirm the behavior on 1.x-dev (the patch was only tested on 1.12.0).

User interface changes

None.

API changes

None. Inventory::pastRevisionItems() returns fewer items when entities are in Trash.

Data model changes

None.