Issue #2509898 by dawehner, znerol, larowlan, lauriii, Aki Tendo, Wim Leers:...

Issue #2509898 by dawehner, znerol, larowlan, lauriii, Aki Tendo, Wim Leers: Additional uncaught exception thrown while handling exception after service changes

core/core.services.yml

@@ -1031,7 +1031,7 @@ services:
     class: Drupal\Core\EventSubscriber\DefaultExceptionSubscriber
     tags:
       - { name: event_subscriber }
-    arguments: ['@config.factory', '@bare_html_page_renderer']
+    arguments: ['@config.factory']
   exception.logger:
     class: Drupal\Core\EventSubscriber\ExceptionLoggingSubscriber
     tags:

core/includes/errors.inc

@@ -9,6 +9,7 @@
 use Drupal\Component\Utility\Xss;
 use Drupal\Core\Logger\RfcLogLevel;
 use Drupal\Core\Utility\Error;
+use Symfony\Component\HttpFoundation\Response;
 
 /**
  * Maps PHP error constants to watchdog severity levels.
@@ -117,21 +118,6 @@ function error_displayable($error = NULL) {
  */
 function _drupal_log_error($error, $fatal = FALSE) {
   $is_installer = drupal_installation_attempted();
-  // Initialize a maintenance theme if the bootstrap was not complete.
-  // Do it early because drupal_set_message() triggers a
-  // \Drupal\Core\Theme\ThemeManager::initTheme().
-  if ($fatal && \Drupal::hasService('theme.manager')) {
-    // The installer initializes a maintenance theme at the earliest possible
-    // point in time already. Do not unset that.
-    if (!$is_installer) {
-      \Drupal::theme()->resetActiveTheme();
-    }
-    if (!defined('MAINTENANCE_MODE')) {
-      define('MAINTENANCE_MODE', 'error');
-    }
-    // No-op if the active theme is set already.
-    drupal_maintenance_theme();
-  }
 
   // Backtrace array is not a valid replacement value for t().
   $backtrace = $error['backtrace'];
@@ -152,22 +138,37 @@ function _drupal_log_error($error, $fatal = FALSE) {
         'line' => $error['%line'],
       ),
     );
+    // For non-fatal errors (e.g. PHP notices) _drupal_log_error can be called
+    // multiple times per request. In that case the response is typically
+    // generated outside of the error handler, e.g., in a controller. As a
+    // result it is not possible to use a Response object here but instead the
+    // headers need to be emitted directly.
     header('X-Drupal-Assertion-' . $number . ': ' . rawurlencode(serialize($assertion)));
     $number++;
   }
 
+  $response = new Response();
+
   // Only call the logger if there is a logger factory available. This can occur
   // if there is an error while rebuilding the container or during the
   // installer.
   if (\Drupal::hasService('logger.factory')) {
-    \Drupal::logger('php')->log($error['severity_level'], '%type: !message in %function (line %line of %file).', $error);
+    try {
+      \Drupal::logger('php')->log($error['severity_level'], '%type: !message in %function (line %line of %file).', $error);
+    }
+    catch (\Exception $e) {
+      // We can't log, for example because the database connection is not
+      // available. At least try to log to PHP error log.
+      error_log(sprintf('Failed to log error: %type: !message in %function (line %line of %file).', $error['%type'], $error['%function'], $error['%line'], $error['%file']));
+    }
   }
 
   if (PHP_SAPI === 'cli') {
     if ($fatal) {
       // When called from CLI, simply output a plain text message.
       // Should not translate the string to avoid errors producing more errors.
-      print html_entity_decode(strip_tags(format_string('%type: !message in %function (line %line of %file).', $error))). "\n";
+      $response->setContent(html_entity_decode(strip_tags(format_string('%type: !message in %function (line %line of %file).', $error))). "\n");
+      $response->send();
       exit;
     }
   }
@@ -177,7 +178,8 @@ function _drupal_log_error($error, $fatal = FALSE) {
       if (error_displayable($error)) {
         // When called from JavaScript, simply output the error message.
         // Should not translate the string to avoid errors producing more errors.
-        print format_string('%type: !message in %function (line %line of %file).', $error);
+        $response->setContent(format_string('%type: !message in %function (line %line of %file).', $error));
+        $response->send();
       }
       exit;
     }
