Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[EventDispatcher] Revers event tracing order#29411

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:3.4fromro0NL:event-order
Dec 17, 2018
Merged

[EventDispatcher] Revers event tracing order#29411

nicolas-grekas merged 1 commit intosymfony:3.4fromro0NL:event-order
Dec 17, 2018

Conversation

@ro0NL
Copy link
Contributor

@ro0NLro0NL commentedDec 1, 2018
edited
Loading

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#...
LicenseMIT
Doc PRsymfony/symfony-docs#...

Split from#29312 for 3.4

This traces events in dispatch order.

Before:

image

After:

image

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 dispatchedbefore 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.

yceruto reacted with thumbs up emojiOskarStark reacted with hooray emoji
@nicolas-grekas
Copy link
Member

Thank you@ro0NL.

@nicolas-grekasnicolas-grekas merged commit2570d6f intosymfony:3.4Dec 17, 2018
nicolas-grekas added a commit that referenced this pull requestDec 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     <!-- seehttps://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.4This traces events in dispatch order.Before:![image](https://user-images.githubusercontent.com/1047696/49330956-5f71e780-f596-11e8-8701-80458e715213.png)After:![image](https://user-images.githubusercontent.com/1047696/49330963-79abc580-f596-11e8-8666-5535afd59b31.png)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
@ro0NLro0NL deleted the event-order branchDecember 17, 2018 12:43
fabpot added a commit that referenced this pull requestDec 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     <!-- seehttps://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 referencedJan 6, 2019
fabpot added a commit that referenced this pull requestFeb 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     <!-- seehttps://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:![image](https://user-images.githubusercontent.com/1047696/50850320-d5c5ee80-1379-11e9-8516-0c6bc54512ce.png)This fixes it by always passing the expected priority at call time.Commits-------9fb619a [EventDispatcher] Fix unknown priority
@ro0NLro0NL mentioned this pull requestApr 2, 2019
fabpot added a commit that referenced this pull requestApr 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     <!-- seehttps://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 freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

3 participants

@ro0NL@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp