src/Render/BigPipe.php | 31 ++++++++++++++- src/Tests/BigPipePlaceholderTestCases.php | 45 +++++++++++++++++++++- src/Tests/BigPipeTest.php | 12 +++++- .../big_pipe_test/src/BigPipeTestController.php | 21 ++++++++-- .../src/EventSubscriber/BigPipeTestSubscriber.php | 42 ++++++++++++++++++-- .../Render/Placeholder/BigPipeStrategyTest.php | 9 +++-- 6 files changed, 145 insertions(+), 15 deletions(-) diff --git a/src/Render/BigPipe.php b/src/Render/BigPipe.php index 02c9cf4..8e4b953 100644 --- a/src/Render/BigPipe.php +++ b/src/Render/BigPipe.php @@ -191,6 +191,12 @@ class BigPipe implements BigPipeInterface { * @param \Drupal\Core\Asset\AttachedAssetsInterface $cumulative_assets * The cumulative assets sent so far; to be updated while rendering no-JS * BigPipe placeholders. + * + * @throws \Exception + * If an exception is thrown during the rendering of a placeholder, it is + * caught to allow the other placeholders to still be replaced. But when + * error logging is configured to be verbose, the exception is rethrown to + * simplify debugging. */ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAssetsInterface $cumulative_assets) { // Split the HTML on every no-JS placeholder string. @@ -260,7 +266,18 @@ class BigPipe implements BigPipeInterface { // - the HTML to load the JS (at the top) can be rendered. $fake_request = $this->requestStack->getMasterRequest()->duplicate(); $fake_request->request->set('ajax_page_state', ['libraries' => implode(',', $cumulative_assets->getAlreadyLoadedLibraries())]); - $html_response = $this->filterEmbeddedResponse($fake_request, $html_response); + try { + $html_response = $this->filterEmbeddedResponse($fake_request, $html_response); + } + catch (\Exception $e) { + if (\Drupal::config('system.logging')->get('error_level') === ERROR_REPORTING_DISPLAY_VERBOSE) { + throw $e; + } + else { + continue; + } + } + // Send this embedded HTML response. print $html_response->getContent(); @@ -354,7 +371,17 @@ class BigPipe implements BigPipeInterface { // allows us to track the total set of asset libraries sent in the // initial HTML response plus all embedded AJAX responses sent so far. $fake_request->request->set('ajax_page_state', ['libraries' => implode(',', $cumulative_assets->getAlreadyLoadedLibraries())] + $cumulative_assets->getSettings()['ajaxPageState']); - $ajax_response = $this->filterEmbeddedResponse($fake_request, $ajax_response); + try { + $ajax_response = $this->filterEmbeddedResponse($fake_request, $ajax_response); + } + catch (\Exception $e) { + if (\Drupal::config('system.logging')->get('error_level') === ERROR_REPORTING_DISPLAY_VERBOSE) { + throw $e; + } + else { + continue; + } + } // Send this embedded AJAX response. $json = $ajax_response->getContent(); diff --git a/src/Tests/BigPipePlaceholderTestCases.php b/src/Tests/BigPipePlaceholderTestCases.php index 6459631..e7bed06 100644 --- a/src/Tests/BigPipePlaceholderTestCases.php +++ b/src/Tests/BigPipePlaceholderTestCases.php @@ -260,7 +260,7 @@ class BigPipePlaceholderTestCases { $current_time->embeddedHtmlResponse = ''; - // 6. Edge case: #lazy_builder that throws an exception. + // 6. Exception: #lazy_builder that throws an exception. $exception = new BigPipePlaceholderTestCase( [ '#lazy_builder' => ['\Drupal\big_pipe_test\BigPipeTestController::exception', ['llamas', 'suck']], @@ -300,13 +300,54 @@ class BigPipePlaceholderTestCases { ]; $exception->embeddedHtmlResponse = NULL; + // 7. Exception: placeholder that causes response filter to throw exception. + $embedded_response_exception = new BigPipePlaceholderTestCase( + [ + '#lazy_builder' => ['\Drupal\big_pipe_test\BigPipeTestController::responseException', []], + '#create_placeholder' => TRUE, + ], + '', + [ + '#lazy_builder' => ['\Drupal\big_pipe_test\BigPipeTestController::responseException', []], + ] + ); + $embedded_response_exception->bigPipePlaceholderId = 'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&&token=2a9bd022'; + $embedded_response_exception->bigPipePlaceholderRenderArray = [ + '#markup' => '
', + '#cache' => $cacheability_depends_on_session_and_nojs_cookie, + '#attached' => [ + 'library' => ['big_pipe/big_pipe'], + 'drupalSettings' => [ + 'bigPipePlaceholderIds' => [ + 'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&&token=2a9bd022' => TRUE, + ], + ], + 'big_pipe_placeholders' => [ + 'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&&token=2a9bd022' => $embedded_response_exception->placeholderRenderArray, + ], + ], + ]; + $embedded_response_exception->embeddedAjaxResponseCommands = NULL; + $embedded_response_exception->bigPipeNoJsPlaceholder = '
'; + $embedded_response_exception->bigPipeNoJsPlaceholderRenderArray = [ + '#markup' => $embedded_response_exception->bigPipeNoJsPlaceholder, + '#cache' => $cacheability_depends_on_session_and_nojs_cookie, + '#attached' => [ + 'big_pipe_nojs_placeholders' => [ + $embedded_response_exception->bigPipeNoJsPlaceholder => $embedded_response_exception->placeholderRenderArray, + ], + ], + ]; + $exception->embeddedHtmlResponse = NULL; + return [ 'html' => $status_messages, 'html_attribute_value' => $form_action, 'html_attribute_value_subset' => $csrf_token, 'edge_case__invalid_html' => $hello, 'edge_case__html_non_lazy_builder' => $current_time, - 'edge_case__html_exception' => $exception, + 'exception__lazy_builder' => $exception, + 'exception__embedded_response' => $embedded_response_exception, ]; } diff --git a/src/Tests/BigPipeTest.php b/src/Tests/BigPipeTest.php index dc84ca0..3a97522 100644 --- a/src/Tests/BigPipeTest.php +++ b/src/Tests/BigPipeTest.php @@ -162,9 +162,12 @@ class BigPipeTest extends WebTestBase { $this->assertBigPipePlaceholders([ $cases['html']->bigPipePlaceholderId => Json::encode($cases['html']->embeddedAjaxResponseCommands), $cases['edge_case__html_non_lazy_builder']->bigPipePlaceholderId => Json::encode($cases['edge_case__html_non_lazy_builder']->embeddedAjaxResponseCommands), - $cases['edge_case__html_exception']->bigPipePlaceholderId => NULL, + $cases['exception__lazy_builder']->bigPipePlaceholderId => NULL, + $cases['exception__embedded_response']->bigPipePlaceholderId => NULL, ]); + $this->assertRaw('', 'Closing body tag present.'); + $this->pass('Verifying BigPipe assets are present…', 'Debug'); $this->assertFalse(empty($this->getDrupalSettings()), 'drupalSettings present.'); $this->assertTrue(in_array('big_pipe/big_pipe', explode(',', $this->getDrupalSettings()['ajaxPageState']['libraries'])), 'BigPipe asset library is present.'); @@ -177,6 +180,7 @@ class BigPipeTest extends WebTestBase { $this->assertRaw('The website encountered an unexpected error. Please try again later'); $this->assertRaw('You are not allowed to say llamas are not cool!'); $this->assertNoRaw(static::STOP_SIGNAL, 'BigPipe stop signal absent: error occurred before then.'); + $this->assertNoRaw('', 'Closing body tag absent: error occurred before then.'); // The exception is expected. Do not interpret it as a test failure. unlink(\Drupal::root() . '/' . $this->siteDirectory . '/error.log'); } @@ -217,7 +221,8 @@ class BigPipeTest extends WebTestBase { $cases['html_attribute_value_subset']->bigPipeNoJsPlaceholder => $cases['html_attribute_value_subset']->embeddedHtmlResponse, $cases['html']->bigPipeNoJsPlaceholder => $cases['html']->embeddedHtmlResponse, $cases['edge_case__html_non_lazy_builder']->bigPipeNoJsPlaceholder => $cases['edge_case__html_non_lazy_builder']->embeddedHtmlResponse, - $cases['edge_case__html_exception']->bigPipePlaceholderId => NULL, + $cases['exception__lazy_builder']->bigPipePlaceholderId => NULL, + $cases['exception__embedded_response']->bigPipePlaceholderId => NULL, ]); $this->pass('Verifying there are no BigPipe placeholders & replacements…', 'Debug'); @@ -229,6 +234,8 @@ class BigPipeTest extends WebTestBase { $this->pass('Verifying BigPipe assets are absent…', 'Debug'); $this->assertFalse(empty($this->getDrupalSettings()), 'drupalSettings and BigPipe asset library absent.'); + $this->assertRaw('', 'Closing body tag present.'); + // Simulate development. $this->pass('Verifying BigPipe provides useful error output when an error occurs while rendering a placeholder if verbose error logging is enabled.', 'Debug'); $this->config('system.logging')->set('error_level', ERROR_REPORTING_DISPLAY_VERBOSE)->save(); @@ -236,6 +243,7 @@ class BigPipeTest extends WebTestBase { // The 'edge_case__html_exception' case throws an exception. $this->assertRaw('The website encountered an unexpected error. Please try again later'); $this->assertRaw('You are not allowed to say llamas are not cool!'); + $this->assertNoRaw('', 'Closing body tag absent: error occurred before then.'); // The exception is expected. Do not interpret it as a test failure. unlink(\Drupal::root() . '/' . $this->siteDirectory . '/error.log'); } diff --git a/tests/modules/big_pipe_test/src/BigPipeTestController.php b/tests/modules/big_pipe_test/src/BigPipeTestController.php index 6200929..2ac5a99 100644 --- a/tests/modules/big_pipe_test/src/BigPipeTestController.php +++ b/tests/modules/big_pipe_test/src/BigPipeTestController.php @@ -3,6 +3,7 @@ namespace Drupal\big_pipe_test; use Drupal\big_pipe\Render\BigPipeMarkup; +use Drupal\big_pipe_test\EventSubscriber\BigPipeTestSubscriber; class BigPipeTestController { @@ -34,8 +35,11 @@ class BigPipeTestController { // 5. Edge case: non-#lazy_builder placeholder. $build['edge_case__html_non_lazy_builder'] = $cases['edge_case__html_non_lazy_builder']->renderArray; - // 6. Edge case: #lazy_builder that throws an exception. - $build['edge_case__html_exception'] = $cases['edge_case__html_exception']->renderArray; + // 6. Exception: #lazy_builder that throws an exception. + $build['exception__lazy_builder'] = $cases['exception__lazy_builder']->renderArray; + + // 7. Exception: placeholder that causes response filter to throw exception. + $build['exception__embedded_response'] = $cases['exception__embedded_response']->renderArray; return $build; } @@ -43,7 +47,7 @@ class BigPipeTestController { /** * @return array */ - public function nope() { + public static function nope() { return ['#markup' => '

Nope.

']; } @@ -82,4 +86,15 @@ class BigPipeTestController { throw new \Exception('You are not allowed to say llamas are not cool!'); } + /** + * #lazy_builder callback; returns content that will trigger an exception. + * + * @see \Drupal\big_pipe_test\EventSubscriber\BigPipeTestSubscriber::onRespondTriggerException() + * + * @return array + */ + public static function responseException() { + return ['#plain_text' => BigPipeTestSubscriber::CONTENT_TRIGGER_EXCEPTION]; + } + } diff --git a/tests/modules/big_pipe_test/src/EventSubscriber/BigPipeTestSubscriber.php b/tests/modules/big_pipe_test/src/EventSubscriber/BigPipeTestSubscriber.php index 6af1028..3cf9bca 100644 --- a/tests/modules/big_pipe_test/src/EventSubscriber/BigPipeTestSubscriber.php +++ b/tests/modules/big_pipe_test/src/EventSubscriber/BigPipeTestSubscriber.php @@ -7,6 +7,7 @@ namespace Drupal\big_pipe_test\EventSubscriber; +use Drupal\Core\Render\AttachmentsInterface; use Drupal\Core\Render\HtmlResponse; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; use Symfony\Component\HttpKernel\KernelEvents; @@ -15,12 +16,44 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; class BigPipeTestSubscriber implements EventSubscriberInterface { /** + * @see \Drupal\big_pipe_test\BigPipeTestController::responseException() + * + * @var string + */ + const CONTENT_TRIGGER_EXCEPTION = 'NOPE!NOPE!NOPE!'; + + /** + * Triggers exception for embedded HTML/AJAX responses with certain content. + * + * @see \Drupal\big_pipe_test\BigPipeTestController::responseException() + * + * @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event + * The event to process. + * + * @throws \Exception + */ + public function onRespondTriggerException(FilterResponseEvent $event) { + $response = $event->getResponse(); + + if (!$response instanceof AttachmentsInterface) { + return; + } + + $attachments = $response->getAttachments(); + if (!isset($attachments['big_pipe_placeholders']) && !isset($attachments['big_pipe_nojs_placeholders'])) { + if (strpos($response->getContent(), static::CONTENT_TRIGGER_EXCEPTION) !== FALSE) { + throw new \Exception('Oh noes!'); + } + } + } + + /** * Exposes all BigPipe placeholders (JS and no-JS) via headers for testing. * * @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event * The event to process. */ - public function onRespond(FilterResponseEvent $event) { + public function onRespondSetBigPipeDebugPlaceholderHeaders(FilterResponseEvent $event) { $response = $event->getResponse(); if (!$response instanceof HtmlResponse) { return; @@ -44,8 +77,11 @@ class BigPipeTestSubscriber implements EventSubscriberInterface { * {@inheritdoc} */ public static function getSubscribedEvents() { - // Run *just* before \Drupal\big_pipe\EventSubscriber\HtmlResponseBigPipeSubscriber::onRespond(). - $events[KernelEvents::RESPONSE][] = ['onRespond', -99999]; + // Run just before \Drupal\big_pipe\EventSubscriber\HtmlResponseBigPipeSubscriber::onRespond(). + $events[KernelEvents::RESPONSE][] = ['onRespondSetBigPipeDebugPlaceholderHeaders', -9999]; + + // Run just after \Drupal\big_pipe\EventSubscriber\HtmlResponseBigPipeSubscriber::onRespond(). + $events[KernelEvents::RESPONSE][] = ['onRespondTriggerException', -10001]; return $events; } diff --git a/tests/src/Unit/Render/Placeholder/BigPipeStrategyTest.php b/tests/src/Unit/Render/Placeholder/BigPipeStrategyTest.php index 04e0257..da78dd2 100644 --- a/tests/src/Unit/Render/Placeholder/BigPipeStrategyTest.php +++ b/tests/src/Unit/Render/Placeholder/BigPipeStrategyTest.php @@ -75,7 +75,8 @@ class BigPipeStrategyTest extends UnitTestCase { $cases['html_attribute_value_subset']->placeholder => $cases['html_attribute_value_subset']->placeholderRenderArray, $cases['edge_case__invalid_html']->placeholder => $cases['edge_case__invalid_html']->placeholderRenderArray, $cases['edge_case__html_non_lazy_builder']->placeholder => $cases['edge_case__html_non_lazy_builder']->placeholderRenderArray, - $cases['edge_case__html_exception']->placeholder => $cases['edge_case__html_exception']->placeholderRenderArray, + $cases['exception__lazy_builder']->placeholder => $cases['exception__lazy_builder']->placeholderRenderArray, + $cases['exception__embedded_response']->placeholder => $cases['exception__embedded_response']->placeholderRenderArray, ]; return [ @@ -91,7 +92,8 @@ class BigPipeStrategyTest extends UnitTestCase { $cases['html_attribute_value_subset']->placeholder => $cases['html_attribute_value_subset']->bigPipeNoJsPlaceholderRenderArray, $cases['edge_case__invalid_html']->placeholder => $cases['edge_case__invalid_html']->bigPipeNoJsPlaceholderRenderArray, $cases['edge_case__html_non_lazy_builder']->placeholder => $cases['edge_case__html_non_lazy_builder']->bigPipePlaceholderRenderArray, - $cases['edge_case__html_exception']->placeholder => $cases['edge_case__html_exception']->bigPipePlaceholderRenderArray, + $cases['exception__lazy_builder']->placeholder => $cases['exception__lazy_builder']->bigPipePlaceholderRenderArray, + $cases['exception__embedded_response']->placeholder => $cases['exception__embedded_response']->bigPipePlaceholderRenderArray, ]], '_no_big_pipe absent, session, no-JS cookie present: no-JS BigPipe placeholder used for HTML placeholders' => [$placeholders, FALSE, TRUE, TRUE, [ $cases['html']->placeholder => $cases['html']->bigPipeNoJsPlaceholderRenderArray, @@ -99,7 +101,8 @@ class BigPipeStrategyTest extends UnitTestCase { $cases['html_attribute_value_subset']->placeholder => $cases['html_attribute_value_subset']->bigPipeNoJsPlaceholderRenderArray, $cases['edge_case__invalid_html']->placeholder => $cases['edge_case__invalid_html']->bigPipeNoJsPlaceholderRenderArray, $cases['edge_case__html_non_lazy_builder']->placeholder => $cases['edge_case__html_non_lazy_builder']->bigPipeNoJsPlaceholderRenderArray, - $cases['edge_case__html_exception']->placeholder => $cases['edge_case__html_exception']->bigPipeNoJsPlaceholderRenderArray, + $cases['exception__lazy_builder']->placeholder => $cases['exception__lazy_builder']->bigPipeNoJsPlaceholderRenderArray, + $cases['exception__embedded_response']->placeholder => $cases['exception__embedded_response']->bigPipeNoJsPlaceholderRenderArray, ]], ]; }