@@ -185,6 +187,8 @@ function _drupal_log_error($error, $fatal = FALSE) {
   else {
     // Display the message if the current error reporting level allows this type
     // of message to be displayed, and unconditionally in update.php.
+    $message = '';
+    $class = NULL;
     if (error_displayable($error)) {
       $class = 'error';
 
@@ -219,37 +223,40 @@ function _drupal_log_error($error, $fatal = FALSE) {
         // Generate a backtrace containing only scalar argument values.
         $message .= '<pre class="backtrace">' . Error::formatBacktrace($backtrace) . '</pre>';
       }
-      if (\Drupal::hasService('session')) {
-        // Message display is dependent on sessions being available.
-        drupal_set_message(SafeMarkup::set($message), $class, TRUE);
-      }
-      else {
-        print $message;
-      }
     }
 
     if ($fatal) {
       // We fallback to a maintenance page at this point, because the page generation
       // itself can generate errors.
       // Should not translate the string to avoid errors producing more errors.
-      $message = 'The website encountered an unexpected error. Please try again later.';
+      $message = 'The website encountered an unexpected error. Please try again later.' . '<br />' . $message;
+
       if ($is_installer) {
         // install_display_output() prints the output and ends script execution.
         $output = array(
           '#title' => 'Error',
           '#markup' => $message,
         );
-        install_display_output($output, $GLOBALS['install_state']);
+        install_display_output($output, $GLOBALS['install_state'], $response->headers->all());
         exit;
       }
 
-      $bare_html_page_renderer = \Drupal::service('bare_html_page_renderer');
-      $response = $bare_html_page_renderer->renderBarePage(['#markup' => $message], 'Error', 'maintenance_page');
+      $response->setContent($message);
       $response->setStatusCode(500, '500 Service unavailable (with message)');
-      // An exception must halt script execution.
+
       $response->send();
+      // An exception must halt script execution.
       exit;
     }
+    else {
+      if (\Drupal::hasService('session')) {
+        // Message display is dependent on sessions being available.
+        drupal_set_message(SafeMarkup::set($message), $class, TRUE);
+      }
+      else {
+        print $message;
+      }
+    }
   }
 }
 
@@ -277,9 +284,16 @@ function _drupal_get_error_level() {
     return ERROR_REPORTING_DISPLAY_VERBOSE;
   }
   $error_level = NULL;
