Problem/Motivation
WorkspacePublisher::publish() wraps the entity save loop in a try / catch (\Exception $e) block that rolls back the open transaction, logs the exception via Error::logException(), and rethrows. PHP \Error subclasses (\TypeError, \AssertionError, \Error from assert(), etc.) are not subclasses of \Exception, so the catch block is bypassed when one is thrown from inside the closure (for example from a hook implementation or downstream service call invoked during $entity->save()).
Steps to reproduce
- Track two entities in a workspace.
- Trigger a
\TypeError(or any\Errorsubclass) fromhook_entity_presave()on the second of the two entities. - Call
$workspace->publish(). - Observe that the first entity's workspace revision was published to Live and no error was logged on the
workspaceschannel.
Proposed resolution
Widen the catch in WorkspacePublisher::publish() from \Exception to \Throwable. Error::logException() already accepts \Throwable, so no other change is required in the catch body.
Remaining tasks
Review.
User interface changes
Nope.
Introduced terminology
N/A
API changes
Nope.
Data model changes
Nope.
Release notes snippet
N/A
Issue fork drupal-3590350
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:
- 3590350-workspace-publisher-catch-throwable
changes, plain diff MR !15771
Comments
Comment #3
amateescu commentedComment #4
plachLooks good and works well here, thanks!
Comment #5
godotislateOne question on the MR.
Comment #6
smustgrave commentedNot a huge fan of the hooks inside kernel tests but it is valid and probably worth doing. Moving to NW for that.
Comment #7
amateescu commentedComment #8
smustgrave commentedFeedback appears to be addressed
Comment #12
godotislateCommitted and pushed 24b0358 to main and f239996 to 11.x. Thanks!
Comment #14
amateescu commentedAs a bug fix, this should go into 11.4.x as well. I checked and it can be cherry-picked cleanly :)