Problem/Motivation
In HtmlRenderer::prepare, the main content is rendered by wrapping the execution in the following:
$this->renderer->executeInRenderContext();
The page attachments are generated in the same method, but without executing the rendering in a context:
$this->invokePageAttachmentHooks($page);
The issue is that big_pipe_page_attachments calls Url::toString which invokes RendererInterface:render in the background, trying to render something without a render context. This causes the following exception to be thrown:
The controller result claims to be providing relevant cache metadata, but leaked metadata was detected.
Steps to reproduce
1. Enable bigpipe
2. In a custom route controller use this:
$build = [
'#title' => 'A title',
'content' => ['#markup' => 'Some content'],
];
return \Drupal::service('main_content_renderer.html')->renderResponse($build, \Drupal::requestStack()->getCurrentRequest(), \Drupal::routeMatch());
3. Visit the URL of the custom route
4. Notice the error.
Proposed resolution
Two solutions:
- Adjust HtmlRenderer::prepare so it invokes invokePageAttachmentHooks inside a render context.
- Adjust big_pipe_page_attachments so it calls Url::toString inside a render context.
Remaining tasks
n/a
User interface changes
n/a
API changes
n/a
Data model changes
n/a
Release notes snippet
n/a
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3303874-early-rendering-issue_3.patch | 4.44 KB | alecsmrekar |
Comments
Comment #2
wim leers🤯
How has this never been an issue so far?!
Based on the description, we should be able to reproduce this with any response? Or … only if your controller calls
main_content_renderer.htmldirectly, rather than just returning the render array? Or can you even reproduce this when your controller returns hardcoded HTML?Comment #3
alecsmrekar commentedI think this only happens when a controller returns a Response object. It doesn't happen if you return a render array.
You can see this in
EarlyRenderingControllerWrapperSubscriber::wrapControllerExecutionInRenderContext.I'm attaching an initial patch.
Comment #4
alecsmrekar commentedComment #6
alecsmrekar commentedAdjusted the patch to resolve the deprecation notices.
Comment #7
alecsmrekar commentedComment #8
alecsmrekar commentedComment #9
alecsmrekar commentedComment #11
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
One of the proposed solutions
- Adjust big_pipe_page_attachments so it calls Url::toString inside a render context.
Still appears to need to be done.
Comment #12
wim leersClarifying based on #3.
#11: no, the patch just implements the other possible solution. And that solution is actually preferable because it solves the problem generically.
Testing against
10.1.x…Comment #13
smustgrave commentedThanks @Wim Leers! D10.1 didn't have any failures
Comment #16
catchCommitted/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x, thanks!