Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
In Drupal\workspaces\Form\WorkspacePublishForm::submitForm we catch \Exception and then output a generic message and indicate that the error has been logged but we are not logging it.
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/works...
catch (\Exception $e) {
$this->messenger()->addMessage($this->t('Publication failed. All errors have been logged.'), 'error');
}
Proposed resolution
We should either allow the exception to be thrown or log it so that it can surface issues.
Issue fork drupal-3376177
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:
- 3376177-errors-on-workspacepublishformsubmitform
changes, plain diff MR !4447
Comments
Comment #3
nlisgo commentedThe fix I am proposing is that we should not try to catch
\Exception. This is the easiest way to ensure the unexpected error is triggered and logged.If it is thrown then an issue should be created.
Comment #4
smustgrave commentedCan you add the exact steps taken to reproduce the error?
Also will need a test case showing the issue.
Comment #5
nlisgo commentedThe steps I followed to trigger the exception were found in this issue: https://www.drupal.org/project/drupal/issues/3092549
The fix for that issue is independent of this related issue because the problem with this code is that unexpected exceptions are being completely overlooked. I could force an error to trigger the exception in a unit test but I'm not sure what value that would add?
There is no justification for the catch all of \Exception and it clearly isn't logging as is stated in the message that I'm removing in the MR.
I will attempt to add a test next week.
Comment #6
amateescu commentedThe justification for catching all exceptions there is that content editors usually can't do anything to fix them, so the idea was to fail gracefully. Not logging the exception is a problem though, so we should use
\Drupal\Core\Utility\Error::logException()there.Comment #7
nlisgo commentedRespectfully, I'm going to suggest that this issue does not require steps to recreate or a test. Not trying to be difficult but a caught exception that discards the exception and isn't logged as expected is steps to recreate enough?
The issue https://www.drupal.org/project/drupal/issues/3092549 drew my attention to this issue. I needed to refactor this code to surface the error.
I will persist with adding a test but the test I'm tempted to add will not be related to any known issue. I will just trigger a random exception and then confirm that it is logged.
Removing tag to add steps to reproduce.
Comment #8
nlisgo commentedI've added a test. It's quite involved to create the conditions to trigger this exception.
Comment #9
smustgrave commentedCC failure
Also instead of the new parameter (which may need a CR) could we use LoggerTrait?
Comment #10
nlisgo commentedI've addressed the CC failure and also now use LoggerTrait.
Comment #11
amateescu commentedI checked why we were saying that errors were logged in the form but not actually logging them, and the reason is that we're doing it in the workspace publisher: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/works...
However, there are a few things happening before and after that try/catch where an exception can be thrown, so the patch from this issue is correct, we also need to log errors in the form.
Comment #14
lauriiiCommitted 919cebf and pushed to 11.x. Cherry-picked to 10.1.x as a non-disruptive bug fix. Thanks!