Page Cache causes different subrequests to return the same responses

faced the same issue, the first request populate own response to the following request responses in the queue. I'm looking to the cause of the problem

Please follow #3050383: PageCache cannot handle multiple main requests
This patch helps me receive the correct response.
I suggest to set a status for that issue Closed(works as designed) as this looks like a Drupal core issue.

Trying to fix this issue on the module level. Looks like `\Drupal::service('page_cache_kill_switch')->trigger();` is a great solution for this issue. I put this call after generating the response of the request list item. And this fix works for my case. I had used the `simple_oauth` module for authorization and made calls to the JSON:API and the REST api resource endpoints without duplicating responses now.
So I don't need the patch from #5 #3050383: PageCache cannot handle multiple main requests anymore as that patch has an influence on the caching of all requests to the project.

Hi voleger,

Thank you for sharing your patch.
But it doesn't seem to work for me, whereas https://www.drupal.org/project/drupal/issues/3050383 works, with it's effect on caching as you have mentioned.

Could you please share any insight on this?
I am using Drupal 8.7.3.

Thank you.

#6 patch tries to handle the cache invalidation. Instead of breaking the caching mechanism in page_cache module using the patch from #3050383: PageCache cannot handle multiple main requests issue, the #6 patch trigger events subscribers which should (as I understand) invalidate caches generated during processed request in the queue.
Current dIfferences between solutions:
- #3050383: PageCache cannot handle multiple main requests patch breaks caching of any kind of requests (API calls or just Drupal frontend requests);
- #6 patch breaks caching only for requests that processed by the module (request to the subrequests.front-controller route)

Both of the patches require some attention to improvement in cache handling.

I've change the request type in the processBatchSequence function of the SubrequestsManager class from MASTER_REQUEST to SUB_REQUEST and this solves the issue for me. I don't know if changing the type has other consequences.

Had the same issue, #3 gave me a shot to investigate further, since i don't use cookie nor basic auth but oauth with Bearer.

Confirmed as #3 reported, this is due to authentication method used in first request.
You MUST pass Authorization header string (which comprises your method, of course).
In my case, request(s) worked with Bearer token in EACH AND EVERY request and subrequest (co-dependent) made.

{
        "requestId": "userID",
        "action": "view",
        "uri": "/api",
        "headers": {
        	"Authorization": "Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImp0aSI6Ijc0Y2IzNjVjN2I4MWI0NTRhMGMwNmE0MmQ3ZTU0OGI3YzIzMjQwODE3ZTk1MjA5YTA5NDcwYjUxMjViNWE1YmqwrqwE1NDc2NmJmYTBmMDdjOGFlIn0.eyJhdWQiOiI2ZjMwZDJmZS1lNmMwLTRhNDEtODhlZC1hMjE2MzNiNjQwMzUiLCJqdGkiOiI3NGNiMzY1YzdiODFiNDU0YTBjMDZhfffffghgehetuyNDJkN2U1NDhiN2MyMzI0MDgxN2U5NTIwOWEwOTQ3MGI1MTI1YjVhNWJhNTQ3NjZiZmEwZjA3YzhhZSIsImlhdCI6MTU2NDEyNjkyOSwibmJmIjoxNTY0MTI2OTI5LCJleHAiOjE1NjU1NjY5MjksInN1YiI6IjI2Iiwic2NvcGVzIjpbImF1dGhlbnRpY2F0ZWQiLCJiaW9uZXNzIl19.tUjr9g0KOeSxE0v29iPj1gMtRbAcL9sS6xdXIY0b8uyP6Xcec-Bzcu1vOUHqcTu5Y7sxSPEkt_qtTXm1lTfJPUMwqTjbh-BBxuAtj3prAQ5NT9XIkVgHLDLoJT2h8dMvZ2V-mwsyDFBc6lvX3V-P4VScPLyV4GaJx9vTjAKB0nUstiuCQ3QI59JiW-E044Drlv-ypWcViP1vQMmqsruqF_msK3Qz2Ya6rfCL6hAuYh_0EF16oAFATQ63_x8ZQMHZsdJz3vKP9pG6SoetkCRXExFtTMgOoVK6GPwtuC57XMxjGrkoVkMeyLh3_JPRMO0ncLIegU9jfFkJUCDIlGs64A",
            "Accept": "application/vnd.application+json"
        }
    }

I left MASTER_REQUEST in line 72 of src/SubrequestsManager.php. no further patching required.

Thanks for the investigation @andsigno82! Hopefully others can post weather or not the problem is solved for them.

I must correct my first statement as reported in #10

The Bearer is mandatory to be included ONLY IN THE FIRST request (master). subsequent sub-requests WHICH DEPENDS FROM MASTER REQUEST, "grabs" the token from the master.
in case you have parallel requests which NOT DEPENDS from master, actually creating another master, Auth Header must be RE-INCLUDED

It appears that this issue occurs even when authorization isn't required, e.g., public endpoints.

While the patch from #9 "works", in my case it drastically increases the query time.

Currently investigating solutions and alternative workarounds.

The patch solves the issue for me.

I can confirm the performance decrease described in #13. In my case it is quite drastic. @Deciphered have you made progress on this?

Sorry Lukas, but I opted instead to no longer use this module and handle the multiple requests via the frontend.

I can confirm that passing the Authorization header string with each request and subrequest resolves this problem (for me, at least).

Seems #9 solved the issue for me. Will do more testing later, but at first look it works.

#9 solved the issue for me too.
here is the blueprint used:

[
    {
      "requestId": "req-1",
      "uri": "/api/node/article/22064f58-5680-4657-b2db-52b7d4eb869a",
      "action": "view",
      "headers": {
        "Accept": "application/json"
      }
    },
    {
      "requestId": "req-2",
      "uri": "/decoupled_kit/breadcrumb?path=/article/my-article",
      "action": "view",
      "headers": {
        "Accept": "application/json"
      }
    }
]

This also happens in version 3 of Drupal 9.
the change to make it work was in src/SubrequestsManager.php

->handle($request, HttpKernelInterface::MASTER_REQUEST);
to
->handle($request, HttpKernelInterface::SUB_REQUEST);

Added required changes to the Unit test

Confirming #9 fixes the issue for me in Drupal core 8.9.14

MR is the same as #9 but with test coverage changes and it is mergeable

I was having the same problem.
Same results on different action view requests.
#9 works.
Drupal version 9.2.0
Can we have this merged on the latest version of the module please ?

After further testing, i still get the problem...

[
  {
    "requestId": "req1",
    "uri": "/fr/api/json/node/homepage?include=field_video.field_media_video_file",
    "action": "view"
  },
    {
    "requestId": "req2",
    "uri": "/fr/api/json/node/homepage?include=field_offres",
    "action": "view"
  },
      {
    "requestId": "req3",
    "uri": "/fr/api/json/node/homepage?include=field_nos_realisations.field_temoignages",
    "action": "view"
  },
        {
    "requestId": "req4",
    "uri": "/fr/api/json/node/homepage?include=field_a_propos.field_chiffres_cles",
    "action": "view"
  },
  {
    "requestId": "req5",
    "uri": "/fr/api/json/node/homepage?include=field_nos_valeurs",
    "action": "view"
  },
  {
    "requestId": "req6",
    "uri": "/fr/api/json/views/latest_articles",
    "action": "view"
  },
    {
    "requestId": "req7",
    "uri": "/fr/api/json/node/homepage/xxb038a2-9df4-401f-b7fd-45c70f8ce122/field_nos_references",
    "action": "view"
  }
]

Response body of req5 contains the response of req1,req2,req3 and req4.

As you can see in the screenshot and in the comments above merging this would have a drastic consequence. I think others have mentioned that this can be solved otherwise:

I can confirm that passing the Authorization header string with each request and subrequest resolves this problem (for me, at least).

Maybe there could be some dedicated code that forwards headers from the request to all subrequests in the blueprint?

The patch gives the right results, but it seems to break tests/caching?

Is there a way to achieve the same result with requests that do NOT use authentication as @Deciphered also encountered?

Will this work in apline linux?

im getting errors trying to apply the patch with composer.

  - Applying patches for drupal/subrequests
    https://www.drupal.org/files/issues/2019-07-18/change_request_type-63049395-09.patch (Get same results on different request)