-  if (\Drupal::hasService('config.factory')) {
+  // Try to get the error level configuration from database. If this fails,
+  // for example if the database connection is not there, try to read it from
+  // settings.php.
+  try {
     $error_level = \Drupal::config('system.logging')->get('error_level');
   }
+  catch (\Exception $e) {
+    $error_level = isset($GLOBALS['config']['system.logging']['error_level']) ? $GLOBALS['config']['system.logging']['error_level'] : ERROR_REPORTING_HIDE;
+  }
+
   // If there is no container or if it has no config.factory service, we are
   // possibly in an edge-case error situation while trying to serve a regular
   // request on a public site, so use the non-verbose default value.

core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php

@@ -9,7 +9,6 @@
 
 use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Core\Config\ConfigFactoryInterface;
-use Drupal\Core\Render\BareHtmlPageRendererInterface;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
 use Drupal\Core\Utility\Error;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
@@ -43,24 +42,14 @@ class DefaultExceptionSubscriber implements EventSubscriberInterface {
    */
   protected $configFactory;
 
-  /**
-   * The bare HTML page renderer.
-   *
-   * @var \Drupal\Core\Render\BareHtmlPageRendererInterface
-   */
-  protected $bareHtmlPageRenderer;
-
   /**
    * Constructs a new DefaultExceptionSubscriber.
    *
    * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
    *   The configuration factory.
-   * @param \Drupal\Core\Render\BareHtmlPageRendererInterface $bare_html_page_renderer
-   *   The bare HTML page renderer.
    */
-  public function __construct(ConfigFactoryInterface $config_factory, BareHtmlPageRendererInterface $bare_html_page_renderer) {
+  public function __construct(ConfigFactoryInterface $config_factory) {
     $this->configFactory = $config_factory;
-    $this->bareHtmlPageRenderer = $bare_html_page_renderer;
   }
 
   /**
@@ -87,15 +76,13 @@ protected function onHtml(GetResponseForExceptionEvent $event) {
 
     // Display the message if the current error reporting level allows this type
     // of message to be displayed, and unconditionally in update.php.
+    $message = '';
     if (error_displayable($error)) {
-      $class = 'error';
-
       // If error type is 'User notice' then treat it as debug information
       // instead of an error message.
       // @see debug()
       if ($error['%type'] == 'User notice') {
         $error['%type'] = 'Debug';
-        $class = 'status';
       }
 
       // Attempt to reduce verbosity by removing DRUPAL_ROOT from the file path
@@ -125,11 +112,11 @@ protected function onHtml(GetResponseForExceptionEvent $event) {
         // sure the backtrace is escaped as it can contain user submitted data.
         $message .= '<pre class="backtrace">' . SafeMarkup::escape(Error::formatBacktrace($backtrace)) . '</pre>';
       }
-      drupal_set_message(SafeMarkup::set($message), $class, TRUE);
     }
 
     $content = $this->t('The website encountered an unexpected error. Please try again later.');
-    $response = $this->bareHtmlPageRenderer->renderBarePage(['#markup' => $content], $this->t('Error'), 'maintenance_page');
+    $content .= $message ? '</br></br>' . $message : '';
+    $response = new Response($content, 500);
 
     if ($exception instanceof HttpExceptionInterface) {
       $response->setStatusCode($exception->getStatusCode());

core/modules/system/src/Tests/Bootstrap/ErrorContainer.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Tests\Bootstrap\ErrorContainer.
+ */
+
+namespace Drupal\system\Tests\Bootstrap;
+
+use Drupal\Core\DependencyInjection\Container;
+
+/**
+ * Container base class which triggers an error.
+ */
+class ErrorContainer extends Container {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE) {
+    // Enforce a recoverable error.
+    $callable = function(ErrorContainer $container) {
+    };
+    $callable(1);
+  }
+
+}

core/modules/system/src/Tests/Bootstrap/ExceptionContainer.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Tests\Bootstrap\ExceptionContainer.
+ */
+
+namespace Drupal\system\Tests\Bootstrap;
+
+use Drupal\Core\DependencyInjection\Container;
+
+/**
+ * Base container which throws an exception.
+ */
+class ExceptionContainer extends Container {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE) {
+    throw new \Exception('Thrown exception during Container::get');
+  }
+
+}

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

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Tests\System\ErrorContainerRebuildKernel.
+ */
+
+namespace Drupal\system\Tests\System;
+
+use Drupal\Core\DrupalKernel;
+
+/**
+ * A kernel which produces a container which triggers an error.
+ */
+class ErrorContainerRebuildKernel extends DrupalKernel {
+
+  /**
+   * {@inheritdoc}
+   */
+  const CONTAINER_BASE_CLASS = '\Drupal\system\Tests\Bootstrap\ErrorContainer';
+
+}

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

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Tests\System\ExceptionContainerRebuildKernel.
+ */
+
+namespace Drupal\system\Tests\System;
+
+use Drupal\Core\DrupalKernel;
+
+/**
+ * A kernel which produces a container which triggers an exception.
+ */
+class ExceptionContainerRebuildKernel extends DrupalKernel {
+
+  /**
+   * {@inheritdoc}
+   */
+  const CONTAINER_BASE_CLASS = '\Drupal\system\Tests\Bootstrap\ExceptionContainer';
+
+}

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

+<?php
+/**
+ * @file
+ * Contains \Drupal\system\Tests\System\UncaughtExceptionTest
+ */
+
+namespace Drupal\system\Tests\System;
+
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests kernel panic when things are really messed up.
+ *
+ * @group system
+ */
+class UncaughtExceptionTest extends WebTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('error_service_test');
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $settings_filename = $this->siteDirectory . '/settings.php';
+    chmod($settings_filename, 0777);
+    $settings_php = file_get_contents($settings_filename);
+    $settings_php .= "\ninclude_once 'core/modules/system/src/Tests/Bootstrap/ErrorContainer.php';\n";
+    $settings_php .= "\ninclude_once 'core/modules/system/src/Tests/Bootstrap/ExceptionContainer.php';\n";
+    file_put_contents($settings_filename, $settings_php);
+
+    $settings = [];
+    $settings['config']['system.logging']['error_level'] = (object) [
+      'value' => ERROR_REPORTING_DISPLAY_VERBOSE,
+      'required' => TRUE,
+    ];
+    $this->writeSettings($settings);
+  }
+
+  /**
+   * Tests uncaught exception handling when system is in a bad state.
+   */
+  public function testUncaughtException() {
+    \Drupal::state()->set('error_service_test.break_bare_html_renderer', TRUE);
+
+    $this->config('system.logging')
+      ->set('error_level', ERROR_REPORTING_HIDE)
+      ->save();
+    $settings = [];
+    $settings['config']['system.logging']['error_level'] = (object) [
+      'value' => ERROR_REPORTING_HIDE,
+      'required' => TRUE,
+    ];
+    $this->writeSettings($settings);
+
+    $this->drupalGet('');
+    $this->assertResponse(500);
+    $this->assertText('The website encountered an unexpected error. Please try again later.');
+    $this->assertNoText('Oh oh, bananas in the instruments');
+
+    $this->config('system.logging')
+      ->set('error_level', ERROR_REPORTING_DISPLAY_ALL)
+      ->save();
+    $settings = [];
+    $settings['config']['system.logging']['error_level'] = (object) [
+      'value' => ERROR_REPORTING_DISPLAY_ALL,
+      'required' => TRUE,
+    ];
+    $this->writeSettings($settings);
+
+    $this->drupalGet('');
+    $this->assertResponse(500);
+    $this->assertText('The website encountered an unexpected error. Please try again later.');
+    $this->assertText('Oh oh, bananas in the instruments');
+  }
+
+  /**
+   * Tests a missing dependency on a service.
+   */
+  public function testMissingDependency() {
+    $this->drupalGet('broken-service-class');
+
+    $message = 'Argument 1 passed to Drupal\error_service_test\LonelyMonkeyClass::__construct() must be an instance of Drupal\Core\Database\Connection, non';
+
+    $this->assertRaw('The website encountered an unexpected error.');
+    $this->assertRaw($message);
+
+    $found_exception = FALSE;
+    foreach ($this->assertions as &$assertion) {
+      if (strpos($assertion['message'], $message) !== FALSE) {
+        $found_exception = TRUE;
+        $this->deleteAssert($assertion['message_id']);
+        unset($assertion);
+      }
+    }
+
+    $this->assertTrue($found_exception, 'Ensure that the exception of a missing constructor argument was triggered.');
+  }
+
+  /**
+   * Tests a container which has an error.
+   */
+  public function testErrorContainer() {
+    $kernel = ErrorContainerRebuildKernel::createFromRequest($this->prepareRequestForGenerator(), $this->classLoader, 'prod', TRUE);
+    $kernel->rebuildContainer();
+
+    $this->prepareRequestForGenerator();
+    // Ensure that we don't use the now broken generated container on the test
+    // process.
+    \Drupal::setContainer($this->container);
+
+    $this->drupalGet('');
+
+    $message = 'Argument 1 passed to Drupal\system\Tests\Bootstrap\ErrorContainer::Drupal\system\Tests\Bootstrap\{closur';
+    $this->assertRaw($message);
+
+    $found_error = FALSE;
+    foreach ($this->assertions as &$assertion) {
+      if (strpos($assertion['message'], $message) !== FALSE) {
+        $found_error = TRUE;
+        $this->deleteAssert($assertion['message_id']);
+        unset($assertion);
+      }
+    }
+
+    $this->assertTrue($found_error, 'Ensure that the error of the container was triggered.');
+  }
+
+  /**
+   * Tests a container which has an exception really early.
+   */
+  public function testExceptionContainer() {
+    $kernel = ExceptionContainerRebuildKernel::createFromRequest($this->prepareRequestForGenerator(), $this->classLoader, 'prod', TRUE);
+    $kernel->rebuildContainer();
+
+    $this->prepareRequestForGenerator();
+    // Ensure that we don't use the now broken generated container on the test
+    // process.
+    \Drupal::setContainer($this->container);
+
+    $this->drupalGet('');
+
+    $message = 'Thrown exception during Container::get';
+
+    $this->assertRaw('The website encountered an unexpected error');
+    $this->assertRaw($message);
+
+    $found_exception = FALSE;
+    foreach ($this->assertions as &$assertion) {
+      if (strpos($assertion['message'], $message) !== FALSE) {
+        $found_exception = TRUE;
+        $this->deleteAssert($assertion['message_id']);
+        unset($assertion);
+      }
+    }
+    $this->assertTrue($found_exception, 'Ensure that the exception of the container was triggered.');
+  }
+
+  /**
+   * Tests the case when the database connection is gone.
+   */
+  public function testLostDatabaseConnection() {
+    // We simulate a broken database connection by rewrite settings.php to no
+    // longer have the proper data.
+    $settings['databases']['default']['default']['password'] = (object) array(
+      'value' => $this->randomMachineName(),
+      'required' => TRUE,
+    );
+    $this->writeSettings($settings);
+
+    $this->drupalGet('');
+
+    $message = 'Access denied for user';
+    $this->assertRaw($message);
+
+    $found_exception = FALSE;
+    foreach ($this->assertions as &$assertion) {
+      if (strpos($assertion['message'], $message) !== FALSE) {
+        $found_exception = TRUE;
+        $this->deleteAssert($assertion['message_id']);
+        unset($assertion);
+      }
+    }
+    $this->assertTrue($found_exception, 'Ensure that the access denied DB connection exception is thrown.');
+
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function error($message = '', $group = 'Other', array $caller = NULL) {
+    if ($message === 'Oh oh, bananas in the instruments.') {
+      // We're expecting this error.
+      return;
+    }
+    return parent::error($message, $group, $caller);
+  }
+
+}

