Commit ce13a058 authored by alexpott's avatar alexpott

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

Revert "Issue #2678568 by Wim Leers: Ensure good UX & DX even when A) rendering of placeholder fails, B) some response event subscriber fails"

This reverts commit eee35007.
parent 5f65d693
...@@ -205,12 +205,6 @@ protected function sendPreBody($pre_body, array $no_js_placeholders, AttachedAss ...@@ -205,12 +205,6 @@ protected function sendPreBody($pre_body, array $no_js_placeholders, AttachedAss
* @param \Drupal\Core\Asset\AttachedAssetsInterface $cumulative_assets * @param \Drupal\Core\Asset\AttachedAssetsInterface $cumulative_assets
* The cumulative assets sent so far; to be updated while rendering no-JS * The cumulative assets sent so far; to be updated while rendering no-JS
* BigPipe placeholders. * 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) { protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAssetsInterface $cumulative_assets) {
// Split the HTML on every no-JS placeholder string. // Split the HTML on every no-JS placeholder string.
...@@ -244,19 +238,7 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse ...@@ -244,19 +238,7 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse
], ],
], ],
]; ];
try { $elements = $this->renderPlaceholder($placeholder, $placeholder_plus_cumulative_settings);
$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 // 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 // before the HTML they're associated with. In other words: ensure the
...@@ -281,19 +263,7 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse ...@@ -281,19 +263,7 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse
// - the HTML to load the JS (at the top) can be rendered. // - the HTML to load the JS (at the top) can be rendered.
$fake_request = $this->requestStack->getMasterRequest()->duplicate(); $fake_request = $this->requestStack->getMasterRequest()->duplicate();
$fake_request->request->set('ajax_page_state', ['libraries' => implode(',', $cumulative_assets->getAlreadyLoadedLibraries())]); $fake_request->request->set('ajax_page_state', ['libraries' => implode(',', $cumulative_assets->getAlreadyLoadedLibraries())]);
try { $html_response = $this->filterEmbeddedResponse($fake_request, $html_response);
$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. // Send this embedded HTML response.
print $html_response->getContent(); print $html_response->getContent();
...@@ -320,12 +290,6 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse ...@@ -320,12 +290,6 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse
* @param \Drupal\Core\Asset\AttachedAssetsInterface $cumulative_assets * @param \Drupal\Core\Asset\AttachedAssetsInterface $cumulative_assets
* The cumulative assets sent so far; to be updated while rendering BigPipe * The cumulative assets sent so far; to be updated while rendering BigPipe
* placeholders. * 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) { protected function sendPlaceholders(array $placeholders, array $placeholder_order, AttachedAssetsInterface $cumulative_assets) {
// Return early if there are no BigPipe placeholders to send. // Return early if there are no BigPipe placeholders to send.
...@@ -356,18 +320,7 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde ...@@ -356,18 +320,7 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde
// Render the placeholder. // Render the placeholder.
$placeholder_render_array = $placeholders[$placeholder_id]; $placeholder_render_array = $placeholders[$placeholder_id];
try { $elements = $this->renderPlaceholder($placeholder_id, $placeholder_render_array);
$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. // Create a new AjaxResponse.
$ajax_response = new AjaxResponse(); $ajax_response = new AjaxResponse();
...@@ -389,18 +342,7 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde ...@@ -389,18 +342,7 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde
// allows us to track the total set of asset libraries sent in the // allows us to track the total set of asset libraries sent in the
// initial HTML response plus all embedded AJAX responses sent so far. // 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']); $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);
$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. // Send this embedded AJAX response.
$json = $ajax_response->getContent(); $json = $ajax_response->getContent();
......
...@@ -260,94 +260,12 @@ public static function cases(ContainerInterface $container = NULL, AccountInterf ...@@ -260,94 +260,12 @@ public static function cases(ContainerInterface $container = NULL, AccountInterf
$current_time->embeddedHtmlResponse = '<time datetime=1991-03-14"></time>'; $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 [ return [
'html' => $status_messages, 'html' => $status_messages,
'html_attribute_value' => $form_action, 'html_attribute_value' => $form_action,
'html_attribute_value_subset' => $csrf_token, 'html_attribute_value_subset' => $csrf_token,
'edge_case__invalid_html' => $hello, 'edge_case__invalid_html' => $hello,
'edge_case__html_non_lazy_builder' => $current_time, '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: ...@@ -6,11 +6,11 @@ big_pipe_test:
requirements: requirements:
_access: 'TRUE' _access: 'TRUE'
no_big_pipe: big_pipe_test.no_big_pipe:
path: '/no_big_pipe' path: '/no_big_pipe'
defaults: defaults:
_controller: '\Drupal\big_pipe_test\BigPipeTestController::nope' _controller: '\Drupal\big_pipe_test\BigPipeTestController::test'
_title: '_no_big_pipe route option test' _title: 'BigPipe test'
options: options:
_no_big_pipe: TRUE _no_big_pipe: TRUE
requirements: requirements:
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
namespace Drupal\big_pipe_test; namespace Drupal\big_pipe_test;
use Drupal\big_pipe\Render\BigPipeMarkup; use Drupal\big_pipe\Render\BigPipeMarkup;
use Drupal\big_pipe_test\EventSubscriber\BigPipeTestSubscriber;
class BigPipeTestController { class BigPipeTestController {
...@@ -35,22 +34,9 @@ public function test() { ...@@ -35,22 +34,9 @@ public function test() {
// 5. Edge case: non-#lazy_builder placeholder. // 5. Edge case: non-#lazy_builder placeholder.
$build['edge_case__html_non_lazy_builder'] = $cases['edge_case__html_non_lazy_builder']->renderArray; $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 $build;
} }
/**
* @return array
*/
public static function nope() {
return ['#markup' => '<p>Nope.</p>'];
}
/** /**
* #lazy_builder callback; builds <time> markup with current time. * #lazy_builder callback; builds <time> markup with current time.
* *
...@@ -77,24 +63,4 @@ public static function helloOrYarhar() { ...@@ -77,24 +63,4 @@ 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,7 +7,6 @@ ...@@ -7,7 +7,6 @@
namespace Drupal\big_pipe_test\EventSubscriber; namespace Drupal\big_pipe_test\EventSubscriber;
use Drupal\Core\Render\AttachmentsInterface;
use Drupal\Core\Render\HtmlResponse; use Drupal\Core\Render\HtmlResponse;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent; use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\HttpKernel\KernelEvents;
...@@ -15,45 +14,13 @@ ...@@ -15,45 +14,13 @@
class BigPipeTestSubscriber implements 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. * Exposes all BigPipe placeholders (JS and no-JS) via headers for testing.
* *
* @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event * @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event
* The event to process. * The event to process.
*/ */
public function onRespondSetBigPipeDebugPlaceholderHeaders(FilterResponseEvent $event) { public function onRespond(FilterResponseEvent $event) {
$response = $event->getResponse(); $response = $event->getResponse();
if (!$response instanceof HtmlResponse) { if (!$response instanceof HtmlResponse) {
return; return;
...@@ -77,11 +44,8 @@ public function onRespondSetBigPipeDebugPlaceholderHeaders(FilterResponseEvent $ ...@@ -77,11 +44,8 @@ public function onRespondSetBigPipeDebugPlaceholderHeaders(FilterResponseEvent $
* {@inheritdoc} * {@inheritdoc}
*/ */
public static function getSubscribedEvents() { public static function getSubscribedEvents() {
// Run just before \Drupal\big_pipe\EventSubscriber\HtmlResponseBigPipeSubscriber::onRespond(). // Run *just* before \Drupal\big_pipe\EventSubscriber\HtmlResponseBigPipeSubscriber::onRespond().
$events[KernelEvents::RESPONSE][] = ['onRespondSetBigPipeDebugPlaceholderHeaders', -9999]; $events[KernelEvents::RESPONSE][] = ['onRespond', -99999];
// Run just after \Drupal\big_pipe\EventSubscriber\HtmlResponseBigPipeSubscriber::onRespond().
$events[KernelEvents::RESPONSE][] = ['onRespondTriggerException', -10001];
return $events; return $events;
} }
......
...@@ -75,8 +75,6 @@ public function placeholdersProvider() { ...@@ -75,8 +75,6 @@ public function placeholdersProvider() {
$cases['html_attribute_value_subset']->placeholder => $cases['html_attribute_value_subset']->placeholderRenderArray, $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__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_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 [ return [
...@@ -92,8 +90,6 @@ public function placeholdersProvider() { ...@@ -92,8 +90,6 @@ public function placeholdersProvider() {
$cases['html_attribute_value_subset']->placeholder => $cases['html_attribute_value_subset']->bigPipeNoJsPlaceholderRenderArray, $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__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_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, [ '_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, $cases['html']->placeholder => $cases['html']->bigPipeNoJsPlaceholderRenderArray,
...@@ -101,8 +97,6 @@ public function placeholdersProvider() { ...@@ -101,8 +97,6 @@ public function placeholdersProvider() {
$cases['html_attribute_value_subset']->placeholder => $cases['html_attribute_value_subset']->bigPipeNoJsPlaceholderRenderArray, $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__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_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