patch '-p1' --no-backup-if-mismatch -d 'web/modules/contrib/subrequests' < '/tmp/62979ad33131c.patch'
sh: patch: not found

patch '-p0' --no-backup-if-mismatch -d 'web/modules/contrib/subrequests' < '/tmp/62979ad33131c.patch'
sh: patch: not found

patch '-p2' --no-backup-if-mismatch -d 'web/modules/contrib/subrequests' < '/tmp/62979ad33131c.patch'
sh: patch: not found

patch '-p4' --no-backup-if-mismatch -d 'web/modules/contrib/subrequests' < '/tmp/62979ad33131c.patch'
sh: patch: not found

   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2019-07-18/change_request_type-63049395-09.patch

Maybe there could be some dedicated code that forwards headers from the request to all subrequests in the blueprint?

Core has an example of this; its exception handler creates a subrequest to the appropriate exception page (403 or 404), and forwards everything along. See https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21EventSubs....

One question, probably for @e0ipso to answer, is why Subrequests specifically treats the subrequests as master requests. By all rights, as far as I can tell, the subrequests should be explicitly marked as subrequests.

Was that only done in order to make Page Cache work for subrequests?

#30: Based on #26, that seems to have been the reason. It says explicitly that merging this would have a drastic consequence, with a screenshot referring to Page Cache.

I've added a functional test in a new merge request, and it reproduces the bug as described. As long as page caching is guaranteed to engage, this will occur.

I also added this new test to the existing merge request, so we will be able to see clearly that whatever fix we ultimately land on, really fixes it.

The original merge request is pretty out of date, and I guess that approach will not be taken anyway since as @e0ipso points out, it will have an outsized effect.

At least now we have some solid test coverage on which to build a fix ;)

After consulting with @effulgentsia, I think a viable approach for now is to store the cache ID on the request itself, rather than have the PageCache middleware statically cache it. That's what my newest merge request implements, and the new test passes with that, so I'm going to mark this as needing review.

Meanwhile, I think we should also implement a similar fix in core -- PageCache should store the cache ID with the request, if it hasn't already.

I left a nitpick and a question there. Great job! It is always so awesome to get a patch and some additional test coverage!!!

@phenaproxima I cannot spot why we have a test failure. Did you mention it works in your local?

Yeah, passes for me. I'm using a 9.5.x checkout of core and PHP 8.1.

Alright, re-testing with those parameters to see if that is the cause.

Well, look at that. Not only it did not fix the failing test, but now we have 3 failures.

Call to undefined method Drupal\Tests\subrequests\Unit\JsonPathReplacerTest::assertInternalType()

Yeah, this is a thing that I think was deprecated in PHPUnit 8. We should probably just open another issue to use the replacement assertions (assertIsInt(), assertIsFloat(), etc.)

I know why this is failing: Drupal CI runs all of its tests against a Drupal installation in a subdirectory, precisely to catch these kinds of bugs. My localhost doesn't install Drupal in a subdirectory.

With some Docker trickery, I managed to reproduce it. Here, once the bug occurred, were the entries in the page cache database table:

Notice those first two entries: they aren't prefixed with the web/ directory, which is where Drupal was legitimately installed. That's no good. Subrequests didn't prefix the subdirectory, thereby resulting in the cache miss.

So I guess we'll have to fix both bugs here. Assigning to myself to wrangle this one.

Updated change request type patch as per new release.

change_request_type-63049395-09.patch patch was failing, hence created new subrequests-3049395-change-request-type-47.patch patch, but not required as both seems identical hence removing.

I've upgraded to Drupal 10 and use the Merge request !9 patch, but I'm then getting the following error:

