Issue #3137872 introduced a bug, if a Drupal block doesn't define cache metadata in the build method the page breaks with:

TypeError: Argument 1 passed to Drupal\Core\Cache\CacheableMetadata::setCacheTags() must be of the type array, null given, called in /app/web/modules/contrib/gutenberg/src/Plugin/Filter/BlockFilter.php on line 77

The cacheabilityMetadata function should check if #cache is set in $build, and if not fall back to using available methods for the block to get that metadata. (Or merge the two? I'm not sure what the standard behavior is.)

CommentFileSizeAuthor
#2 3153392-2.patch1.27 KBcantrellnm

Comments

cantrellnm created an issue. See original summary.

cantrellnm’s picture

cantrellnm commented
StatusFileSize
new 3153392-2.patch1.27 KB

This patch assumes #cache data in $block->build() should override cache data from functions like $block->getCacheTags().

cantrellnm’s picture

cantrellnm commented
Status: Active » Needs review
martin_andersn’s picture

martin_andersn commented

Hi!
I will attempt to replicate this bug first. Any more information you can provide me towards this goal would be greatly appreciated!

What Drupal version did you run it with?
What other modules, both active not active worth mentioning?
Composer used for install ?

can you check if this problem still persist after recent core updates and Gutenberg module updates?

I love that you provided at patch in the same breath as you reported the bug! First i need to replicate the issue and then test the patch if the above question indicates the problem stil exist.

Thank you for your continued interest and input on this project :)

Best, Martin Andersen

cantrellnm’s picture

cantrellnm commented

Thanks for looking into it, Martin.

I'm using Composer for install, and just replicated the error with Drupal 8.9.2 and Gutenberg 1.11 with the patch from issue 3137872 that was already committed but not released in the 1.x branch. Replicated also with Drupal 8.9.3 and Gutenberg 2.0-beta2 (no patch since the commit is included in that release).

It happens if you're adding a Drupal plugin-type block to a node through Gutenberg where the block's php class doesn't use a #cache key in the array returned by its build() function. The block works on the node edit form, but then the page breaks after saving when trying to view the node. The cacheabilityMetadata() function in src/Plugin/Filter/BlockFilter.php assumes that $build['#cache'] is set, but it doesn't have to be. And then that null value in the $metadata array gets used as a parameter for setCacheTags() and throws this error.

There are probably contrib modules where this could happen, but in my case it was a custom module defining a block plugin. Something as simple as the example block at https://www.drupal.org/docs/8/creating-custom-modules/creating-custom-blocks/create-a-custom-block would work to replicate to problem.

martin_andersn’s picture

martin_andersn commented

hi cantrellnm.

Excellent! you provide very detailed instructions on the steps to reproduce, big +! :D

## 'src/Plugin/Filter/BlockFilter.php' - Gutenberg version: '8.x-2.0-beta2':

private function cacheabilityMetadata($text) {
    $metadata = [
      'tags'     => [],
      'contexts' => [],
      'max-age'  => 0,
    ];

    preg_match_all('#<!-- wp:drupalblock\/.*\s(\{.*\})\s\/-->#', $text, $matches);

    if (!empty($matches[1])) {
      foreach ($matches[1] as $match) {
        $attributes = json_decode($match);
        $block      = $this->blocksRenderer->getBlockFromPluginId($attributes->blockId, []);
        $build      = $block->build();

        $metadata['tags']     = array_merge($metadata['tags'], $build['#cache']['tags']);
        $metadata['contexts'] = array_merge($metadata['contexts'], $build['#cache']['contexts']);

        if ($build['#cache']['max-age'] < $metadata['max-age']) {
          $metadata['max-age'] = $build['#cache']['max-age'];
        }
      }

      $metadata['tags']     = array_unique($metadata['tags']);
      $metadata['contexts'] = array_unique($metadata['contexts']);
    }

    return $metadata;
  }
}

Are you still getting a null thrown if using Gutenberg version: '8.x-2.0-beta2' ?

Thinking out loud here, but if problem stil presist maybe something like:
array_replace_recursive (SOME_VALUE $metadata['tags'], $build['#cache']['tags'] ): SOME_VALUE
Will investigate this.

Keep you posted.
.m

cantrellnm’s picture

cantrellnm commented

Yes the issue is also in 8.x-2.0-beta2. Using array_replace_rescursive() sounds fine to me, but doesn't fix the problem that $build['#cache']['tags'] may not be defined. A block doesn't need to use a '#cache' key in the array it returns from the build() function.

Patch in #2 fixes that problem, and falls back to using the block's cache data from functions like $block->getCacheTags() if needed. Or if we want cache data from both the $build array and block functions, something like this might work (assuming PHP 7):

$metadata['tags'] = array_replace_recursive($metadata['tags'], $build->getCacheTags(), $build['#cache']['tags'] ?? []);

nimoatwoodway’s picture

nimoatwoodway
Location Tyrol / Austria
commented

Confirm. Patch in #2 fixes this issue for 8.x-2.0-beta2.

thorandre’s picture

thorandre commented
Assigned: Unassigned » marcofernandes

This is relevant to https://www.drupal.org/project/gutenberg/issues/3213784

Marco is working on that and will evaluate the patch here also. We should see a solution for this by the end of next week.

szeidler’s picture

szeidler commented
Version: 8.x-1.x-dev » 8.x-2.x-dev

Any further development goes against 8.x-2.x

szeidler’s picture

szeidler commented
Status: Needs review » Fixed

Since Drupal 8.x-2.0 the problem seems to be solve, because of the major restructuring of the block processor. I think we can close this issue therefore. Feel free to create another issue, if you still experience the issue.

Marking it as fixed to grant credits for the patch + reviews.

  • szeidler committed 41e5fa9 on 8.x-1.x authored by cantrellnm 
    Issue #3153392 by cantrellnm, martin_andersn, nimoatwoodway: Page breaks...
szeidler’s picture

szeidler commented
Version: 8.x-2.x-dev » 8.x-1.x-dev

Status: Fixed » Closed (fixed)

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