[EventDispatcher] Revers event tracing order #29411
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
src/Symfony/Component/EventDispatcher/Debug/TraceableEventDispatcher.php
Outdated
Show resolved
Hide resolved
nicolas-grekas
approved these changes
Dec 17, 2018
|
Thank you @ro0NL. |
nicolas-grekas
added a commit
that referenced
this pull request
Dec 17, 2018
This PR was merged into the 3.4 branch. Discussion ---------- [EventDispatcher] Revers event tracing order | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Split from #29312 for 3.4 This traces events in dispatch order. Before:  After:  Here we see it also collects "terminate" events (both kernel & console for not-called listeners). In case of exception page the fix is even better: #29312 (review) Which now correctly shows the kernel.exception event of the main request is dispatched _before_ the kernel.request event of the sub-request. Moreover, it de-duplicates events. So we actually see the sub-request events 🎉 _Havent looked at tests yet._ Commits ------- 2570d6f [EventDispatcher] Revers event tracing order
This was referenced Dec 27, 2018
fabpot
added a commit
that referenced
this pull request
Dec 29, 2018
…eset() callStack to null (mlievertz) This PR was merged into the 3.4 branch. Discussion ---------- [Event Dispatcher] fixed 29703: TraceableEventDispatcher reset() callStack to null [Event Dispatcher] fixed 29703: TraceableEventDispatcher reset now sets callStack to null with test to dispatch after reset. | Q | A | ------------- | --- | Branch? | 3.4 <!-- see below --> | Bug fix? | yes | New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #29703 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | N/A <!-- required for new features --> [Event Dispatcher] fixed 29703: TraceableEventDispatcher reset now sets callStack to null with test to dispatch after reset. Basically #29411 introduced an issue where calling dispatch after a reset would throw an error because now reset set callStack to array but dispatch expected either SplObjectStorage or null. Now reset sets null. Also added a test to verify dispatch works following a call to reset. Commits ------- 51bcdb8 [Event Dispatcher] fixed 29703: TraceableEventDispatcher reset now sets callStack to null with test to dispatch after reset.
This was referenced Jan 6, 2019
Merged
Merged
Merged
This was referenced Jan 8, 2019
fabpot
added a commit
that referenced
this pull request
Feb 21, 2019
This PR was merged into the 3.4 branch. Discussion ---------- [EventDispatcher] Fix unknown priority | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Somehow, after #29411 the profiler actually shows the security firewall `ContextListener`. This listener removes itself at call time, but at this point it's wrapped reference is already in the call stack; to be displayed in the profiler. Because the wrapped listener lazily collects its priority - it asks it from the dispatcher - we get null; the listener was already removed. This causes the profiler to render `-` by default:  This fixes it by always passing the expected priority at call time. Commits ------- 9fb619a [EventDispatcher] Fix unknown priority
fabpot
added a commit
that referenced
this pull request
Apr 2, 2019
This PR was merged into the 3.4 branch. Discussion ---------- [EventDispatcher] cleanup | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> forgotten in #29411 (https://github.com/symfony/symfony/pull/29411/files#diff-a72bc1daccda0ec7bfcfae0e8d3df141L287) Commits ------- 19134c0 [EventDispatcher] cleanup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Split from #29312 for 3.4
This traces events in dispatch order.
Before:
After:
Here we see it also collects "terminate" events (both kernel & console for not-called listeners). In case of exception page the fix is even better: #29312 (review)
Which now correctly shows the kernel.exception event of the main request is dispatched before the kernel.request event of the sub-request.
Moreover, it de-duplicates events. So we actually see the sub-request events 🎉
Havent looked at tests yet.