Problem/Motivation

Working on #2626298: REST module must cache only responses to GET requests we detected that EarlyRenderingControllerWrapperSubscriber is not executed only for GET requests.

Only GET requests are cached so we don't need to verify render contexts for other methods(i.e. POST).

Proposed resolution

Avoid to check the render context to detect early rendering for Non-GET requests.

CommentFileSizeAuthor
#20 2663638-20_early_rendering.patch1.39 KBarturs.v
#16 2663638.patch1.14 KBnick.monaco15@gmail.com
#13 2663638-13.patch1.4 KBjespermb
#2 2663638-2.patch1.39 KBmarthinal

Comments

marthinal created an issue. See original summary.

marthinal’s picture

marthinal commented
Status: Needs work » Needs review
StatusFileSize
new 2663638-2.patch1.39 KB

I don't know if we can prevent to execute the subscriber instead of verify the Method... For the moment let's try this patch.

Status: Needs review » Needs work

The last submitted patch, 2: 2663638-2.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

steveoliver’s picture

steveoliver commented
Status: Needs work » Needs review

Still applies locally to 8.1.x. Want to see what testbot has to say.

Status: Needs review » Needs work

The last submitted patch, 2: 2663638-2.patch, failed testing.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

themic8’s picture

themic8 commented

For Drupal 8.1.x.

Most recent tests from September look like they have passed.

I have also applied and tested the patch. Specifically for 8.1.7.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

cilefen’s picture

cilefen commented
Title: Avoid to check the render context to detect early rendering for Non-GET requests. » Avoid checking the render context to detect early rendering for Non-GET requests.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jespermb’s picture

jespermb commented
StatusFileSize
new 2663638-13.patch1.4 KB

I updated the patch for 8.6

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location Pune(MH), INDIA 🇮🇳
commented
Status: Needs work » Needs review

Triggering test bots.

Status: Needs review » Needs work

The last submitted patch, 13: 2663638-13.patch, failed testing. View results

nick.monaco15@gmail.com’s picture

nick.monaco15@gmail.com commented
StatusFileSize
new 2663638.patch1.14 KB

This patch seems to work.

nick.monaco15@gmail.com’s picture

nick.monaco15@gmail.com commented
Version: 8.5.x-dev » 8.6.x-dev
pfrilling’s picture

pfrilling
Location Minster, OH
commented
Status: Needs work » Needs review

I am using the jsonapi module to update commerce product variations. When using the PATCH method to update products, I received the following error:

Got error 'PHP message: Uncaught PHP Exception LogicException: "The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: Drupal\\jsonapi\\ResourceResponse." at /web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php line 154\n'

After I applied the patch in #16, my product variations updated correctly.

Status: Needs review » Needs work

The last submitted patch, 16: 2663638.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

arturs.v’s picture

arturs.v commented
StatusFileSize
new 2663638-20_early_rendering.patch1.39 KB

Patch from #16 worked in our case. Attaching another patch (functionally the same) with slightly updated formatting.

leksat’s picture

leksat commented

I met the following error when worked on a custom mutation for the GraphQL module:
LogicException: The controller result claims to be providing relevant cache metadata, but leaked metadata was detected.

I used this patch, but then I met the other error on the taxonomy term overview page:
LogicException: Render context is empty, because render() was called outside of a renderRoot() or renderPlain() call.

More details and a debug tip: https://github.com/drupal-graphql/graphql/issues/610#issuecomment-440998061

In general, I think this patch can't be used in its current form (only use wrapControllerExecutionInRenderContext for GET requests) because:
- we have several places in core where rendering happens on POST requests (see failed tests on this issue)
- there can be a lot more such cases in contrib/custom code

leksat’s picture

leksat commented

The issue was solved in GraphQL module like this:

  public function resolve($value, array $args, ResolveContext $context, ResolveInfo $info) {
    // There are cases where the Drupal entity API calls emit the cache metadata
    // in the current render context. In such cases
    // EarlyRenderingControllerWrapperSubscriber throws the leaked cache
    // metadata exception. To avoid this, wrap the execution in its own render
    // context.
    return $this->renderer->executeInRenderContext(new RenderContext(), function () use ($value, $args, $context, $info) {
      // Do the entity API calls here...
    });
  }

REST and JSON:API modules can probably do the same.

gerzenstl’s picture

gerzenstl commented

I was getting similar error mentioned on #18 when using POST method through jsonapi module.

I can confirm that patch from #20 works fine.

My setup:
Drupal: 8.6.9
eck: 1.0-alpha5
jsonapi: 1.24
jsonapi_extras: 2.15

lexsoft00’s picture

lexsoft00 commented

Patch from #20 does not work with drupal blocks.

Drupal Version
8.7.8

Whenever you update a drupal block you get the following error:

LogicException: Render context is empty, because render() was called outside of a renderRoot() or renderPlain() call. Use renderPlain()/renderRoot() or #lazy_builder/#pre_render instead. in Drupal\Core\Render\Renderer->doRender() (line 241 of /var/www/drupalvm/mydrupal8/docroot/core/lib/Drupal/Core/Render/Renderer.php).
fabianx’s picture

fabianx commented

JSON API / GraphQL / ... should not return a cacheable response for PUT / POST in the first place, but use a non-cacheable variation.

Alternatively we could check max-age on the returned response and not throw the Exception if it’s 0 (uncacheable).

Though I do think that we have extra code in Dynamic Page Cache to account for GET vs POST so a patch like this should be fine but likely needs to update the tests.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Jumping in, does this relate #3072076: JSON:API returns a CacheableResponseInterface instance for non-cacheable methods; causes unnecessary exceptions.? Which would stop JSON:API from always returning a cacheable response.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.