Hi,
I am using Drupal core 8.8.4 and am still getting below issue even with the latest code in #3048348: Denormalizing NULL for an optional @FieldType=address or @FieldType=geolocation field fails due to either no main property name or computed read-only main property committed. I am using JSON API and JSON API extras.
Below is the error.
Error: Call to a member function getClass() on null in Drupal\jsonapi\Normalizer\FieldItemNormalizer->denormalize() ( in /core/modules/jsonapi/src/Normalizer/FieldItemNormalizer.php)
This happens for only certain content type which has below fields.
1. List (Text)
2. Date range
3. Entity reference
4. Link
5. Text (plain)
6. Date
7. Text(formatted, long)
8. Boolean
9. Metatags.
The important point to note here is that am using Entity Share module to sync my content and the content syncs successfully. Am getting this error after the content sync which stops the batch process.
I also tried removing the Metatag field suspecting if this might be related to #3060702: Compatibility with Metatag Fields and #2945817: Support JSON API, REST, GraphQL and custom normalizations via new computed field but the issue still exists even after that.
It would be great if someone can shred some light on why this is happening and which module is responsible for this. When can we expect this to be fixed?
Entity Share module Issue reference (where they have pointed out that Meta tag module is causing the issue): #3044732: Compatibility with JSONAPI 2.x / Core 8.7.x
Parent issue: #3048348: Denormalizing NULL for an optional @FieldType=address or @FieldType=geolocation field fails due to either no main property name or computed read-only main property
Comment | File | Size | Author |
---|---|---|---|
#45 | 3127883-45.patch | 8.65 KB | Wim Leers |
| |||
#45 | interdiff.txt | 6.03 KB | Wim Leers |
#40 | 3127883-40.patch | 6.59 KB | Wim Leers |
| |||
#40 | interdiff.txt | 927 bytes | Wim Leers |
#36 | 3127883-36.patch | 6.58 KB | Wim Leers |
Comments
Comment #4
jhedstromI saw this behavior when the fields and properties had mis-matched casing (eg, for an address field,
Address_line1
would throw the error, whileaddress_line1
works fine.)It would be great to add an exception if a property isn't found, instead of the current behavior which tries to call
getClass()
on a NULL, which doesn't reveal the incorrect field property in the error logs.An
assert()
could be used instead of an exception, but the downside there is it wouldn't catch these errors on production where API clients may be doing their implementation work.Comment #5
bbralaI agree that improving the error message would be a good thing. A "call on null" error has wreaked havoc in another issue a while back.
Comment #6
jhedstromI think we can leave this as a bug rather than a feature request. Adding the DX tag.
Comment #7
challeypeng CreditAttribution: challeypeng commentedI have always encountered the same error when sync some entities to half:
An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /en/batch?id=1988&op=do_nojs&op=do
StatusText: parsererror
ResponseText:
and check the dblog, the error message is :
Error: Call to a member function getClass() on null in Drupal\jsonapi\Normalizer\FieldItemNormalizer->denormalize() (line 130 of /home/wwwroot/sancy-php7.4-d8.9.18-ok/core/modules/jsonapi/src/Normalizer/FieldItemNormalizer.php)
Hope someone can solve it.
Thanks very much!
Comment #9
Oscaner CreditAttribution: Oscaner at CI&T commentedSorry, this patch just a workaround.
Comment #10
camoz CreditAttribution: camoz commentedHaving the same issue with field type: "Text (plain, long)" when passing in JSON.stringified / serialized string.
zzz
Comment #11
ptmkenny CreditAttribution: ptmkenny commentedI encountered this error when I forgot to use JSON.stringify() before POSTing a json object to a "JSONB" field.
I'm attaching a basic patch that throws an exception for this error as suggested in #4.
I do not think the workaround in #9 is appropriate because it ignores this error, which is a genuine error and should halt processing.
Comment #12
hazra.bhaskar CreditAttribution: hazra.bhaskar commentedError: Call to a member function getClass() on null in Drupal\jsonapi\Normalizer\FieldItemNormalizer->denormalize() (line 130 of /var/www/html/core/modules/jsonapi/src/Normalizer/FieldItemNormalizer.php)
#0 /var/www/html/vendor/symfony/serializer/Serializer.php(182): Drupal\jsonapi\Normalizer\FieldItemNormalizer->denormalize()
#1 /var/www/html/core/modules/jsonapi/src/Serializer/Serializer.php(75): Symfony\Component\Serializer\Serializer->denormalize()
#2 /var/www/html/core/modules/jsonapi/src/Normalizer/FieldNormalizer.php(65): Drupal\jsonapi\Serializer\Serializer->denormalize()
#3 /var/www/html/vendor/symfony/serializer/Serializer.php(182): Drupal\jsonapi\Normalizer\FieldNormalizer->denormalize()
#4 /var/www/html/core/modules/jsonapi/src/Serializer/Serializer.php(75): Symfony\Component\Serializer\Serializer->denormalize()
#5 /var/www/html/core/modules/jsonapi/src/Normalizer/ContentEntityDenormalizer.php(85): Drupal\jsonapi\Serializer\Serializer->denormalize()
#6 /var/www/html/core/modules/jsonapi/src/Normalizer/EntityDenormalizerBase.php(99): Drupal\jsonapi\Normalizer\ContentEntityDenormalizer->prepareInput()
#7 /var/www/html/vendor/symfony/serializer/Serializer.php(182): Drupal\jsonapi\Normalizer\EntityDenormalizerBase->denormalize()
#8 /var/www/html/core/modules/jsonapi/src/Serializer/Serializer.php(75): Symfony\Component\Serializer\Serializer->denormalize()
#9 /var/www/html/core/modules/jsonapi/src/Normalizer/JsonApiDocumentTopLevelNormalizer.php(167): Drupal\jsonapi\Serializer\Serializer->denormalize()
#10 /var/www/html/modules/contrib/entity_share/modules/entity_share_client/src/Service/JsonapiHelper.php(287): Drupal\jsonapi\Normalizer\JsonApiDocumentTopLevelNormalizer->denormalize()
#11 /var/www/html/modules/contrib/entity_share/modules/entity_share_client/src/Service/JsonapiHelper.php(467): Drupal\entity_share_client\Service\JsonapiHelper->extractEntity()
#12 /var/www/html/modules/contrib/entity_share/modules/entity_share_async/src/Plugin/QueueWorker/EntityShareAsyncWorker.php(144): Drupal\entity_share_client\Service\JsonapiHelper->importEntityListData()
#13 /var/www/html/core/lib/Drupal/Core/Cron.php(180): Drupal\entity_share_async\Plugin\QueueWorker\EntityShareAsyncWorker->processItem()
#14 /var/www/html/modules/contrib/ultimate_cron/src/UltimateCron.php(69): Drupal\Core\Cron->processQueues()
#15 /var/www/html/modules/contrib/ultimate_cron/src/ProxyClass/UltimateCron.php(70): Drupal\ultimate_cron\UltimateCron->run()
#16 /var/www/html/core/modules/automated_cron/src/EventSubscriber/AutomatedCron.php(65): Drupal\ultimate_cron\ProxyClass\UltimateCron->run()
#17 [internal function]: Drupal\automated_cron\EventSubscriber\AutomatedCron->onTerminate()
#18 /var/www/html/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(111): call_user_func()
#19 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(88): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch()
#20 /var/www/html/vendor/stack/builder/src/Stack/StackedHttpKernel.php(32): Symfony\Component\HttpKernel\HttpKernel->terminate()
#21 /var/www/html/core/lib/Drupal/Core/DrupalKernel.php(686): Stack\StackedHttpKernel->terminate()
#22 /var/www/html/index.php(27): Drupal\Core\DrupalKernel->terminate()
#23 {main}
Module:entity_share
Drupal:8.9
how to fix this issue?
please give me the idea
.
Comment #14
hazra.bhaskar CreditAttribution: hazra.bhaskar commentedany update on this issue?
Comment #15
Oscaner CreditAttribution: Oscaner at CI&T commentedComment #17
BenStallings CreditAttribution: BenStallings at Interdependent Web LLC for Share Good USA commented#15 works for me!
Comment #18
alexpottThanks for filing this bug report and for fixing the issue. Bug fixing is very valuable. In order to commit a bug fix, we need an automated test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal, see the following links:
Comment #19
ptmkenny CreditAttribution: ptmkenny commentedIn addition to adding a test as per #18, I think this should throw an exception as per #4. When a property is requested that doesn't actually have a value set, that is an error, and we should throw an exception to stop processing and make debugging easier.
As I noted in #11, at least in the case of forgetting to use JSON.stringify() before POSTing a json object to a JSON field, it seems clear that an exception should be thrown because that is an error on the part of the developer.
Comment #20
Wim LeersThank you, @jhedstrom in #4, for making this issue actionable through your crystal-clear comment! 👍
\Drupal\jsonapi\Controller\EntityResource::deserialize()
does this:so AFAICT the proposed fix in #15 is wrong here. Instead, we should be able to just throw
UnexpectedValueException
? Similar but different to what #11 was doing.Comment #21
Wim LeersTest.
Comment #22
Wim LeersFix.
Comment #24
Wim LeersThat worked. Time to run the full test suite!
Comment #25
ptmkenny CreditAttribution: ptmkenny commentedThe patch in #24 is what I was trying to do in #11, but better. 😊 This throws a clear error that makes it immediately clear to the developer/API user what the issue is, which is a big win for DX and will save a lot of time debugging.
Comment #26
pradhumanjain2311 CreditAttribution: pradhumanjain2311 at OpenSense Labs for DrupalFit commentedPatch #24 applied successfully in version 10.1.x.
Comment #27
Wim Leers@pradhumanjainOSL The test results say the same thing… and more. I'm not sure what your point is? 🤔
Comment #28
bbralaThis looke like a great improvement for DX really really glad to see this change.
One of the things that is sometimes a little annoying as a developer is when validation only really validated parts of what I ask of the system. This means that when i solve validation i try again, get the next error and try again.
What if we do that here also.
So instead of the current:
We change this to first loop, then error.
Although this is a relatively small improvement of the possible error messages the developer gets, the extra code it requires is minimal.
Possible drawbacks might be;
$invalid_properties
.I'm not married to this improvement, but I feel like it would be a net gain if we allow multiple errors right there.
Comment #29
Wim LeersI like it!
The second drawback is not really a drawback IMHO. This extra loop is negligible compared to the time interacting with the database.
The first drawback is mitigated easily: extract this validation step completely, and run it first. IOW: don't even try to denormalize until we've made sure all properties are valid.
P.S.: this is also how we validate all the things in CKEditor 5. We even have extensive test coverage for it. See
\Drupal\Tests\ckeditor5\Unit\HTMLRestrictionsTest::providerConstruct()
for\Drupal\ckeditor5\HTMLRestrictions::validateAllowedRestrictionsPhase*
,\Drupal\Tests\ckeditor5\Kernel\CKEditor5PluginManagerTest::providerTestInvalidPluginDefinitions()
for\Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::validate(CKEditor5|Drupal)Aspects()
, and more 🤓 I actually learned that lesson while working on REST and JSON:API: concrete, specific error messages are such an enormous productivity boost!Comment #30
bbralaHmm, good one to separate the logic. That sounds like a great plan.
Looks pretty structured in ckeditor. Lovely to see that iteration on validation.
Comment #31
Wim LeersImplemented that. And went further still 🤓 Automatically finding the property name that the user probably meant! (Copy/pasted
\Drupal\Component\DependencyInjection\Container::getAlternatives()
for that.)But first … a failing test that will prove that @bbrala's dream in #28 will become a reality.
Comment #33
Wim LeersComment #34
bbralaI think I'm in love with this change. I just removed the drupalci.yml additions. BUt RTBC for me.
Comment #35
catchlevenstein() has a maximum string length of 255 characters. I can't remember what the maximum length of a configurable field is, but should we throw an exception when the string is two long (either field length, or 255) given this is user data?
Comment #36
Wim Leers🤯 Wow :D
Let's not throw an exception, let's just not find alternatives 🤓
Comment #37
bbralaChange is minimal. But while looking. Shouldn't the static methods argument be typehinted? First should be a string right.
Comment #39
bbralaDrupalci is having a fit again. Unrelated test failure.
Was gonna rtbc anyways, but the method without a rypehint could actually result in weirdness and possible bugs at some point.
Comment #40
Wim LeersOh, right, we can do that now! 👍 Done.
Comment #41
bbralaChange is good. Tests pass. Rtbc.
Comment #42
alexpottIt's quite possible that $invalid_property_names will be empty. In this case the exception does not make much sense. I think having both the guess and full list of writable properties is unnecessary and odd. I'm not sure I think that getAlternatives is actually worth it. Also why are we listing all writable properties but the alternatives could include non-writable properties?
Comment #43
Wim LeersNot sure I follow, because if
$invalid_property_names
is empty, no exception is thrown? 🤔I disagree. If you make a minor typo,
getAlternatives()
will find the right one. If it's wildly wrong, the full list of writable properties addresses the developer's confusion.Oh wait — I think you didn't mean
$invalid_property_names
would be empty, but$invalid_property_names[$SOMETHING]
— i.e. the list of alternatives could very well be empty. That's absolutely true, and is handled in a sort of brute-force way — it could be done more helpfully if we increase the LoC.I believe this change addresses your concerns. 😊
Comment #44
Wim LeersOMG. d.o patch removal still results in my comment being posted prematurely 🤯
Comment #45
Wim LeersThe patch that goes with #43.
Comment #47
Wim LeersRandom fail:
1) Drupal\Tests\ckeditor5\FunctionalJavascript\AdminUiTest::testSettingsOnlyFireAjaxWithCkeditor5
— this was fixed recently in #3315319: Random fails in Drupal\Tests\ckeditor5\FunctionalJavascript\AdminUiTest and Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5Test. Re-testing.Comment #48
bbralaAl seems good, but waiting for full testrun.
Comment #49
bbralaComment #51
catchCommitted/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x, thanks!