@ro0NL theforward icon doesn't appear anymore, can you check why?

Not sure.. what code are you referring to? Assuming symfony-demo.. i installed the latest version but i dont get a forward; or im missing it somewhere else in the app.

I see now what's happen and but I'm not sure if it's expected or not.
Before, theforward icon is displayed if at least one request is forwarded and it doesn't matter if main request is forwarded or not But Now it appear only if the main request is forwarded.
Try with this:

publicfunctionindexAction(){$bar1 =$this->forward(__CLASS__.'::barAction');$bar2 =$this->forward(__CLASS__.'::bar2Action');return$this->render('index.html.twig');}

Imo. the arrow / "Forwarded to" implies the request was as-is forwarded to a sub-request, this is why i think the current behavior is misleading. In your exampleindexAction returns it's own response, it's not forwarded.

Note the sub-requests are still available in the request panel, nothing changed with that.

rebase needed
ping@javiereguiluz also since this is "profiler panel" related

this should be done against 3.4, not master, when rebasing (no need to reopen a different PR, as Github allows changing the branch)

I don't get the use case, isn't "forward" meant to return "the" response?

This looks really weird to handle this. The forward feature should not be confused with making standard sub requests IMO (with were already profiled before#17589 btw).

Well.. imo. whats weird is the profiler assumes a forward request to the last sub request, whereas we may forward to the first sub request.

the profiler assumes a forward request to the last sub request

This is not exact. The last forwarded request is considered since "forwarded" actions could be chained.

Yet we imply a forward to bar2Action whereas you agree it should be bar1Action right?

The last forwarded request is considered since "forwarded" actions could be chained.

You mean ifA > B > C the profiler currently shows C for A? Indeed this PR would show B for A (and C for B).

Sorry for the short answer above, let me try to explain myself better.

Your case:

publicfunctionindexAction(Request$request){$bar1 =$this->forward(__CLASS__.'::barAction');$bar2 =$this->forward(__CLASS__.'::bar2Action');return$bar1;}

is not a valid use case for forwarding IMHO, and such cannot be profiled correctly.

What I assumed when working on#17589 is:

class FooController{publicfunctionindexAction(Request$request)    {return$this->forward('BarController::barAction');    }}class BarController{publicfunctionbarAction(Request$request)    {return$this->forward('BazController::bazAction');    }}

Would show thebazAction profile in the toolbar as forwarded frombarAction (with a link to its profile) and this las one as forwarded fromindexAction.

SoA > B > C, would show the C profile in the toolbar with a link to B that links A.

is not a valid use case for forwarding IMHO, and such cannot be profiled correctly.

Agree it's a bit edgy, but this PR does fix it...

So A > B > C, would show the C profile in the toolbar with a link to B that links A.

Tested it and currently A shows B, and B shows C. So no change there.

@HeahDude good catch, ive removed the_forwarded attribute.

@javiereguiluz could you have a look? Is the change clear for you?

@ro0NL the example you showed in the description is very very edgy to me. If this PR solves it, that's great ... but I wouldn't mind if that remains unsolved.

My concern is this comment from@HeahDude:

Doesn't this make any sub request a forwarded one?

If that's true ... isn't it wrong? We've never considered sub-request as forwarded requests, right?

but I wouldn't mind if that remains unsolved.

Same, but experienced this issue (once now :)) whenadditionally doing a sub request (just to trigger some logging, ignoring it's response). Yet the forwarded response was requested before that, the one we return. Might be solved by changing it's order on our side, sure :) (well.. not sure really, but i could try).

However, the case, made me think of different approach, this PR.

If that's true ... isn't it wrong?

Not really, every sub request simply gets aX-Debug-Token. Nothing new here.

Thus when we return a sub request we basically return a responsewhich already has aX-Debug-Token (otherwise this wont happen, as the profiler assigns it).

Now the profiler is updated to check if it has X-Debug-Token beforehand, if so, that becomes X-Debug-Previous-Token and again assigns a new X-Debug-Token. So only in this case we have 2 debug tokens.

Consequently if we have aX-Debug-Previous-Token we know we returned a forwarded response.

That's robust to me.

@javiereguiluz other fix is it also works if you dont use forward(), ie dont extend from base controller. Thats nice actually.

@ro0NL rebase needed again sorry

Moving to 4.1. Rebase on master might be needed, where PHP 7.1 features can be used btw.

LGTM. Needs to be reworked to take into account that this will be merged into 4.1 (mostly deprecation messages).

@fabpot good to go. However something else is bugging me:

(on master in case of a forward request)

The token differs, the link goes to the actual subrequest made (which is the right one here actually :D)

f561d2 should be5a8b10 here (which is what this PR solves)

So i dont understand whyx-debug-token-link resolves different currently 🤔 moreover shouldnt it just be the current token instead?

Also not sure if you want me to apply fabbot.io patch..

edit:X-Debug-Token-Link is only set wrong in the profiler. THe actual HTTP header value is indeed linking to the current token.

@ro0NL The WebProfiler uses new methods on the data collector class, so it looks like some Composer constraints need to be adjusted so that the web profiler does not allow using an older version of the HTTP Kernel component.

Correct, i confused deps with framework bundle. Means safety checks in templates (collector.forwardtoken|default(null)) are actually not needed 🎉

Now updated. Ill have a look at wrongX-Debug-Token-Link in profiler soonish, that's unrelated.

Thank you@ro0NL.