Issue #2364381 by Wim Leers: Exception thrown in drupal_render() causes an...

Issue #2364381 by Wim Leers: Exception thrown in drupal_render() causes an exception during the rendering of exceptions

core/lib/Drupal/Core/Render/Renderer.php

@@ -84,6 +84,30 @@ public function renderPlain(&$elements) {
    * {@inheritdoc}
    */
   public function render(&$elements, $is_root_call = FALSE) {
+    // Since #pre_render, #post_render, #post_render_cache callbacks and theme
+    // functions/templates may be used for generating a render array's content,
+    // and we might be rendering the main content for the page, it is possible
+    // that any of them throw an exception that will cause a different page to
+    // be rendered (e.g. throwing
+    // \Symfony\Component\HttpKernel\Exception\NotFoundHttpException will cause
+    // the 404 page to be rendered). That page might also use Renderer::render()
+    // but if exceptions aren't caught here, the stack will be left in an
+    // inconsistent state.
+    // Hence, catch all exceptions and reset the stack and re-throw them.
+    try {
+      return $this->doRender($elements, $is_root_call);
+    }
+    catch (\Exception $e) {
+      // Reset stack and re-throw exception.
+      $this->resetStack();
+      throw $e;
+    }
+  }
+
+  /**
+   * See the docs for ::render().
+   */
+  protected function doRender(&$elements, $is_root_call = FALSE) {
     if (!isset($elements['#access']) && isset($elements['#access_callback'])) {
       if (is_string($elements['#access_callback']) && strpos($elements['#access_callback'], '::') === FALSE) {
         $elements['#access_callback'] = $this->controllerResolver->getControllerFromDefinition($elements['#access_callback']);
@@ -143,23 +167,7 @@ public function render(&$elements, $is_root_call = FALSE) {
         if (is_string($callable) && strpos($callable, '::') === FALSE) {
           $callable = $this->controllerResolver->getControllerFromDefinition($callable);
         }
-        // Since #pre_render callbacks may be used for generating a render
-        // array's content, and we might be rendering the main content for the
-        // page, it is possible that a #pre_render callback throws an exception
-        // that will cause a different page to be rendered (e.g. throwing
-        // \Symfony\Component\HttpKernel\Exception\NotFoundHttpException will
-        // cause the 404 page to be rendered). That page might also use
-        // drupal_render(), but if exceptions aren't caught here, the stack will
-        // be left in an inconsistent state.
-        // Hence, catch all exceptions and reset the stack and re-throw them.
-        try {
-          $elements = call_user_func($callable, $elements);
-        }
-        catch (\Exception $e) {
-          // Reset stack and re-throw exception.
-          $this->resetStack();
-          throw $e;
-        }
+        $elements = call_user_func($callable, $elements);
       }
     }
 
@@ -231,7 +239,7 @@ public function render(&$elements, $is_root_call = FALSE) {
     // same process as Renderer::render() but is inlined for speed.
     if ((!$theme_is_implemented || isset($elements['#render_children'])) && empty($elements['#children'])) {
       foreach ($children as $key) {
-        $elements['#children'] .= $this->render($elements[$key]);
+        $elements['#children'] .= $this->doRender($elements[$key]);
       }
       $elements['#children'] = SafeMarkup::set($elements['#children']);
     }

core/modules/system/src/Tests/System/ErrorHandlerTest.php

@@ -105,6 +105,13 @@ function testExceptionHandler() {
       '%line' => 64,
       '%file' => drupal_get_path('module', 'error_test') . '/error_test.module',
     );
+    $error_renderer_exception = array(
+      '%type' => 'Exception',
+      '!message' => 'This is an exception that occurs during rendering',
+      '%function' => 'Drupal\error_test\Controller\ErrorTestController->Drupal\error_test\Controller\{closure}()',
+      '%line' => 82,
+      '%file' => drupal_get_path('module', 'error_test') . '/error_test.module',
+    );
 
     $this->drupalGet('error-test/trigger-exception');
     $this->assertTrue(strpos($this->drupalGetHeader(':status'), '500 Service unavailable (with message)'), 'Received expected HTTP status line.');
@@ -119,6 +126,10 @@ function testExceptionHandler() {
     $error_details = format_string('in %function (line ', $error_pdo_exception);
     $this->assertRaw($error_details, format_string("Found '!message' in error page.", array('!message' => $error_details)));
 
+    $this->drupalGet('error-test/trigger-renderer-exception');
+    $this->assertTrue(strpos($this->drupalGetHeader(':status'), '500 Service unavailable (with message)'), 'Received expected HTTP status line.');
+    $this->assertErrorMessage($error_renderer_exception);
+
     // The exceptions are expected. Do not interpret them as a test failure.
     // Not using File API; a potential error must trigger a PHP warning.
     unlink(\Drupal::root() . '/' . $this->siteDirectory . '/error.log');

core/modules/system/tests/modules/error_test/error_test.routing.yml

@@ -26,3 +26,10 @@ error_test.trigger_pdo_exception:
     _controller: '\Drupal\error_test\Controller\ErrorTestController::triggerPDOException'
   requirements:
     _access: 'TRUE'
+
+error_test.trigger_renderer_exception:
+  path: '/error-test/trigger-renderer-exception'
+  defaults:
+    _controller: '\Drupal\error_test\Controller\ErrorTestController::triggerRendererException'
+  requirements:
+    _access: 'TRUE'

core/modules/system/tests/modules/error_test/src/Controller/ErrorTestController.php

@@ -72,4 +72,18 @@ public function triggerPDOException() {
     $this->database->query('SELECT * FROM bananas_are_awesome');
   }
 
+  /**
+   * Trigger an exception during rendering.
+   */
+  public function triggerRendererException() {
+    return [
+      '#type' => 'page',
+      '#post_render' => [
+        function () {
+          throw new \Exception('This is an exception that occurs during rendering');
+        }
+      ],
+    ];
+  }
+
 }