Missing View not helpfully reported in preRenderViewElement()

+++ b/core/modules/views/src/Element/View.php
@@ -41,6 +41,10 @@ public static function preRenderViewElement($element) {
       $view = Views::getView($element['#name']);
+      if (is_null($view)) {
+        drupal_set_message(t('View '.$element['#name'].', used on this page, is missing.'), 'error');
+        return;
+      }

I'm wondering whether it would be better to actually throw an exception ...

Yeah throwing an exception makes sense to me. So maybe something like this?

1. +++ b/core/modules/views/src/Element/MissingDataException.php
   @@ -0,0 +1,8 @@
   +class MissingDataException extends \Exception {}
   

   "Data" always sounds super generic / without any meaning

2. +++ b/core/modules/views/tests/src/Kernel/Element/ViewElementTest.php
   @@ -0,0 +1,28 @@
   +  /**
   +   * Tests that an exception is thrown when an invalid View is passed.
   +   */
   +  public function testInvalidView() {
   +    $renderer = $this->container->get('renderer');
   +    $render_element = [
   +      '#type' => 'view',
   +      '#name' => 'invalid_view_name',
   +      '#embed' => FALSE,
   +    ];
   +    $this->setExpectedException(MissingDataException::class);
   +    $renderer->renderRoot($render_element);
   

   Nice test

Ah yes 'naming things' :)

This any better?

+1

+++ b/core/modules/views/src/Element/View.php
@@ -41,6 +41,9 @@ public static function preRenderViewElement($element) {
+      if (!$view) {
+        throw new InvalidViewException('Invalid View name given.');
+      }

How about including the invalid name in the exception? Would seem helpful - no?

This one should be really really easy to fix :)

Normally, adding a new exception would be a minor-only change. However, given that there is currently a fatal error in HEAD, a new exception is not any more disruptive than that.

Also, given that this causes a WSOD and has steps to reproduce via the user interface, it can be considered major, and is potentially eligible for backport to 8.9.x still for that reason.

An updated patch for #7 and the reroll can also be tested against 9.2.x and 9.1.x if it applies there as well. Otherwise, we can create separate patches.

Thank you!

Going to work on a reroll/merge request and try to incorporate the suggestion from #7.

This is a re-roll of #5 that incorporates the suggestion in #7. This should apply cleanly to 8.9.x, 9.1.x, and 9.2.x.

Fixed (existing) coding standards issues in #16.

Created merge request from #17 to make tugboat live preview available. This should be ready for review/testing.

Updated patch with latest changes in MR branch:
- Fixed template literal in exception message.
- Replaced deprecated test method.
- Moved exception to Drupal\views\Exception namespace.
- Moved new test into existing ViewElementTest kernel test class.

Tests should pass and this should really be ready for review now.

Unrelated fail

+++ b/core/modules/views/src/Exception/InvalidViewException.php
@@ -0,0 +1,8 @@
+ * Exception thrown when an element misses a valid name value.

I think the description here has too much knowledge of when it is thrown. Which would prevent us from reusing this exception elsewhere.

Suggest that we rename the class to ViewRenderElementException and make the the docblock say

'Defines an exception for an invalid View render element' or similar

Then we can re-use this exception later for other uses cases such as when the display name is wrong or the arguments.

Thoughts?

I don't agree with this being a major bug, so dropping the priority, its a developer nuisance more than anything - I would even go close to marking it as minor.

Updated patch with @larowlan's suggestions from #27.

RTBC if bot agrees.

Revisiting earlier comments I see that xjm classed this as major, so re-instating that.

On that basis this qualifies for backport, but I think the ship has sailed for 8.9.

Committed 1ab0f48 and pushed to 9.3.x. Thanks!

Backported to 9.2 and 9.1

Thanks everyone involved