[HttpKernel] [EventDispatcher] TraceableEventDispatcher kills event priorities #8405
+90
−3
Conversation
This file contains hidden or 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
…s to 0 when wrapping the events. I stumpled across this problem when running multiple consecutive BrowserKit requests from a single functional test case. Sometimes the ContextListener's kernel.response event handler was called after the TestSessionListener's handler, which leads to a started session after finishing the request. This causes a "Cannot set session ID after the session has started." error on the subsequent request.
| @@ -66,6 +66,8 @@ public function addSubscriber(EventSubscriberInterface $subscriber); | |||
| * | |||
| * @param string|array $eventName The event(s) to remove a listener from | |||
| * @param callable $listener The listener to remove | |||
| * | |||
| * @return integer The listener's priority value | |||
There was a problem hiding this comment.
Ok, point taken. I've added a new method getListenerPriority() to the EventDispatcherInterface.
…ity() method on the EventDispatcherInterface
| * | ||
| * @return integer|null The listener's priority value, null if the listener wasn't found | ||
| */ | ||
| public function getListenerPriority($eventName, $listener); |
There was a problem hiding this comment.
adding a new method in an existing interface is, unfortunately, also a BC break :(
|
Hm, but how can I fix this bug without BC breaks? A workaround would be to disable the TraceableEventDispatcher for the "test" environment, but then app/config/config_test.* could not import app/config/config_dev.*. And the order of events in the env-environment would still be different than in the prod-environment, which is really bad for debugging. |
|
Since this is obviously not the right way to solve this problem I closed this pull request for now and opened a issue (#8426). |
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.
Fix a bug where the TraceableEventDispatcher sets all event priorities to 0 when wrapping the events.
I stumpled across this problem when running multiple consecutive BrowserKit requests from a single functional test case.
Sometimes the ContextListener's kernel.response event handler was called after the TestSessionListener's handler, which leads to a started session after finishing the request.
This causes a "Cannot set session ID after the session has started." error on the subsequent request.
I fixed this by introducing a new return value to the EventDispatcher's removeListener method: it now returns the priority of the Event removed so it's easy for the TraceableEventDispatcher to add a wrapped event with the same priority.