Problem with function _field_invoke when used on a content type with no field instance

I confirm the existence of the bug in 7.x-dev. _field_invoke_get_instances might return empty arrays. Subsequently _field_invoke just assumes it contains a field name.
In my particular situation it results in the following error. It renders imports completely unusable for fieldable entities.

Fatal error: Cannot access empty property in /var/www/dev/modules/field/field.attach.inc on line 197

Call Stack:
Entity->save( ) ../mymodule.module:550
EntityAPIControllerExportable->save( ) ../entity.inc:168
EntityAPIController->save( ) ../entity.controller.inc:694
EntityAPIController->invoke( ) ../entity.controller.inc:356
field_attach_insert( ) ../entity.controller.inc:257
_field_invoke_default( ) ../field.attach.inc:898
_field_invoke( ) ../field.attach.inc:375

subscribing

According to http://drupal.org/node/45111, this bug qualifies as major.

The error is avoided, when I add the following code to to modules/field/field.attach.inc:188

if (!isset($instance['field_name']))
    continue;

But then a secondary error occurs.

Notice: Undefined index: field_id in field_attach_insert() (line 916 of /var/www/dev/modules/field/field.attach.inc).

When I add a similar piece of code there, it all works fine.
Their main divider in the call-stack is _field_info_collate_fields() in modules/field/field.info.inc. I suspect this function to be the source of the bug, but I don't have enough insight into the Field module to figure it out.

In my case, the Array returned for my content type is empty (no field instance in the array).

So, at line 189 of the _field_invoke function ($field_name = $instance['field_name'];) :
The instruction is not valid because the offset 'field_name' doesn't exist.

