Commit eee35007 authored by catch's avatar catch

Issue #2678568 by Wim Leers: Ensure good UX & DX even when A) rendering of...

Issue #2678568 by Wim Leers: Ensure good UX & DX even when A) rendering of placeholder fails, B) some response event subscriber fails
parent c6f9c5a5
......@@ -205,6 +205,12 @@ protected function sendPreBody($pre_body, array $no_js_placeholders, AttachedAss
* @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.
......@@ -238,7 +244,19 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse
],
],
];
try {
$elements = $this->renderPlaceholder($placeholder, $placeholder_plus_cumulative_settings);
}
catch (\Exception $e) {
if (\Drupal::config('system.logging')->get('error_level') === ERROR_REPORTING_DISPLAY_VERBOSE) {
throw $e;
}
else {
trigger_error($e, E_USER_ERROR);
continue;
}
}
// Create a new HtmlResponse. Ensure the CSS and (non-bottom) JS is sent
// before the HTML they're associated with. In other words: ensure the
......@@ -263,7 +281,19 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse
// - 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())]);
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 {
trigger_error($e, E_USER_ERROR);
continue;
}
}
// Send this embedded HTML response.
print $html_response->getContent();
......@@ -290,6 +320,12 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse
* @param \Drupal\Core\Asset\AttachedAssetsInterface $cumulative_assets
* The cumulative assets sent so far; to be updated while rendering 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 sendPlaceholders(array $placeholders, array $placeholder_order, AttachedAssetsInterface $cumulative_assets) {
// Return early if there are no BigPipe placeholders to send.
......@@ -320,7 +356,18 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde
// Render the placeholder.
$placeholder_render_array = $placeholders[$placeholder_id];
try {
$elements = $this->renderPlaceholder($placeholder_id, $placeholder_render_array);
}
catch (\Exception $e) {
if (\Drupal::config('system.logging')->get('error_level') === ERROR_REPORTING_DISPLAY_VERBOSE) {
throw $e;
}
else {
trigger_error($e, E_USER_ERROR);
continue;
}
}
// Create a new AjaxResponse.
$ajax_response = new AjaxResponse();
......@@ -342,7 +389,18 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde
// 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']);
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 {
trigger_error($e, E_USER_ERROR);
continue;
}
}
// Send this embedded AJAX response.
$json = $ajax_response->getContent();
......
......@@ -260,12 +260,94 @@ public static function cases(ContainerInterface $container = NULL, AccountInterf
$current_time->embeddedHtmlResponse = '<time datetime=1991-03-14"></time>';
// 6. Edge case: #lazy_builder that throws an exception.
$exception = new BigPipePlaceholderTestCase(
[
'#lazy_builder' => ['\Drupal\big_pipe_test\BigPipeTestController::exception', ['llamas', 'suck']],
'#create_placeholder' => TRUE,
],
'<drupal-render-placeholder callback="\Drupal\big_pipe_test\BigPipeTestController::exception" arguments="0=llamas&amp;1=suck" token="68a75f1a"></drupal-render-placeholder>',
[
'#lazy_builder' => ['\Drupal\big_pipe_test\BigPipeTestController::exception', ['llamas', 'suck']],
]
);
$exception->bigPipePlaceholderId = 'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3Aexception&amp;args[0]=llamas&amp;args[1]=suck&amp;token=68a75f1a';
$exception->bigPipePlaceholderRenderArray = [
'#markup' => '<div data-big-pipe-placeholder-id="callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3Aexception&amp;args[0]=llamas&amp;args[1]=suck&amp;token=68a75f1a"></div>',
'#cache' => $cacheability_depends_on_session_and_nojs_cookie,
'#attached' => [
'library' => ['big_pipe/big_pipe'],
'drupalSettings' => [
'bigPipePlaceholderIds' => [
'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3Aexception&args[0]=llamas&args[1]=suck&token=68a75f1a' => TRUE,
],
],
'big_pipe_placeholders' => [
'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3Aexception&amp;args[0]=llamas&amp;args[1]=suck&amp;token=68a75f1a' => $exception->placeholderRenderArray,
],
],
];
$exception->embeddedAjaxResponseCommands = NULL;
$exception->bigPipeNoJsPlaceholder = '<div data-big-pipe-nojs-placeholder-id="callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3Aexception&amp;args[0]=llamas&amp;args[1]=suck&amp;token=68a75f1a"></div>';
$exception->bigPipeNoJsPlaceholderRenderArray = [
'#markup' => $exception->bigPipeNoJsPlaceholder,
'#cache' => $cacheability_depends_on_session_and_nojs_cookie,
'#attached' => [
'big_pipe_nojs_placeholders' => [
$exception->bigPipeNoJsPlaceholder => $exception->placeholderRenderArray,
],
],
];
$exception->embeddedHtmlResponse = NULL;
// 7. Edge case: response filter throwing an exception for this placeholder.
$embedded_response_exception = new BigPipePlaceholderTestCase(
[
'#lazy_builder' => ['\Drupal\big_pipe_test\BigPipeTestController::responseException', []],
'#create_placeholder' => TRUE,
],
'<drupal-render-placeholder callback="\Drupal\big_pipe_test\BigPipeTestController::responseException" arguments="" token="2a9bd022"></drupal-render-placeholder>',
[
'#lazy_builder' => ['\Drupal\big_pipe_test\BigPipeTestController::responseException', []],
]
);
$embedded_response_exception->bigPipePlaceholderId = 'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&amp;&amp;token=2a9bd022';
$embedded_response_exception->bigPipePlaceholderRenderArray = [
'#markup' => '<div data-big-pipe-placeholder-id="callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&amp;&amp;token=2a9bd022"></div>',
'#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&amp;&amp;token=2a9bd022' => $embedded_response_exception->placeholderRenderArray,
],
],
];
$embedded_response_exception->embeddedAjaxResponseCommands = NULL;
$embedded_response_exception->bigPipeNoJsPlaceholder = '<div data-big-pipe-nojs-placeholder-id="callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&amp;&amp;token=2a9bd022"></div>';
$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,
'exception__lazy_builder' => $exception,
'exception__embedded_response' => $embedded_response_exception,
];
}
......
......@@ -6,11 +6,11 @@ big_pipe_test:
requirements:
_access: 'TRUE'
big_pipe_test.no_big_pipe:
no_big_pipe:
path: '/no_big_pipe'
defaults:
_controller: '\Drupal\big_pipe_test\BigPipeTestController::test'
_title: 'BigPipe test'
_controller: '\Drupal\big_pipe_test\BigPipeTestController::nope'
_title: '_no_big_pipe route option test'
options:
_no_big_pipe: TRUE
requirements:
......
......@@ -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,9 +35,22 @@ public function test() {
// 5. Edge case: non-#lazy_builder placeholder.
$build['edge_case__html_non_lazy_builder'] = $cases['edge_case__html_non_lazy_builder']->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;
}
/**
* @return array
*/
public static function nope() {
return ['#markup' => '<p>Nope.</p>'];
}
/**
* #lazy_builder callback; builds <time> markup with current time.
*
......@@ -63,4 +77,24 @@ public static function helloOrYarhar() {
];
}
/**
* #lazy_builder callback; throws exception.
*
* @throws \Exception
*/
public static function exception() {
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];
}
}
......@@ -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;
......@@ -14,13 +15,45 @@
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 @@ public function onRespond(FilterResponseEvent $event) {
* {@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;
}
......
......@@ -75,6 +75,8 @@ public function placeholdersProvider() {
$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['exception__lazy_builder']->placeholder => $cases['exception__lazy_builder']->placeholderRenderArray,
$cases['exception__embedded_response']->placeholder => $cases['exception__embedded_response']->placeholderRenderArray,
];
return [
......@@ -90,6 +92,8 @@ public function placeholdersProvider() {
$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['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,
......@@ -97,6 +101,8 @@ public function placeholdersProvider() {
$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['exception__lazy_builder']->placeholder => $cases['exception__lazy_builder']->bigPipeNoJsPlaceholderRenderArray,
$cases['exception__embedded_response']->placeholder => $cases['exception__embedded_response']->bigPipeNoJsPlaceholderRenderArray,
]],
];
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment