Problem/Motivation

While there is some exception in RemoteManager::doRequest method with empty $options parameter this method returns NULL. 


Client exception when requesting the URL: https://domain.com/jsonapi/taxonomy_term/tag/9f2d03f5-f9fc-406b-b84b-8bf377431cfb/parent with method GET: Client error: `GET https://domain.com/jsonapi/taxonomy_term/tag/9f2d03f5-f9fc-406b-b84b-8bf377431cfb/parent` resulted in a `404 Not Found` response

Error: Call to a member function getBody() on null in Drupal\entity_share_client\Plugin\EntityShareClient\Processor\EntityReference->importUrl() (line 241 of modules/contrib/entity_share/modules/entity_share_client/src/Plugin/EntityShareClient/Processor/EntityReference.php

Steps to reproduce

We are occasionally, getting this error while some entity are removed from server but jsonApi served from cache.

Proposed resolution

Add a check for Null value return from jsonApiRequest method in
modules/entity_share_client/src/Plugin/EntityShareClient/Processor/EntityReference.php line number 244. 

$referenced_entities_response = $this->remoteManager->jsonApiRequest($runtime_import_context->getRemote(), 'GET', $url);

Remaining tasks

Add patch

User interface changes

NA

API changes

NA

Data model changes

NA

CommentFileSizeAuthor
#2 3244881-1.patch1.14 KBbunty badgujar

Issue fork entity_share-3244881

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

Bunty Badgujar created an issue. See original summary.

bunty badgujar’s picture

bunty badgujar commented
StatusFileSize
new 3244881-1.patch1.14 KB

Add null check on response.

bunty badgujar’s picture

bunty badgujar commented
Status: Active » Needs work
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Hello,

Thanks for reporting this.

+++ b/modules/entity_share_client/src/Plugin/EntityShareClient/Processor/EntityReference.php
@@ -242,7 +242,11 @@ class EntityReference extends ImportProcessorPluginBase implements PluginFormInt
+    // Null check in case of exception.
+    if (!is_null($referenced_entities_response)) {
...
+    }

I think this should be done as 

 if (is_null()) {
return $referenced_entities_ids;
}

$referenced_entities_json = Json::decode((string) $referenced_entities_response->getBody());
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs work » Postponed (maintainer needs more info)

Strange, because when I go in a fake URL like:
- https://site1-web-entity-share.docker.localhost/jsonapi/taxonomy_term/es...
- https://site1-web-entity-share.docker.localhost/jsonapi/taxonomy_term/es...

Where there is no taxonomy term with this UUID, for both URLs I have a JSON API response (in 404).

And also even with cache, when udpating/deleting your content entities, JSON:API cache should be invalidated properly so this should not happen.

Can you please provide detailed steps to reproduce the bug starting from a fresh standard install?

arpitr’s picture

arpitr commented

I think condition handling would be better since importURL() method is expecting response to be a valid guzzle response object whereas in case of exception NULL is returned, this creates situation to cause Error.

There are scenarios created by issues in other modules https://www.drupal.org/project/memcache/issues/2996615 which could cause the json API to not update in certain situations.

Though this is less likely to happen but better type check and make it less error prone.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: bunty badgujar » grimreaper
Status: Postponed (maintainer needs more info) » Needs work

Hello @arpitr,

Thanks for pointing this.

I will update the patch using MR.

Grimreaper opened merge request !33

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs work » Needs review

I have updated the patch to change other spaces where this logic happened.

I will merge if tests are green.

  • Bunty Badgujar authored e19ea17 on 8.x-3.x 
    Issue #3244881 by Bunty Badgujar, Grimreaper, arpitr: Error: Call to a...
  • Grimreaper authored f63570e on 8.x-3.x
    Issue #3244881 by Bunty Badgujar, Grimreaper, arpitr: Error: Call to a...
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned
Status: Needs review » Fixed

Merged!

Thanks everyone!

Status: Fixed » Closed (fixed)

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