Drupal Version

11.3

Domain module version

2.0.0-beta8

Expected Behavior

No warnings

Actual Behavior

Warning generated

Warning: Undefined property: Drupal\Core\Field\EntityReferenceFieldItemList::$entity in domain_source_get() (line 117 of .../web/modules/contrib/domain/domain_source/domain_source.module

Steps to reproduce

  1. Install Drupal 11.3 and Domain 2.0.0.-beta8
  2. Create a web page "all default links" that only includes links to nodes whose field_domain_source is the default domain
  3. Create a web page "non-default links" that includes links to nodes whose field_domain_source is a non-default domain
  4. Rebuild caches
  5. View the "all default links" page - no warning is generated
  6. View the "non-default links" page - a warning is generated for the first link rendered for each non-default domain
  7. Refresh the "non-default links" page - no warning is generated

My guess at what is happening

When domain_source_get() gets the 'entity' property of the Node's field_domain_source, EntityReference::getTarget() is called to return the entity. Usually this works fine, but in the rendering of a response, Fibers are used. The warning appears to be generated when the current Fiber is suspended - when it resumes the 'entity' property evaluates as NULL. The Fiber appears to be suspended when EntityReference::getTarget() accesses the database rather than the cache.

Evidence for this includes ...

  • No warnings are generated for links to Nodes for the default domain - there are many calls made to DomainStorage::loadDefaultDomain() before the response is rendered so the default domain is cached
  • Warnings are only generated after the cache is cleared
  • When debugging domain_source_get() the warning is only generated when the current Fiber is suspended and restarted

Workaround

The warnings go away if we load all the domains before the page is rendered. For example ...

function THEME_preprocess_html(&$variables) {
  $domain_storage = \Drupal::entityTypeManager()->getStorage('domain');
  $domains = $domain_storage->loadMultiple();
}

Issue fork domain-3565121

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kenwest created an issue. See original summary.

mably’s picture

mably commented

We need a Fibers expert here 😉

Or should we just handle the warning properly?

Is it really specific to the Domain source module?

Is the warning still present when using the 3.x branch?

mably opened merge request !295

mably opened merge request !296

mably’s picture

mably commented
Status: Active » Needs review
mably’s picture

mably commented

I haven't been able to reproduce the problem on a Drupal 11.3.1 instance.

So will probably close the issue unless new information is provided.

  • mably committed 855017c9 on 3.x 
    fix: #3565121 Warning: Undefined property: Drupal\Core\Field\...

  • mably committed f59a3ba2 on 2.0.x 
    fix: #3565121 Warning: Undefined property: Drupal\Core\Field\...
mably’s picture

mably commented
Status: Needs review » Fixed

The warnings should be fixed.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

kenwest’s picture

kenwest commented

Thanks for your prompt response @mably

Unfortunately your patch for MR 296 doesn't fix my warnings, and my workaround still fixes the problem in my context after that patch is applied.

I have no answers to your questions ATM so I'll need to do some digging. The Fibers stuff is weird and may just be an artifact of my IDE.

mably’s picture

mably commented

Looks like we fixed the wrong warnings then.

What is the full stack trace of the remaining warning?

kenwest’s picture

kenwest commented

Hi @mably, if you can't reproduce the problem then perhaps this is a feature of my environment? Perhaps I should dig further before you expend more effort on this.

BTW I'll be offline for the next 2 weeks.

Here is the stack trace

Warning: Undefined property: Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem::$entity in domain_source_get() (line 117 of /var/www/drupal/web/modules/contrib/domain/domain_source.module) 
#0 /var/www/drupal/web/core/includes/bootstrap.inc(104): _drupal_error_handler_real()
#1 /var/www/drupal/web/modules/contrib/domain/domain_source/domain_source.module(117): _drupal_error_handler()
#2 /var/www/drupal/web/modules/contrib/domain/domain_source/src/HttpKernel/DomainSourcePathProcessor.php(169): domain_source_get()
#3 /var/www/drupal/web/core/lib/Drupal/Core/PathProcessor/PathProcessorManager.php(108): Drupal\domain_source\HttpKernel\DomainSourcePathProcessor->processOutbound()
#4 /var/www/drupal/web/core/modules/system/src/Hook/SystemHooks.php(242): Drupal\Core\PathProcessor\PathProcessorManager->processOutbound()
#5 /var/www/drupal/web/core/lib/Drupal/Core/Extension/ModuleHandler.php(460): Drupal\system\Hook\SystemHooks->jsSettingsAlter()
#6 /var/www/drupal/web/core/lib/Drupal/Core/Asset/AssetResolver.php(418): Drupal\Core\Extension\ModuleHandler->alter()
#7 /var/www/drupal/web/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php(107): Drupal\Core\Asset\AssetResolver->getJsAssets()
#8 /var/www/drupal/web/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php(74): Drupal\Core\Ajax\AjaxResponseAttachmentsProcessor->buildAttachmentsCommands()
#9 /var/www/drupal/web/core/lib/Drupal/Core/EventSubscriber/AjaxResponseSubscriber.php(56): Drupal\Core\Ajax\AjaxResponseAttachmentsProcessor->processAttachments()
#10 /var/www/drupal/vendor/symfony/event-dispatcher/EventDispatcher.php(206): Drupal\Core\EventSubscriber\AjaxResponseSubscriber->onResponse()
#11 /var/www/drupal/vendor/symfony/event-dispatcher/EventDispatcher.php(56): Symfony\Component\EventDispatcher\EventDispatcher->callListeners()
#12 /var/www/drupal/web/core/modules/big_pipe/src/Render/BigPipe.php(689): Symfony\Component\EventDispatcher\EventDispatcher->dispatch()
#13 /var/www/drupal/web/core/modules/big_pipe/src/Render/BigPipe.php(667): Drupal\big_pipe\Render\BigPipe->filterResponse()
#14 /var/www/drupal/web/core/modules/big_pipe/src/Render/BigPipe.php(564): Drupal\big_pipe\Render\BigPipe->filterEmbeddedResponse()
#15 /var/www/drupal/web/core/modules/big_pipe/src/Render/BigPipe.php(256): Drupal\big_pipe\Render\BigPipe->sendPlaceholders()
#16 /var/www/drupal/web/core/modules/big_pipe/src/Render/BigPipeResponse.php(116): Drupal\big_pipe\Render\BigPipe->sendContent()
#17 /var/www/drupal/vendor/symfony/http-foundation/Response.php(403): Drupal\big_pipe\Render\BigPipeResponse->sendContent()
#18 /var/www/drupal/web/index.php(20): Symfony\Component\HttpFoundation\Response->send()
#19 {main}.
kenwest’s picture

kenwest commented

As an aside, I noticed the latest release of Paragraphs has a Fiber-related bug fix.

The fix they made to line 42 of Paragraph's modules/paragraphs_library/src/LibraryItemAccessControlHandler.php looks very similar to the code in domain_source_get().

I've asked the developer over there if our symptoms are like theirs.

kenwest’s picture

kenwest commented

Using the hint from the Paragraphs issue, I've created a patch that prevents the warning being generated. I don't know if I'll have enough time to update the merge requests before I go away, so I'm just pasting the raw diffs here.

This is the patch to 2.0.0-beta8 ...

diff --git a/domain_source/domain_source.module b/domain_source/domain_source.module
index ed576c5..09b9c21 100644
--- a/domain_source/domain_source.module
+++ b/domain_source/domain_source.module
@@ -114,7 +114,7 @@ function domain_source_get(EntityInterface $entity) {
     return NULL;
   }
 
-  $domain = $entity->get($field)->entity;
+  $domain = $entity->get($field)->first()?->get('entity')->getValue();
   if (!$domain instanceof DomainInterface) {
     return NULL;
   }

This is the patch to Merge Requests 295 and 296 ...

diff --git a/domain_source/domain_source.module b/domain_source/domain_source.module
index f75621e..b02d1f7 100644
--- a/domain_source/domain_source.module
+++ b/domain_source/domain_source.module
@@ -120,7 +120,7 @@ function domain_source_get(EntityInterface $entity) {
     return NULL;
   }
 
-  $domain = $item->entity;
+  $domain = $item->get('entity')->getValue();
   if (!$domain instanceof DomainInterface) {
     return NULL;
   }
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Fixed » Needs work

See #3518668: Use Fibers for rendering views rows for a core issue with some context, it's a hard to fix php bug that we're running into. Suggested fixes look ok at a glance

mably’s picture

mably commented

Thanks for the share @berdir, will have a look at it.

I will implement the suggested fix.

kenwest opened merge request !297

mably opened merge request !298

mably changed the visibility of the branch 3565121-warning-undefined-property-2.0.x to hidden.

mably changed the visibility of the branch 3565121-warning-undefined-property to hidden.

mably opened merge request !299

mably’s picture

mably commented
Status: Needs work » Needs review

  • mably committed c6a25241 on 3.x 
    fix: #3565121 Warning: Undefined property: Drupal\Core\Field\...

  • mably committed 34e05c55 on 2.0.x 
    fix: #3565121 Warning: Undefined property: Drupal\Core\Field\...
mably’s picture

mably commented
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

mably’s picture

mably commented

Let's remember to check back on this issue when PHP 8.6 is required as the minimum version for Drupal 😉

idebr’s picture

idebr commented

A workaround was implemented in Drupal Core, see #3565937: Workaround PHP bug with fibers and __get()

mably’s picture

mably commented
Status: Fixed » Closed (fixed)