search

xyNNN / GoogleTagManagerBundle

Google Tag Manager Bundle for Symfony 2
https://github.com/xyNNN/GoogleTagManagerBundle
GNU Lesser General Public License v3.0
27 stars 15 forks source link

Empty data when using ESI (not the main request) #32

Open crazyball opened 5 years ago

crazyball commented 5 years ago

Bundle version : 2.4.2 Since the kernel.reset is released i've got a big issue when displaying data. I'm using esi_render to get the template who will be displaying the tagmanager data. But as it's an ESI this is not the main request, so at the end a the main request, all data is reseted and when i print it with the esi request the data is empty. What is the good practice to avoid loss of data between main and esi request in this case ? i can't avoid using esi as all my pages are cached but the datalayer part must not be cached ?

stefandoorn commented 5 years ago

@dnna @xyNNN I'm not sure what is the best fix for this use case, but maybe we can override the tag for this case? Or we override it the other way around and let PHP-PM users add that tag themselves? I'm a bit in doubt what could solve this properly.

dnna commented 5 years ago

If I understand, this is a bug with Symfony's kernel.reset event, in that it resets all requests and not just the master one? I think the proper solution in this case would be to add an extra check in this line:

https://github.com/symfony/http-kernel/blob/master/Kernel.php#L98

to ensure reset is not executed when processing a sub-request. The problem is I'm not sure what is the best way to detect it there, but I will try to find some time in the weekend to research it if noone makes a PR by then :)

For a quick fix in @crazyball's case I guess he can overwrite the service and remove the tag in his project?

stefandoorn commented 5 years ago

Or inject the RequestStack in the GTM service and only reset if it's the master request?

dnna commented 5 years ago

That would be possible too I guess, but its conceivable other bundles using kernel.reset may be affected by this too, so it would be more correct to fix in Symfony IMHO.

stefandoorn commented 5 years ago

That for sure :-)

dnna commented 5 years ago

Btw I'm not actually sure, are ESI requests treated as non-master requests in the RequestStack? Never really worked with this tech.

dnna commented 5 years ago

I have created an issue in Symfony for this. Let's see what they come up with, otherwise we can try to fix it in the bundle.

xyNNN commented 5 years ago

Thank you all for your effort. Let's wait until we get feedback from the maintainers of Symfony. Afterwards, based on the feedback, we can implement a solution (I think the mentioned solution from @stefandoorn to inject the request stack would be the best, or they fix it ;)).

dnna commented 5 years ago

According to the Symfony maintainers, ESI requests are actually real requests, not subrequests. If they are real requests, they shouldn't be sharing any state in FPM, so I'm a bit confused as to how @crazyball's solution with esi_render worked in the first place :)

@crazyball could you provide a little more information about your application and tag manager workflow?

stefandoorn commented 5 years ago

Might be using the internal Symfony ESI proxy? Not something like Varnish that does the requests from outside?