An error occurred while trying to build an inline entity from submitted form values

Are there any details about the log message you're reporting here? Something like a stack trace or at least a php file and line number where an error occurs?

In /var/log/apache2/error.log is nothing recoreded and Drupal logs only this:

Hmm, I've tried, but I can't reproduce the error. The error message comes from \Drupal\eca_form\HookHandler::inlineEntityFormAfterBuild which doesn't do anything when the request is an ajax request. But clicking on Add another item is exactly that, an ajax request.

So, we need additional information to reproduce and analyse that problem.

Ah, wait a minute, a cache rebuild helped me to get the error now as well. I can investigate further now.

Here is the exact error that occurs:

Drupal\Core\Field\WidgetBase::massageFormValues(): Argument #1 ($values) must be of type array, string given, called in /var/www/html/web/core/lib/Drupal/Core/Field/WidgetBase.php on line 505

$values = $this->massageFormValues($values, $form, $form_state);

At that point, field name is field_tags and $values is _none.

The $values is received from $values = NestedArray::getValue($form_state->getValues(), $path, $key_exists); while path has the following value:

$path = array
(
    [0] => field_basic
    [1] => 0
    [2] => inline_entity_form
    [3] => field_tags
)

It gets to the same code again soon after where path is just array([0] => field_tags), which is an array as expected. So, my assumption is, it processes all this once for the parent form and then for the IEF. The former fails, the latter succeeds. If that assumption is correct, then we need to prevent the processing for the parent form.

Assumption is NOT correct. I've removed the field_tags from the parent form, so it now only gets to that point once with the path field_basic / 0 / inline_entity_form / field_tags. This is the correct path from the parent form to the field field_basic, the entity reference to basic page and from there down to the field_tags which returns a string instead of an array.

Not sure what we can do about it. Seems like an issue in core since the above methods are massaging the form values badly. The autocomplete field widget has a default value null and returns an empty array, which is correct. Whereas checkboxes or select list field widgets have an empty array as default value, but that returns the string _null, which is incorrect.

There you go, if I change the cardinality of the field_tags from 1 to unlimited, then the error goes away, as the $values = NestedArray::getValue($form_state->getValues(), $path, $key_exists); then returns an array as expected. Makes me think even more that this is an issue in core.

Yep, there are 48+ issues in core that are related to massageFormValues.

For details, see https://www.drupal.org/project/issues/drupal?text=massageFormValues&stat... →

Thank you very much for the detailed answers, impressive as always!
I'm gonna try your advice from #9 and will hide the "Add another item" button. Since the form will be used internally, this trick is enough.

Thanks again for all your work ~

The exception message was introduced for being able to identify bugs like this, while preventing that the site stops working. Therefore treating as a bug makes more sense.

Agreed, if there's anything we can do about it, let's do it. I was just under the impression there's something wrong in core and I could find a solution or even workaround.

I'll mark this as a task, until we know that this is indeed an ECA bug that is actually fixable.

It is a bug, as explained in #12. The origin may be somewhere else, but it occurs here and only when ECA is enabled. I currently don't understand the logic of issue type handling, especially regarding found bugs.

As I described in #6, core uses $values = $this->massageFormValues($values, $form, $form_state); and assumes, that $values contains an array afterwards. That is a core bug in the first place.

The exception captured in ECA only happens because of that, and I am not convinced that we have any influence over that massageFormValues method to prevent that from happening. Or am I missing something @mxh?

The affected code block where the exception is being thrown is currently implemented as the following:

<?php
    try {
      $form['#entity'] = clone $entity;
      /** @var \Drupal\inline_entity_form\InlineFormInterface $handler */
      $handler = \Drupal::entityTypeManager()->getHandler($form['#entity']->getEntityTypeId(), 'inline_form');
      $handler->entityFormSubmit($form, $form_state);

      $info = $form['#eca_ief_info'];
      $form_state->set([
        'eca_ief',
        $info['parent'],
        $info['field_name'],
        $info['delta'],
      ], $form['#entity']);
    }
    catch (\Throwable $t) {
      \Drupal::logger('eca_form')->error("An error occurred while trying to build an inline entity from submitted form values. This might be a problem in case you use ECA to extend inline entity forms. Please report this to the ECA issue queue to help us improving it.");
    }