This isn't possible:

 $instances = _field_invoke_get_instances($entity_type, $bundle, $options);

  // Iterate through the instances and collect results.
  $return = array();
  foreach ($instances as $instance) {
    $field_name = $instance['field_name'];
    $field = field_info_field($field_name);
    $function = $options['default'] ? 

If $instances is an empty array, then the foreach will never reach that section of code. Something else might be going on, but the description doesn't sound right to me at all.

Please post the output of var_dump($instances); before it goes into that loop, and var_dump($field) just before the notice is thrown.

I'm seeing this on user add pages and order pages in Ubercart.
I'm using Ubercart 3 and Drupal 7 and get this error when I tail httpd error_log.

PHP Fatal error: Cannot access empty property in modules/field/field.attach.inc on line 197

The page fails to render I just get a 500 error. Refreshing the page I came to before the error presents me with

Notice: Undefined index: field_name in _field_invoke() (line 188 of /var/www/html/kelderman/drupal/modules/field/field.attach.inc).

The error is easy to re-produce in 7.x. I don't know about 8.x.

Say a content type does not have a field named "field_random". But you want to display the field with

$node = node_load(xx);
$output = field_view_field('node', $node, 'field_random');

You got "Fatal error: Cannot access empty property".

This is because in the function _field_invoke_get_instances()

$instances = array(field_info_instance($entity_type, $options['field_name'], $bundle));

generates array(0 =>NULL).

Additions:

The field "field_random" needs to be existing but doesn't bundle to the content type.

Drupal 8 is the same.

I have a simple patch here - adding an "array_filter()" because field_info_instance() can return NULL.

No patch for the tests yet.

Why would you have code that does:

<?php
$node = node_load(xx);
$output = field_view_field('node', $node, 'field_random');

in the first place? If the code is wrong we shouldn't silently fail.

I can confirm this bug exists, and currently bugs me with Views. Fatal errors aren't a nice way to fail, even though it could be argued the code is „wrong“.
Attached is my patch that fixes this, which seems to me to be slightly more straight-forward than #12, but I'd be fine with either. In any case, the patch applies cleanly on both D7 and D8.

@ catch: The context this occurs in for me is, as said, in Views. I add a field that is only present on some content types to the view. The view calls field_view_field() -> BOOM!
Of course, now that I know this, I can just check whether the field is present – but this was quite a drag to debug, and if we can fix it that easily, why not? Nothing's gonna explode just because we silently fail when people want to view a non-existing field. On the contrary, things are already exploding when we throw fatal errors all over the place in such a case.

Well what if I'm developing and remove a field instance, with current head it inform me that I left a crufty call to field_view_field() in, now there is no warning at all. Throwing an exception rather than the fatal might be fine though.

If the field doesn't exist, it just displays nothing without errors. Because you got

if ($field = field_info_field($field_name)) {

in the function field_view_field(). Only if the field exists but there's no instance, it fails. IMO It would be logically better if we have something like

if ($field = field_info_field($field_name) && there_is_an_instance_for_the_bundle) {

But it's not easy to check it there. And I think it doesn't make sense for the function _field_invoke_get_instances() to return an array(NULL). Thus the patch.

But it's not easy to check it there. And I think it doesn't make sense for the function _field_invoke_get_instances() to return an array(NULL). Thus the patch.

Yes, code-wise at least this makes no sense. You could argue that we should throw an exception or flag an error in other ways, but returning false results, not matching the function's contract, surely isn't the way to go.

And yes, if you don't get any warning or error for completely inexistent fields, this is additionally inconsistent. To me, this has much more the appearance of being an oversight than really having the attention to fail and inform the user.
Also, for lots of other code we also don't have any errors in such cases. Try loading a number of nodes with partly inexistent NIDs, for example. When developing, just watch what you're doing and you're fine.

I ran into this problem with Views too.

Here is the patch rerolled for D8, and with tests. The tests help demonstrate the inconsistency in the current behavior; if we want there to be an error thrown in this scenario that might be reasonable (although probably D8-only), but the fact that with the current codebase the first test returns an empty array and the second throws a PHP fatal error definitely doesn't make sense, so for now they should both return an empty array for consistency (which is what the patch makes them do).

By the way, here's the Views issue I came across: #1461536: Fatal error when using aggregation and a field is not attached to all entities in the view

It seems like it's possible to work around this in Views if necessary, but I think it's a core bug so it's better to be fixed here.

Ran into this with a client project as well. The patch looks good and fixes the issue and the tests make sense to me.

Rerolled for PSR-0 tests and removed t() from assertion messages.

Leaving at RTBC.

OK I can live with this, but can we at least add a watchdog() call in the 8.x patch so there's not a silent failure here?

Then we'd have to build in logic that determines if parameters are invalid, of if a bundle has no field attached yet, or if the field doesn't exist. I don't agree with adding a watchdog call here because an empty array is valid output.

I don't think we need all that.

+    $instance = field_info_instance($entity_type, $options['field_name'], $bundle);

If this doesn't return anything, then log to watchdog.

Hrm, I just agree to disagree. Tim, David: opinions?

To help myself decide what I thought about logging something to watchdog, I thought about what it would say.
The best I came up with was "This content type has no fields, thought you should know."
There's nothing wrong with having no fields on a content type.

I think this is fine as is.

The problem isn't that you have a bundle with no fields, it's that you're trying to render a field that doesn't have an instance on a content type (or retrieve an instance on a bundle that doesn't have the field assigned). That's exactly the sort of error which can lead to headbanging and debugging so it'd be good to give people a clue that they're doing something strange. Also field API is full of very specific exceptions that do this in lots of other circumstances.

If we could throw a fatal error previously, I don't see why it's impossible to log some kind of error now. Back to CNW one more time.

(OP coding format+)

19: 1161708-field_invoke-trouble-19.patch queued for re-testing.

Is this issue still relevant for Drupal 8?

The problem isn't that you have a bundle with no fields, it's that you're trying to render a field that doesn't have an instance on a content type (or retrieve an instance on a bundle that doesn't have the field assigned).

That is true if you trigger this via field_view_field(), but aren't there many ways to call this function besides that, any many of them aren't error conditions?

So at first glance I don't think we'd want a watchdog() call or similar inside _field_invoke_get_instances() itself. Maybe inside field_view_field()...

This is no longer relevant for Drupal 9, moving back to Drupal 7.