core/modules/system/tests/modules/error_service_test/error_service_test.info.yml

+name: 'Error service test'
+type: module
+description: 'Support module for causing bedlam in container rebuilds.'
+package: Testing
+version: VERSION
+core: 8.x

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

+error_service_test.broken_class:
+  path: broken-service-class
+  defaults:
+    _controller: \Drupal\error_service_test\Controller\LonelyMonkeyController::testBrokenClass
+  requirements:
+    _access: 'TRUE'

core/modules/system/tests/modules/error_service_test/error_service_test.services.yml

+services:
+  http_middleware.monkeys:
+    class: Drupal\error_service_test\MonkeysInTheControlRoom
+    tags:
+      - { name: http_middleware, priority: 400 }
+  # Set up a service with a missing class dependency.
+  broken_class_with_missing_dependency:
+    class: Drupal\error_service_test\LonelyMonkeyClass

core/modules/system/tests/modules/error_service_test/src/Controller/LonelyMonkeyController.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\error_service_test\Controller\LonelyMonkeyController.
+ */
+
+namespace Drupal\error_service_test\Controller;
+
+use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
+use Drupal\error_service_test\LonelyMonkeyClass;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+ * Provides a controller which calls out to a service with missing dependencies.
+ */
+class LonelyMonkeyController extends ControllerBase implements ContainerInjectionInterface {
+
+  public function __construct(LonelyMonkeyClass $class) {
+    $this->class = $class;
+  }
+
+  public function testBrokenClass() {
+    return [
+      '#markup' => $this->t('This should be broken.'),
+    ];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container) {
+    return new static($container->get('broken_class_with_missing_dependency'));
+  }
+
+}