?>

... where the part $handler->entityFormSubmit($form, $form_state); is triggering something that will most probably lead to the error. This calling part is a potential problem because it may call other components too early and they may not be ready for that instruction yet. This is due to the complex form building, validation and submission system in general, but at that place we need an entity with the most recent values. As it's a potential problematic call, it was wrapped intentionally with that exception handling. It is ECA that is causing the error. Furthermore, it is a runtime problem and not a problem of static code.

Yes, I understand that and it's good that we have that shout out mechanic included so that this creates awareness.

In addition to that, can you agree that the following code is a bug as \Drupal\Core\Field\WidgetInterface::massageFormValues restricts $values to be an array while \Drupal\Component\Utility\NestedArray::getValue returns mixed types and nothing ensure values to be an array?

$values = NestedArray::getValue($form_state->getValues(), $path, $key_exists);
$values = $this->massageFormValues($values, $form, $form_state);

In addition to that, can you agree that the following code is a bug as \Drupal\Core\Field\WidgetInterface::massageFormValues restricts $values to be an array while \Drupal\Component\Utility\NestedArray::getValue returns mixed types and nothing ensure values to be an array?

Im trying to explain, feel free to ask if you don't understand or need more clarification. I know this is a complex part.

I don't generally categorize places where a fatal may occur as a bug. Just because something throws a fatal error, does not always mean the place where it happens is the place where the but resides. There are valid cases where a fatal error to be thrown is intentional, for example when passing along invalid parameter values. It then enforces triggering / invoking parts to fix passing of invalid arguments or wrong times of invocation. That's basically my point of #17, that this is a runtime problem.

The code part you ask me about in #17 is extracted from Drupal\Core\Field\WidgetBase::extractFormValues::extractFormValues right? The difficulty here is that this is one of the most complex layers of Drupal, the form API, which is extremely dependent on the correct timings and order of invocations, and where it's extremely hard to see what the right thing is to do in what circumstance. I personally don't like it and find it too complex. I also still struggle with it when working with it. But it is what it is, and when making use of it, the triggering part needs to be fixed that is will do the right calls at the right time and in the right order.

Thanks for explaining. I can follow your logic and agree with your conclusions. However, in the context of safe coding, the code block in core should still do something about that potential type inconsistency and handle that explicitly, rather than implicitly assuming the type would be correct. There is even an issue about it (see 🐛 WidgetBase::extractFormValues relies on non guaranteed return value Needs work ) and at least 2 other use cases came across the same problem. The patch there would actually avoid the situation we face here as well, but as you say, this doesn't mean it's the correct approach.

The problem is that the submit instruction $handler->entityFormSubmit($form, $form_state); is being called so early that form validation has not yet happened. However form validation is the layer where submitted form values are being converted to be compatible for the massageFormValues instruction later on. One example is Drupal\Core\Field\Plugin\Field\FieldWidget\OptionsWidgetBase::validateElement where the last instruction "normalizes" the submitted values with $form_state->setValueForElement($element, $items);

Created an MR !440 that adds in a manual form validation call. I hope that it doesn't break anything else - it's hard to tell.

This is impressive, so glad you know your way around the form API @mxh.

The MR looks good regarding the code itself. Leaving the issue at NR, though. Hoping that @gge can actually test this in his context to see, if the messages disappear.

Hello,

I can confirm that in my context the message disappear. I'm gonna stress the inline entity form build event for a while and will report of there will be any problems. For now everything seems to work very good!

Thank you very much!

In that case, this is RTBC for the current issue. @mxh how do you feel about merging this into 2.1 and back porting to 2.0? If follow-ups occur, we can still open new issues then.

Fine for me I guess

Thanks. Merged and back ported.

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

ATTENTION: this fix causes trouble with the complex IEF widget, see details at 🐛 ECA Form breaks complex IEF widget Active .