Errors on WorkspacePublishForm::submitForm are not being logged

The 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.

Can you add the exact steps taken to reproduce the error?

Also will need a test case showing the issue.

The steps I followed to trigger the exception were found in this issue: https://www.drupal.org/project/drupal/issues/3092549 🐛 Custom blocks with media are not being displayed Active

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.

The 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.

The 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.

Respectfully, 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 🐛 Custom blocks with media are not being displayed Active 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.

I've added a test. It's quite involved to create the conditions to trigger this exception.

CC failure

Also instead of the new parameter (which may need a CR) could we use LoggerTrait?

I've addressed the CC failure and also now use LoggerTrait.

I 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.

Committed 919cebf and pushed to 11.x. Cherry-picked to 10.1.x as a non-disruptive bug fix. Thanks!

lauriii → closed merge request !4447

Automatically closed - issue fixed for 2 weeks with no activity.