Warning: Undefined array key "sub-content-type" in Drupal\subrequests\Normalizer\MultiresponseJsonNormalizer->normalize() (line 19 of modules/contrib/subrequests/src/Normalizer/MultiresponseJsonNormalizer.php).
Drupal\subrequests\Normalizer\MultiresponseJsonNormalizer->normalize(Array, 'json', Array) (Line: 159)
Symfony\Component\Serializer\Serializer->normalize(Array, 'json', Array) (Line: 138)
Symfony\Component\Serializer\Serializer->serialize(Array, 'json', Array) (Line: 142)
Drupal\rest\Plugin\views\style\Serializer->render() (Line: 438)
Drupal\rest\Plugin\views\display\RestExport->Drupal\rest\Plugin\views\display\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 439)
Drupal\rest\Plugin\views\display\RestExport->render() (Line: 1548)
Drupal\views\ViewExecutable->render() (Line: 429)
Drupal\rest\Plugin\views\display\RestExport->execute() (Line: 1645)
Drupal\views\ViewExecutable->executeDisplay('rest_export', Array) (Line: 81)
Drupal\views\Element\View::preRenderViewElement(Array)
call_user_func_array(Array, Array) (Line: 111)
Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 797)
Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array) (Line: 386)
Drupal\Core\Render\Renderer->doRender(Array, 1) (Line: 204)
Drupal\Core\Render\Renderer->render(Array, 1) (Line: 148)
Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 149)
Drupal\Core\Render\Renderer->renderRoot(Array) (Line: 407)
Drupal\rest\Plugin\views\display\RestExport::buildResponse('slider_groups', 'rest_export', Array) (Line: 56)
Drupal\views\Routing\ViewPageController->handle('slider_groups', 'rest_export', Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 44)
Drupal\redirect_after_login\RedirectMiddleware->handle(Object, 1, 1) (Line: 68)
Drupal\simple_oauth\HttpMiddleware\BasicAuthSwap->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 53)
Asm89\Stack\Cors->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
Warning: Undefined array key "sub-content-type" in Drupal\subrequests\Normalizer\MultiresponseJsonNormalizer->normalize() (line 19 of modules/contrib/subrequests/src/Normalizer/MultiresponseJsonNormalizer.php).
Drupal\subrequests\Normalizer\MultiresponseJsonNormalizer->normalize(Array, 'json', Array) (Line: 159)
Symfony\Component\Serializer\Serializer->normalize(Array, 'json', Array) (Line: 138)
Symfony\Component\Serializer\Serializer->serialize(Array, 'json', Array) (Line: 142)
Drupal\rest\Plugin\views\style\Serializer->render() (Line: 438)
Drupal\rest\Plugin\views\display\RestExport->Drupal\rest\Plugin\views\display\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 439)
Drupal\rest\Plugin\views\display\RestExport->render() (Line: 1548)
Drupal\views\ViewExecutable->render() (Line: 429)
Drupal\rest\Plugin\views\display\RestExport->execute() (Line: 1645)
Drupal\views\ViewExecutable->executeDisplay('rest_export', Array) (Line: 81)
Drupal\views\Element\View::preRenderViewElement(Array)
call_user_func_array(Array, Array) (Line: 111)
Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 797)
Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array) (Line: 386)
Drupal\Core\Render\Renderer->doRender(Array, 1) (Line: 204)
Drupal\Core\Render\Renderer->render(Array, 1) (Line: 148)
Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 149)
Drupal\Core\Render\Renderer->renderRoot(Array) (Line: 407)
Drupal\rest\Plugin\views\display\RestExport::buildResponse('slider_groups', 'rest_export', Array) (Line: 56)
Drupal\views\Routing\ViewPageController->handle('slider_groups', 'rest_export', Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 44)
Drupal\redirect_after_login\RedirectMiddleware->handle(Object, 1, 1) (Line: 68)
Drupal\simple_oauth\HttpMiddleware\BasicAuthSwap->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 53)
Asm89\Stack\Cors->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

And I'm really getting lost into this thread. What is the status of this issue and how to fix it?

This issue is currently stalled.

Pinged @phenaproxima, he's not working on this. I'm going to take some time to go through the issue and MRs/patches.

Tests are passing, now. However I think the approach is wrong. We should be using SplObjectStorage to store cache IDs per-request object instead of the current approach. See \Drupal\Core\Path\CurrentPathStack for an example.

Okay, the tests are fixed. I also updated the implementation to be something to be more resilient than relying on manipulating the request object.

This looks wonderful!

Thanks for the work, and the patience everyone!!

Up!

Updated patch from #47 for new release 3.0.12

This issue is marked fixed, is the patch in 58 still needed?

This bug is fixed. The patch changes the type of request sent, which looks like a different issue.