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.
The exception class is not imported, so it won't work. Patch attached.
Comments
Comment #1
fagoComment #2
fagoComment #3
BerdirRelated #1553244: Incorrect method calls in exception handling code in EntityDatabaseStorageController
Change looks good, having a test that triggers an exception would be useful I think, as described in the other issue.
Comment #4
fagoAgreed. Let's do that.
Comment #5
chia CreditAttribution: chia commentedLittle tests to see if exceptions are working okay, may be we can improve the messages,
Comment #6
fagoComment #7
fagoYou got some whitespaces.. ;)
Always start with an uppercase later and end with a dot. Also maybe move the comment before entity_create().
catch should be at a new line.
Comment #8
chia CreditAttribution: chia commentedAnother patch with better naming and fixed the above mentioned issues ;)
Comment #9
Berdir#8: entity-storage-exception-testing-1634442.patch queued for re-testing.
Comment #10
Berdir#8: entity-storage-exception-testing-1634442.patch queued for re-testing.
Comment #12
BerdirRe-rolled, cleaned up the tests a bit and a tests-only patch to make sure that this works. I hope this doesn't blow up the bot due to another missing use there, see #1759026: Testing broken: Exception is never caught. It will certainly conflict with it since it adds a fix for this as well.
Comment #13
webchickI committed the referenced issue, so this will need a re-roll.
Comment #14
BerdirRe-roll.
Comment #15
amateescu CreditAttribution: amateescu commentedUpdated the test to be a EntityUnitTest and cleaned up the patch a litte. I think this is good to go now.
Comment #16
catchThe test uses state but everything runs in one request.
Couldn't we set $entity->throw_exception to TRUE directly - would save the hook_node_presave() and the db hits/state dependency in the test.
Comment #17
amateescu CreditAttribution: amateescu commentedDiscussed in IRC and decided to drop the $entity->throw_exception stuff because the storage controller doesn't have a way to know about it, and just use $GLOBALS instead of the state system, which makes perfect sense.
Comment #18
BerdirHm :)
We're in a DUBT test, so state() (or actually, it should use Drupal::state() within the hooks as well) uses the memory backend. Using it seems cleaner to me than $GLOBALS and is essentially the same. I've written other tests before that rely on state for DUBT tests, see for example \Drupal\system\Tests\DrupalKernel\ServiceDestructionTest.
Comment #19
alexpottI'm not fussed if we use globals or state... I like I prefer globals because it is one less dependency.
Committed ce7ff5c and pushed to 8.x. Thanks!
Comment #21
YesCT CreditAttribution: YesCT commentedfollow-up #2015849: Some standards fixes in entity_test.module and EntityApiTest.php, motivated by "DatabaseStorageController can't catch exceptions"