Notice: Trying to access array offset on value of type null in Drupal\views\Plugin\views\display\EntityReference->query()

@robertom Thanks for supplying the patch.

Usually adding empty() checks really just hides an underlying problem. I remember there being some logic to make sure that entity references always have a limit, but your code path might be circumventing that. We really need an automated test to prove that a normal code path indeed triggers the notice.

Maybe this issue is somehow related? https://www.drupal.org/project/entity_reference_views_select/issues/3179436

I got the notice when using a Viewfield in a node with Always use default value checked and a view with entity references.

I have the same notice when using drupal_view() (twig-tweak) in a node.(Drupal 8.9.14)
The patch #2 from @robertom solved the problem.

Patch #2 for D9 works for me.
Thanks, @robertom!

#2 works for 9.1.7. Thanks!

I'm getting the same error even when just opening the view in the views ui. The problem being, that $options is NULL my in case. It gets defined in line 158 inside the same callback:

$options = $this->getOption('entity_reference_options');

and the DisplayPluginBase does have a lot of options, but none for entity_reference_options. I wasn't able to figure out where those options should come from. I wonder if this is related to the entity type being used in that context?

With D.9.2.9 I am getting a similar message:
I created a new view for type user. But without page or block. Just the master display.
I add the email field. And a second Name field, which I overwrite with values like (name and email ...) .
When I try to ADD an entity reference view the following notice appears

Error message

    Notice: Trying to access array offset on value of type null in Drupal\views\Plugin\views\display\EntityReference->query() (line 195 of core/modules/views/src/Plugin/views/display/EntityReference.php).

    Drupal\views\Plugin\views\display\EntityReference->query() (Line: 1301)
    Drupal\views\ViewExecutable->build('entity_reference_1') (Line: 70)
    views_fieldsets_views_ui_display_tab_alter(Array, Object, 'entity_reference_1') (Line: 539)
    Drupal\Core\Extension\ModuleHandler->alter('views_ui_display_tab', Array, Object, 'entity_reference_1') (Line: 381)
    Drupal\views_ui\ViewEditForm->getDisplayTab(Object) (Line: 204)
    Drupal\views_ui\ViewEditForm->form(Array, Object) (Line: 106)
    Drupal\Core\Entity\EntityForm->buildForm(Array, Object) (Line: 41)
    Drupal\views_ui\ViewFormBase->buildForm(Array, Object)
    call_user_func_array(Array, Array) (Line: 532)
    Drupal\Core\Form\FormBuilder->retrieveForm('view_edit_form', Object) (Line: 278)
    Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 48)
    Drupal\Core\Entity\EntityFormBuilder->getForm(Object, 'edit', Array) (Line: 230)
    Drupal\views_ui\Controller\ViewsUIController->edit(Object, 'entity_reference_1')
    call_user_func_array(Array, Array) (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 578)
    Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 158)
    Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 80)
    Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
    Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
    Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
    Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
    Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
    Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 717)
    Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I did not try the patch yet.

The error is gone in the moment I add a PAGE to the view.

Here's a patch that adds a few lines to the existing test for \Drupal\views\Plugin\views\display\EntityReference to try to trigger the error message.

It looks like this warning shows up when the view is executed directly, instead of being called through the ViewsSelection EntityReferenceSelection plugin. So that's what the new part of the test does.

Well, it looks like that test failed perfectly:

1) Drupal\Tests\views\Functional\Plugin\DisplayEntityReferenceTest::testEntityReferenceDisplay
Trying to access array offset on value of type null

/var/www/html/core/modules/views/src/Plugin/views/display/EntityReference.php:195
/var/www/html/core/modules/views/src/ViewExecutable.php:1301
/var/www/html/core/modules/views/src/ViewExecutable.php:1391
/var/www/html/core/modules/views/tests/src/Functional/ViewTestBase.php:125
/var/www/html/core/modules/views/tests/src/Functional/Plugin/DisplayEntityReferenceTest.php:283
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:722

I'm going to remove the Needs tests tag.

I'm not convinced that simply suppressing the error is the right thing to do. What is a use case for executing an EntityReference display outside of the context of ViewsSelection? Is this something that should even be supported?

Setting to NR to get some feedback.

After sleeping on this, I've decided to post a patch that sets default values for entity_reference_options in the code. These match the default values that would typically be applied if the entity_reference view is executed under normal circumstance, i.e. through Drupal\views\Plugin\EntityReferenceSelection\ViewsSelection::getReferenceableEntities(). This seems reasonable to me.

Note that much of the interdiff is related to running only the updated test in #12 and now wanting to run all tests.

Thanks danflanagan8, good solution! For those of us still lagging behind on D8, here's patch for 8.9.19.

Previous patch missed a line... updated.

Confiming that patch in #17 works in 9.3.9. Thanks!

Works.

Reviewing RTBC issues.

I downloaded the MR and the patch in #17, ran diff and found that the two are identical. Therefor removing credit for the MR.

The Issue summary does not have a proposed resolution, adding tag for that.

The patch is for 9.4 and there is a fail test, in #12. Changing version to 10.0

+++ b/core/modules/views/src/Plugin/views/display/EntityReference.php
@@ -156,7 +156,15 @@ class EntityReference extends DisplayPluginBase {
+    // @see Drupal\views\Plugin\EntityReferenceSelection\ViewsSelection::initializeView()

I did go read ViewsSelection::initializeView() and it offers no insight to me into why the default options are set to the values below. Can we have something more helpful here?

Setting to NW. At least the two items should be easy.

+++ b/core/modules/views/tests/src/Functional/Plugin/DisplayEntityReferenceTest.php
@@ -275,6 +275,12 @@ public function testEntityReferenceDisplay() {
+    // Execute the View without setting the 'entity_reference_options'.
+    $view->destroy();
+    $view = Views::getView('test_display_entity_reference');
+    $view->setDisplay('entity_reference_1');
+    $this->executeView($view);

And it would be great to actually assert something after executing the view. Maybe that the options got set? If we don't assert anything, and don't have comment explaining what why we are executing without setting the 'entity_reference_options', this just looks like dead code outside the context of this issue. So maybe either update the comment to say something like 'Test that this doesn't trigger notices', or add an assertion (I'd vote for adding the assertion).

Thanks for the reviews @quietone and @Lendude.

I have updated the IS. Removing tag.

I'll try to address the comments in a new patch.

Here's an updated patch for 10.0.x that attempts to address the comments in #24 and #25. The new assertion added to the test comes after the line that triggers the fail in #12, so we don't need to run this as a test-only.

Please speak up if the new comments aren't good enough! I'm happy to make additional changes.

Try that again...

Interdiff is still relative to #15.

I have the same issue in drupal 9.4.5 version. I tried #17, but no use.

Any other solution?

Got that issue with drupal 9.4.7.
What solution is suggested?

The patch in #28 was RTBC but it got knocked back by a random test fail. I've triggered a re-test. That patch is worth trying. And maybe you can re-RTBC it if it works for you and the tests pass.

If that patch does not apply to 9.4.x you can tag this issue as needing a re-roll or you can do a re-roll yourself and post it.

I was seeing the same error when creating an entity reference view within the UI. I was able to apply #28 to 9.4.8, and it fixed the error.

Patch #28 fixed problem for me.

#28 fixes the described issue and does not change functionality. Works programmatically as well as with entity select field options arrays.

#28 fixed problem as well
9.5.0 / php 8.0.26

I checked with @rpayanm as to why he opened an MR when there is a working patch. He said he had noticed a previous test failure on the issue and was trying to see if he could fix it, starting with a rebase to get the patch current:
https://www.drupal.org/pift-ci-job/2455217

It looks like that issue was an unrelated failure in the Book module. We had a spate of the same failures in 10.0.x HEAD environments in August and September, around the time of the above job, so it is unrelated to this issue. @rpayanm and I talked a little about the different kinds of test failures and what it looks like when a reroll/rebase is needed (i.e. "Patch does not apply").

Thanks @rpayanm for talking through it! I'm going to close the redundant MR and remove the credit for it. Feel free to ask me anytime for help figuring out if a test failure is related to the issue or not. :) I have the world's second-largest database of historical Drupal core CI failures in my email. :D

So my first thought upon reviewing this is, is there a reason for the custom code to not initialize the view? The Views API expects a series of steps to be taken when preparing the view.

My other concern is hardcoding default values from a different API. However, the test should start failing (I think) if we ever break the 1:1 relationship between the defaults we're setting and the defaults being applied externally, so that should be alright too.

Other than that, this looks good to me. We have a failing test in #12 proving the bug, and review from @Lendude (who is one of the Views maintainers) to improve the test in #25. Overall, if we decide that the view doesn't need to be or shouldn't be initialized, it seems like a good solution. It is also well-documented.

Before committing this, I'd like to get final signoff from a Views subsystem maintainer on the change. (Yes, I am also a Views subsystem maintainer, but it's been awhile OK?) 😅

So, setting NR, tagging for subsystem maintainer review, and assigning to @Lendude. @Lendude, if you think the current patch in #28 is a good solution, please untag the subsystem maintainer review tag and set the issue back to RTBC, and we'll try to get this in. Thanks!

Thanks for taking a good look at this @xjm!

The Views API expects a series of steps to be taken when preparing the view.

So, in this context, it's not the Views API that expects this, but the EntityReferenceSelection that expects this. And now we are executing the View outside the context of EntityReferenceSelection. So, to me, we should either throw a warning that this particular display expects you to do more steps, or we should be lenient and just set what's needed.
I fully agree that hardcoding values from a different API feels iffy, but since this is done specifically in the 'entity_reference' display only, I can live with it here.

But if others feels that emitting an error is the better course here, I do see a point for that.

Thanks @Lendude for your feedback! So we'll go ahead with this change. I'm still a little hesitant about it, but with both docs and test coverage, it's probably the best it can be under the circumstance for fixing the DX here without entirely overhauling how display plugins work. And, given the number of people apparently encountering this bug (just look at the number of followers and "WFM" comments), it's probably best to fix it as we can in production branches.

I'm removing credit for @vrwired for D8 backport patches as they were after D8's end-of-life. Retaining credit for @robertom's original issue report, @danflanagan8's fix, and helpful code reviews.

Committed to 10.1.x. Since this is initializing default values to non-disruptive defaults when a notice would be thrown, it also seems safe in its current form to backport to 10.0.x and 9.5.x, I also cherry-picked it there. (The alternate solution of throwing a warning or exception would not have been backportable.) Thanks everyone!