Exceptions in batch no longer are shown on the page: Javascript error

copying my response to @bnjmnm from the other issue

@bnjmnm thanks for the explanation!

Adding a '#type' => 'status_messages' element to the test page should get the FJS test working,

We actually aren't using a test page this if failing on the regular batch system. We just use a test form to start the batch process. But then it gets directed to the core batch controller. Thats is why this would cause a problem any core or contrib usage of the batch form system when JS enabled(also without JS but the other reasons described on this issue), any exception that happened in batch callbacks would no longer show up unless they specifically were caught and handled in the callbacks.

So this just needed '#type' => 'status_messages' in \Drupal\system\Controller\BatchController::batchPage see https://git.drupalcode.org/project/drupal/-/merge_requests/4957/diffs#9d... in the MR

This gets the JS test passing for locally. I pushed up this change to see if we have any core tests that will fail if the status message element. I wouldn't think so but you never know.

I think we should make fixing the JS version its own issue if it is really this simple

Looking at \Drupal\system\Controller\BatchController::batchPage

$page = [
        '#type' => 'page',
        '#title' => $title,
        '#show_messages' => FALSE,
        'content' => $output,
      ];

My guess is that #show_messages here is useless and a holdover from Drupal 7 or 8. https://api.drupal.org/api/drupal/includes%21common.inc/function/drupal_...

And the only reason messages aren't shown on the batch page and then display after the batch job is finished is that there is no

 '#type' => 'status_messages' 

element on the page.

Update loading a patch and my guess is that \Drupal\Tests\locale\Functional\LocaleConfigTranslationImportTest::testConfigTranslationModuleInstall won't fail like it does when I add the status_messages element.

Marking this as major because it would probably stop ✨ Add Alpha level Experimental Automatic Updates module Needs work . Anyone who used the module would not see any errors when using the form. We did put a temporary fix in the contrib module to address this 🐛 Ensure that errors are shown on form updates in core 10.1.x Fixed which just does

// Ensure error messages will be displayed on the batch page.
  // @todo Remove this work around when https://drupal.org/i/3406612 is fixed.
  if (\Drupal::routeMatch()->getRouteName() === 'system.batch_page.html') {
    // Directly render a status message placeholder without any messages.
    // Messages are not intended to be show on the batch page, but in the event
    // an error in a AJAX callback the messages will be displayed.
    $page_top['messages'] = [
      '#theme' => 'status_messages',
      '#message_list' => [],
      '#status_headings' => [
        'status' => t('Status message'),
        'error' => t('Error message'),
        'warning' => t('Warning message'),
      ],
    ];
  }

in hook_page_top(). If this fix is good enough to alpha commit of Automatic Updates in core then we could bump this issue down.

I pushed another branch with a different approach 3406612-use-messages-theme
Instead of using the StatusMessages render element and adding changes that would allow it to not actually render server side messages this branch just adds and element with '#theme' => 'status_messages',, therefore just putting the mark up on the page.

This may be simpler than changing StatusMessages for the very unique case of the batch page were you don't want the messages to form the server to show

tedbow → closed merge request !5711

Created follow-up 🐛 message.js assumes status messages element has child element, incompatible default template Needs review this keeps us from using stark theme in the test. That issue could also use a review. Fix I think is very simple.

I think this is a reasonable, well-scoped fix for this problem until the more general 🐛 The core/drupal.message library requires a status_messages render element Active lands. It's good to finally have test coverage around this, too.

lauriii → closed merge request !5739

Committed d562eda and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!

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