Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
When a null value is passed into EntityStorageBase::load()
it gets passed into loadMultiple
as [0 => NULL]
, which triggers this warning that doesn't provide much help to developers:
Warning : array_flip(): Can only flip STRING and INTEGER values!
Proposed resolution
Add an assertion to the load
method that the ID is not null.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#10 | 3035980-10.drupal.Provide-a-better-error-when-a-NULL-is-passed-to-EntityStorageBaseload.patch | 1.21 KB | Gogowitsch |
Comments
Comment #2
jhedstromComment #3
jhedstromThis will probably need a test, but pushing up here to see if other tests fail.
Comment #4
Gogowitsch CreditAttribution: Gogowitsch as a volunteer and at QuoData GmbH Quality & Statistics for Merck KGaA commentedFirst of all: I am on board with the catching this typical mistake. However, I don't think assert will throw it into developers' faces, because by default, the assert system is not enabled in my environments. That might be the Ubuntu default, because I have not touched it. In XAMPP on Windows the assert was enabled. In summary, I believe we have to throw an exception.
Comment #5
jhedstromGiven how common this error is, in contrib and certainly in custom code, I'm not sure an exception would be the way to go as that would be a breaking change rather than a better error message. Currently a PHP warning is thrown, which would be disabled on most production sites.
Having assertions on is in the recommended developer settings.
Comment #6
jhedstromThis adds a test.
Comment #9
Gogowitsch CreditAttribution: Gogowitsch as a volunteer and at QuoData GmbH Quality & Statistics for Merck KGaA commented+1 from me. Thanks for pointing me to enable
assert
. I updated all our dev and continuous integration environments.Comment #10
Gogowitsch CreditAttribution: Gogowitsch as a volunteer and at QuoData GmbH Quality & Statistics for Merck KGaA commentedThis is the unchanged patch from #6, all credit belongs to jhedstrom.
I am uploading it to force the test runner to apply it to the 8.8.x branch, so it can be merged by the core maintainers with more confidence.
Comment #13
larowlanCommitted 4dfdaae and pushed to 8.8.x. Thanks!
c/p as 99c6cfcca8 and pushed to 8.7.x