Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Profiler does not show response if Cache plugin is used #243

Open
ostrolucky opened this issue Jan 6, 2018 · 6 comments
Open

Profiler does not show response if Cache plugin is used #243

ostrolucky opened this issue Jan 6, 2018 · 6 comments
Labels

Comments

@ostrolucky
Copy link
Collaborator

Q A
Bug? yes
New Feature? no
Version 1.8.1

Without using cache plugin:

Now do this

            plugins:
                - 'httplug.plugin.content_length'
                - 'httplug.plugin.redirect'
+               - 'httplug.plugin.cache'

Result:
screenshot from 2018-01-06 19-50-09

I'm aware this is because Cache plugin does not pass response over to other middlewares when cache is hit, but this is major usability restriction of Profiler. I would like to not wait for response while developing, but still see responses pulled out of Cache plugin. There should be a workaround in place

@dbu
Copy link
Collaborator

dbu commented Jan 7, 2018

i am surprised why we then see the request at all, but not deep enough into the profiling to tell. is there a reason why we don't treat a cached response the same as a response from the server? i think the plugins that come before the cache in the chain should be executed the same way regardless of cache hit or miss.

or do we record the request when it enters the chain, but the response when its received from the server, before processing it in the plugin chain? then indeed, we might add a check when the response leaves the chain, and record it there if it was not yet recorded, and add some kind of warning that this is a cached response because no "real" request has been sent. detecting cache hits in the debug toolbar sounds useful anyways.

@ostrolucky ostrolucky added the bug label Oct 10, 2021
@dbu
Copy link
Collaborator

dbu commented Aug 19, 2022

is this still relevant? has it meanwhile been solved, or is it important to have this?

@ostrolucky
Copy link
Collaborator Author

Yes

@ostrolucky
Copy link
Collaborator Author

I've did some debugging today on this topic and here are my findings:

  • Response body was actually recorded, but user has to toggle plugin stack, that's where they can see it
    image
  • Reason why "main" panel doesn't show it, is because CachePlugin understandably doesn't call $next($request) plugin, which would trigger calling \Http\HttplugBundle\Collector\ProfileClient. Job of this client is to fill in clientResponse used here
    {{ include('@Httplug/http_message.html.twig', { data: stack.clientResponse, capturedBodyLength: collector.capturedBodyLength, header: 'Response' }, with_context=false) }}

One of the hook points we could use to solve this is to register Cache Listener, so that we record the response from within it. This would probably involve non-trivial refactoring of ProfilerClient where we extract recording functionality outside of the class (we cannot call the public method as it would call wrapped HTTP client).

Another idea of mine was in case clientResponse twig variable is empty, fall back to iterating through profiles and get body from first one that gives back response. Perhaps we could even get rid of ProfilerClient this way and move extra functionality it was providing (eg. time metrics) into ProfilePlugin

@dbu
Copy link
Collaborator

dbu commented Nov 14, 2022

thanks a lot for the analysis, @ostrolucky - great job!

Another idea of mine was in case clientResponse twig variable is empty, fall back to iterating through profiles and get body from first one that gives back response.

can we be sure that the first profile that has something is the right one? we should not display something arbitrarily, or in special cases show things from a previous request, or some such problem.

Perhaps we could even get rid of ProfilerClient this way and move extra functionality it was providing (eg. time metrics) into ProfilePlugin

that would be quite neat if that works.

before we do large refactorings, could we fix the situation by having ProfileClient detect when the $next was never called and attach the necessary information in that special case? working with the cache would not work if somebody for some reason implements e.g. their own cache plugin that works differently and does not do the events.

@ostrolucky
Copy link
Collaborator Author

ProfilePlugin can do it perhaps, or Collector. ProfileClient not, because it is not activated in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants