GetResponse*Events stop after a response was set #4532

Lumbendil · 2014-11-29T09:38:24Z

Q	A
Doc fix?	yes
New docs?	no
Applies to	All versions
Fixed tickets	#4516

The kernel.view and kernel.exception events stop propagation when a response is set, and it wasn't noted on the documentation.

…kernel.exception.

xabbuh · 2014-11-29T12:35:17Z

This also applies to the kernel.request event, doesn't it?

Lumbendil · 2014-11-29T12:45:19Z

@xabbuh you're right, I looked for the uses of the subclasses but not of the main class, good catch.

I'll update this PR in no time.

wouterj · 2014-11-29T13:20:38Z

components/http_kernel/introduction.rst

+.. note::
+
+    When setting a response for the ``kernel.view`` event, the propagation
+    is stopped, so the lower priority listeners on that event don't get called.


I think this might cause some confusion. What about: "When setting a response for the kernel.view event, the propagation is stopped. This means listeners with lower priority won't be executed."

A full stop makes more sense to separate the two concepts. I'll change it.

timglabisch · 2014-11-29T22:03:23Z

components/http_kernel/introduction.rst

@@ -391,6 +396,11 @@ At this stage, if no listener sets a response on the event, then an exception
 is thrown: either the controller *or* one of the view listeners must always
 return a ``Response``.

+.. note::
+
+    When setting a response for the ``kernel.view`` event, the propagation


2 times the same note?

It's for different events.

https://github.com/symfony/symfony-docs/pull/4532/files#diff-e15ca2d264f8727e6c744c50d918aecaR172
https://github.com/symfony/symfony-docs/pull/4532/files#diff-e15ca2d264f8727e6c744c50d918aecaR401

2 times " When setting a response for the kernel.view event, the propagation"

Oh yes you're right. Should be kernel.request above.

You're right, it's fixed now 😄

timglabisch · 2014-11-30T12:10:25Z

cookbook/service_container/event_listener.rst

@@ -57,6 +57,12 @@ event is just one of the core kernel events::
    the ``kernel.exception`` event, it is :class:`Symfony\\Component\\HttpKernel\\Event\\GetResponseForExceptionEvent`.
    To see what type of object each event listener receives, see :class:`Symfony\\Component\\HttpKernel\\KernelEvents`.

+.. note::
+
+    When setting a response for the ``kernel.request``, ``kernel.view`` and


shouldnt this "or" instead oft "and"?

I don't so.

In fact if you set it for kernel.request kernel.view won't even be triggered, and kernel.exception is exclusive with kernel.view (and is triggered after kernel.request) so I thought and was better.

Also, I'm refering to the set of the 3 events.

Maybe it's due to how you would write this in german by I agree with @timglabisch here that "or" sounds more correct.

What do you think about rewording this a bit to make it more clear (sadly, I don't have a good idea right now).

yeah, in german "or" would be more natural.

`kernel.request``, ``kernel.view`` and

you dont have to set kernel.request AND kernel.view AND ...., it's more like this is true for kernel.request || kernel.view | | ... :)

but "and" sounds ok for me, too.

It should indeed be OR. The event has to be either kernel.request, kernel.view or kernel.exception. It can't be kernel.request AND kernel.view AND kernel.exception at the same time, it's one of them. (but well, this is a very high technical view on this topic, I think both are ok to use)

I agree - should be OR - I've got it changed at sha: 5842f5c

Thanks!

weaverryan · 2014-12-20T20:59:35Z

Excellent addition indeed - thanks so much @Lumbendil!

…ndil) This PR was merged into the 2.3 branch. Discussion ---------- GetResponse*Events stop after a response was set | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | All versions | Fixed tickets | #4516 The kernel.view and kernel.exception events stop propagation when a response is set, and it wasn't noted on the documentation. Commits ------- bebce0e Fix incorrect event name. e729750 Changed phrasing to explain the effects off propagation stopping. d13943a Add missing info about kernel.request event. 25e1069 Added notes specifying the propagation behaviour for kernel.view and kernel.exception.

* 2.3: (27 commits) Update forms.rst link to the API documentation Re-wording parameters.yml section and removing note about vendor #4608 [#4608] Using #. for numbered bullets Changing and -> or based on conversation in #4532 Changed to definition lists from Book section Changed to definition lists Fixed heading capitalization to follow the standards Changed paragraph to heading Changed unordered list to definition list Spelling mistake tens to tons Update controllers.rst Update the_controller.rst replace Symfony2 with Symfony Linked the PDO/DBAL Session article from the Doctrine category Fix indentation of YAML example Fixed some code indentation Fixed section headers Minor grammar fix Restored the original section title ... Conflicts: book/forms.rst book/internals.rst components/http_foundation/introduction.rst cookbook/map.rst.inc

* 2.5: (30 commits) Update forms.rst link to the API documentation Re-wording parameters.yml section and removing note about vendor #4608 [#4608] Using #. for numbered bullets Changing and -> or based on conversation in #4532 Changed to definition lists from Book section Changed to definition lists Fixed heading capitalization to follow the standards Changed paragraph to heading Changed unordered list to definition list Spelling mistake tens to tons Removed double `firewall_restriction` entry Normalize the method listings on version 2.5 Update controllers.rst Update the_controller.rst replace Symfony2 with Symfony Linked the PDO/DBAL Session article from the Doctrine category Fix indentation of YAML example Fixed some code indentation Fixed section headers ...

* 2.7: (35 commits) Update forms.rst link to the API documentation Re-wording parameters.yml section and removing note about vendor #4608 [#4608] Using #. for numbered bullets Changing and -> or based on conversation in #4532 Changed to definition lists from Book section Changed to definition lists Fixed heading capitalization to follow the standards Changed paragraph to heading Changed unordered list to definition list Spelling mistake tens to tons Removed double `firewall_restriction` entry Normalize the method listings on version 2.5 Update controllers.rst Update the_controller.rst replace Symfony2 with Symfony Linked the PDO/DBAL Session article from the Doctrine category Fix indentation of YAML example Fixed some code indentation Matching up the index position with the map ...

Added notes specifying the propagation behaviour for kernel.view and …

25e1069

…kernel.exception.

Add missing info about kernel.request event.

d13943a

wouterj reviewed Nov 29, 2014
View reviewed changes

wouterj added the Status: Needs Work label Nov 29, 2014

Changed phrasing to explain the effects off propagation stopping.

e729750

timglabisch reviewed Nov 29, 2014
View reviewed changes

Fix incorrect event name.

bebce0e

timglabisch reviewed Nov 30, 2014
View reviewed changes

wouterj added Status: Reviewed and removed Status: Needs Work labels Dec 17, 2014

weaverryan merged commit bebce0e into symfony:2.3 Dec 20, 2014

weaverryan added a commit that referenced this pull request Dec 20, 2014

Changing and -> or based on conversation in #4532

5842f5c

Lumbendil deleted the kernel_events_propagation branch December 21, 2014 10:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GetResponse*Events stop after a response was set #4532

GetResponse*Events stop after a response was set #4532

Lumbendil commented Nov 29, 2014

xabbuh commented Nov 29, 2014

Lumbendil commented Nov 29, 2014

wouterj Nov 29, 2014

Lumbendil Nov 29, 2014

timglabisch Nov 29, 2014

xabbuh Nov 29, 2014

timglabisch Nov 29, 2014

xabbuh Nov 29, 2014

Lumbendil Nov 30, 2014

timglabisch Nov 30, 2014

wouterj Nov 30, 2014

Lumbendil Nov 30, 2014

timglabisch Dec 1, 2014

xabbuh Dec 1, 2014

timglabisch Dec 1, 2014

wouterj Dec 1, 2014

weaverryan Dec 20, 2014

weaverryan commented Dec 20, 2014

GetResponse*Events stop after a response was set #4532

GetResponse*Events stop after a response was set #4532

Conversation

Lumbendil commented Nov 29, 2014

xabbuh commented Nov 29, 2014

Lumbendil commented Nov 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weaverryan commented Dec 20, 2014