diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

merged branch jfsimon/issue-6227 (PR #7180)

Browse Source 
This PR was submitted for the master branch but it was merged into the 2.2 branch instead (closes #7180).

Commits
-------

6a33bc2 [HttpKernel] Fixes AppCache + ESI + Stopwatch problem

Discussion
----------

[HttpKernel] Fixes AppCache + ESI + Stopwatch problem

There is a very special case when using builtin AppCache class as kernel wrapper, in the case of an ESI request leading to a `stale` response [B]  inside a `fresh` cached response [A]. In this case, `$token` contains the [B] debug token, but the  open `stopwatch` section ID is equal to the [A] debug token. Trying to reopen section with the [B] token throws an exception which must be caught.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| Bug mask?     | no, does @vicb agree?
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #6227, #6230

I tried to find a better solution than just wrapping thrown exceptions with `try/catch`, but IMHO the #6230 solution from @lsmith77 was in fine the best one. I just added some comments in the code to avoid the WTF reactions while reading it.

---------------------------------------------------------------------------

by vicb at 2013-02-25T16:51:51Z

@vicb never agrees :)

I don't have time to check this deeply now but I would like to see a UT.

Could your use case be expressed as "on nested terminate events" ?

---------------------------------------------------------------------------

by jfsimon at 2013-02-25T16:58:49Z

@vicb If I had an idea on how to write a conclusive test for that, I swear, it would be provided in this PR.
This commit is contained in:
Fabien Potencier 2013-02-25 18:19:32 +01:00
commit b9d3d076a6
1 changed files with 13 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/Symfony/Component/HttpKernel/Debug/TraceableEventDispatcher.php
View File

@ -388,7 +388,14 @@ class TraceableEventDispatcher implements EventDispatcherInterface, TraceableEve
break;
case KernelEvents::TERMINATE:
$token = $event->getResponse()->headers->get('X-Debug-Token');
$this->stopwatch->openSection($token);
// There is a very special case when using builtin AppCache class as kernel wrapper, in the case
// of an ESI request leading to a `stale` response [B] inside a `fresh` cached response [A].
// In this case, `$token` contains the [B] debug token, but the open `stopwatch` section ID
// is equal to the [A] debug token. Trying to reopen section with the [B] token throws an exception
// which must be caught.
try {
$this->stopwatch->openSection($token);
} catch (\LogicException $e) {}
break;
}
}
@ -410,7 +417,11 @@ class TraceableEventDispatcher implements EventDispatcherInterface, TraceableEve
break;
case KernelEvents::TERMINATE:
$token = $event->getResponse()->headers->get('X-Debug-Token');
$this->stopwatch->stopSection($token);
// In the special case described in the `preDispatch` method above, the `$token` section
// does not exist, then closing it throws an exception which must be caught.
try {
$this->stopwatch->stopSection($token);
} catch (\LogicException $e) {}
// The children profiles have been updated by the previous 'kernel.response'
// event. Only the root profile need to be updated with the 'kernel.terminate'
// timing informations.