Infinite recursion on field_group_field_info_max_weight()/field_group_info_groups()

This also fatals in a drush cim -y that enables field_group as part of the imported config when a module has defined other fields.

I've also got this error, and, if we disable xdebug from php.ini, we will receive "PHP Fatal error: Maximum execution time of %number seconds exceeded" with the following PHP crushing.

Few ways how to fix it:

1) Check if your updates through update.php/drush updb have been finished correctly, especially if system_post_update_extra_fields() has executed. If it hasn't, try to re-run your updates and pay attention to errors if they appear.
2) Check your hook_entity_extra_field_info() for weight parameter existence. In my case, it was absent and caused the mentioned bug. Your hook_entity_extra_field_info() should look like this:

function test_entity_extra_field_info() {
   $extra['node']['test_type']['display']['test'] = [
     'label' => t('Test label'),
     'weight' => 0,
   ];
 
   return $extra;

Hope it will help you.

I ran into this issue in a roundabout way - while saving a node with the menu settings populated, the new menu item was run through Lingotek, which loaded view display and triggered field_group_field_info_max_weight() and set off the infinite loop (and subsequently caused any node save to fail if that field was populated). I've got a patch that checks for recursion and shuts it down based on similar work here:

https://www.drupal.org/project/drupal/issues/2966137

Marking needs review since the failure was from the branch, not the patch.

I'm not sure this is the right way to fix this.

From what I can tell, the problem happens because of this:

- 1. when an entity display config entity (ie view or form configuration) is loaded, it sets up all of its components.
- 2. if a component doesn't specify a weight, then it puts it at the bottom:

    // If no weight specified, make sure the field sinks at the bottom.
    if (!isset($options['weight'])) {
      $max = $this->getHighestWeight();
      $options['weight'] = isset($max) ? $max + 1 : 0;
    }

3. getHighestWeight() checks all the components' weights. As part of that, it invokes hook_field_info_max_weight() to let other modules act.
4. field_group's implementation of this, field_group_field_info_max_weight(), calls field_group_info_groups() to load all the groups
5. in order to load the groups, it loads up the display entity, EntityFormDisplay::load/EntityViewDisplay::load
6. and BOOM we're back at step 1

I think the problem is that field_group_field_info_max_weight() needs the display config entity in order to act. Arguably, hook_field_info_max_weight() should be providing it as a parameter -- which would be something to fix in core.

IMO, infinite recursion under any circumstances isn't acceptable, and this fixes that in a definite way without losing anything. It'd just be cruft if a theoretical core solution comes along, but in that case, we'll be updating this code anyway and can axe it then. In the meantime, this solves a critical issue for a heavily used module. That said, the patch is there for anyone who wants it.

I've encountered this issue on version 1.x of field_group, I made the same changes as the patch in #7, but for version 1.x.
It solves the problem for me.

I've attached the patch for anyone who is also still using 1.x

@gambry's question from #12 is important.

Why instead of returning NULL don't we read directly from the config entity?

That was my initial thought as well, but not sure whether that is worth the effort. Will talk with alex whether if that's a good idea or not.

Talked with some other people and this approach is fine, so will commit and push. Made a tiny change in the comments: used 'entity display' instead of 'entity' alone.

I'd +1 #11 as this is critical, however currently field_group_field_info_max_weight is returning NULL when a recursion is triggered. This is not ideal if actually there is a field_group with a valid weight?

This is actually fine, NULL is returned for the second time this hooks comes in, but that doesn't matter. When it can finally go through, it returns the max weight fine - unless I saw it wrong in my debugger, but even then I don't mind /that/ much.

commited and pushed, thanks!
pushed to 8.x-1.x as well

I have recently encountered this issue with the update to Drupal 9.4.0 with a base field that does not specify weight in it's form display.

I'm seeing same as Adam Delaney. The workaround is to specify the weight.

For some reason $recursion_check is not working as a static variable should. Its always an empty array even on subsequent calls to field_group_field_info_max_weight().

I'm experiencing the same issue as adam-delaney and moshe weitzman in Drupal 9.4.3. Adding a weight fixes the issue for me (as a workaround).

@gambry We are using field_group version 8.x-3.2 and Drupal Core 9.4.3

The error occurs on the ContentEntityForm of a custom content entity type (@ContentEntityType)

in function baseFieldDefinitions we changed the following field definition:

    $fields['reusable'] = BaseFieldDefinition::create('boolean')
      ->setLabel(new TranslatableMarkup('Can be used multiple times'))
      ->setDescription(new TranslatableMarkup('A boolean indicating whether the discount can be applied multiple times.'))
      ->setDefaultValue(TRUE)
      ->setDisplayOptions('form', [
        'type' => 'boolean_checkbox',
      ])
      ->setDisplayConfigurable('form', FALSE);

and added a weight to 'fix' the issue:

    $fields['reusable'] = BaseFieldDefinition::create('boolean')
      ->setLabel(new TranslatableMarkup('Can be used multiple times'))
      ->setDescription(new TranslatableMarkup('A boolean indicating whether the discount can be applied multiple times.'))
      ->setDefaultValue(TRUE)
      ->setDisplayOptions('form', [
        'type' => 'boolean_checkbox',
        'weight' => 10,
      ])
      ->setDisplayConfigurable('form', FALSE);

Actually another perhaps "cleaner" solution would be if `field_group_info_groups()` would not load the complete view mode / form mode, but just the raw config data. But I don't know if that is possible or would cause other problems.

Actually the entire mechanism and hook in core are quite problematic and seem to be not very useful.
The only implementation I could find is from field group.

And I wonder if it ever does anything useful. I would think that it runs into the infinite recursion.