core/modules/system/tests/modules/error_service_test/src/LonelyMonkeyClass.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\error_service_test\LonelyMonkeyClass.
+ */
+
+namespace Drupal\error_service_test;
+
+use Drupal\Core\Database\Connection;
+
+/**
+ * A class with a single dependency.
+ */
+class LonelyMonkeyClass {
+
+  public function __construct(Connection $connection) {
+    $this->connection = $connection;
+  }
+
+}

core/modules/system/tests/modules/error_service_test/src/MonkeysInTheControlRoom.php

+<?php
+/**
+ * @file
+ * Contains \Drupal\error_service_test\MonkeysInTheControlRoom.
+ */
+
+namespace Drupal\error_service_test;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+
+/**
+ * A http middleware designed to cause bedlam.
+ *
+ * @see error_service_test.services.yml
+ */
+class MonkeysInTheControlRoom implements HttpKernelInterface {
+
+  /**
+   * The app kernel.
+   *
+   * @var \Symfony\Component\HttpKernel\HttpKernelInterface
+   */
+  protected $app;
+
+  /**
+   * MonkeysInTheControlRoom constructor.
+   *
+   * @param \Symfony\Component\HttpKernel\HttpKernelInterface $app
+   *   The wrapper HTTP kernel.
+   */
+  public function __construct(HttpKernelInterface $app) {
+    $this->app = $app;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = TRUE) {
+    if (\Drupal::state()->get('error_service_test.break_bare_html_renderer')) {
+      // Let the bedlam begin.
+      // 1) Force a container rebuild.
+      /** @var \Drupal\Core\DrupalKernelInterface $kernel */
+      $kernel = \Drupal::service('kernel');
+      $kernel->rebuildContainer();
+      // 2) Fetch the in-situ container builder.
+      $container = $kernel->getContainer();
+      // Stop the theme manager from being found - and triggering error
+      // maintenance mode.
+      $container->removeDefinition('theme.manager');
+      // Mash. Mash. Mash.
+      \Drupal::setContainer($container);
+      throw new \Exception('Oh oh, bananas in the instruments.');
+    }
+
+    return $this->app->handle($request, $type, $catch);
+  }
+
+}

sites/example.settings.local.php

@@ -18,6 +18,9 @@
 
 /**
  * Show all error messages, with backtrace information.
+ *
+ * In case the error level could not be fetched from the database, as for
+ * example the database connection failed, we rely only on this value.
  */
 $config['system.logging']['error_level'] = 'verbose';