The description is correct since that's exactly what we introduced in #26138.
Could you describe what's the actual issue with this new behavior? How does it break your code?

I have a bundle with a custom exception listener that handles specific exceptions selectively.
If the exception is not handled, it's left without change and supposed to propagate, either as a fatal error directly or be caught by the application using this bundle (it's not this bundle's responsibility to handle everything).
With this change in 4.0.9, my bundle now unexpectedly receives kernel.response event that has custom response handling. Additionally the _controller attribute is a closure which makes my handler crash on TypeError as it is expecting a string representation.
Also see the failing test which is passing on 4.0.8.

I'm sorry I still don't totally understand where the bug is. To help move forward, here are some comments.

If the exception is not handled, it's left without change and supposed to propagate,
either as a fatal error directly or be caught by the application using this bundle

Correct, that's how exception handling works. The exception doesn't throw back if a listener deals with it, which is what we added recently for HTTP exceptions only.

Was your app doing something special with these exceptions? If yes, shouldn't this extra logic be moved to a listener instead. If no, then the issue is not here specifically, isn it ?

my bundle now unexpectedly receives kernel.response event that has custom response handling.

yes, that feels like normal event dispatching; shouldn't your bundle be able to deal with this since that's regular request/response lifecycle?

Additionally the _controller attribute is a closure which makes my handler crash on TypeError

a closure is a valid value for the _controller attribute so that this looks like the issue is on your side here.

Also see the failing test which is passing on 4.0.8.

Thanks for the reproducer. Reading it, I continue to think this illustrate a very specific expectation for the request/response/exception lifecycle workflow. I feel like any other bundle could alter the expectations you describe here, so that the issue might be in your app, it being too tightly coupled with these expectations. I don't feel like this is something that we should qualify as a BC break or regression.

Of course, I might miss the obvious, if anyone else wants to chime in please do!

Was your app doing something special with these exceptions? If yes, shouldn't this extra logic be moved to a listener instead. If no, then the issue is not here specifically, isn it ?

It is a listener, which was broken by adding ExceptionListener which steals the event now.

yes, that feels like normal event dispatching; shouldn't your bundle be able to deal with this since that's regular request/response lifecycle?

Yes and no. Since your listener is now stealing the event and converting it to Response object, it's impossible to do custom handling & conversion.

Thanks for the reproducer. Reading it, I continue to think this illustrate a very specific expectation for the request/response/exception lifecycle workflow.

It breaks a very basic assumption that was valid for years: Symfony does not catch thrown exceptions.

a closure is a valid value for the _controller attribute so that this looks like the issue is on your side here.

Agreed, the code is not currently compatible with closure controllers, but this is just one of the consequences of the BC break though, the real issue is completely changed lifecycle for exception handling - it now steals the exception event, executes a SUB_REQUEST and modifies output.

I don't feel like this is something that we should qualify as a BC break or regression

Not really happy to hear that, especially given the SF BC promise. I'm afraid I'll have to completely rewrite part of our bundle for exception handling and also rewrite the code that expected exceptions to actually be thrown and causing fatal error.
For now I had to put breaks in place against "symfony/framework-bundle": ">4.0.8"

@Majkl578

It is a listener, which was broken by adding ExceptionListener which steals the event now.

So now your listener is executed after the new Symfony exception listener? Can you change priorities?

Can you change priorities?

Yes for the part where I want to handle the exceptions.
No for the part where I want the exception to bubble up to main() scope (outside Symfony).
Also doing such change in a bundle means that all consuming apps are required to be upgraded.

Either way, making a consumer change anything, even priorities, clearly is a BC break on its own.

Either way, making a consumer change anything, even priorities, clearly is a BC break on its own.

we can lower the priority of the added exception listener and that would fix this part of the issue, isn't it?
what would be a good number do you think?

If the exception is not handled, it's left without change and supposed to propagate

this part is more controversial: I would argue that you should not suppose what other listeners do. If you really need the exception our of the listener stack, I'd say you should throw in yours: throw $event->getException();

Note that I'm just trying to find the best middle ground between your expectations and the ones described in #25844, which justified the new listener. If you have another approach that would handle this, ideas welcome.

we can lower the priority of the added exception listener and that would fix this part of the issue, isn't it?

Sounds like a feasible compromise, although it still requires some work

what would be a good number do you think?

That's a bit tricky question. So a tricky answer would be: lower than the lowest priority used by a user. :) I guess that's not what you wanted to hear though. I don't if there are some recommendations like you should not use priorities outside X-Y range, if not, how about something like -2048 (since the logKernelException event is registered on 2048 which makes me assume it should be on top)?

Either way it will still require some amount of work to adopt to this lifecycle change:

1. exception is now logged by this listener (so a custom error handler now double-logs it),
2. exception is now caught and not bubbled outside (this realistically breaks some PHPUnit tests where I first noticed this issue),
3. a sub-request is now dispatched in my application, re-triggering multiple custom listeners,
4. I am now forced to hack in closure controllers somehow (since sub-request will trigger the listener IIRC)

Ad #25844:
a) it's easy to register a listener manually (one-liner), and I am actually surprised that no one suggested it in the issue...; on the other hand it's not easy to unregister it (requires some DI or EvM hacking/handling),
b) better (and fully BC compliant) would imho be introducing a config switch for this behavior (at least until 5.0 - could be enabled by default when TwigBundle exists, false otherwise).

looks like #27440 is related to this too

See fix in #27505

a sub-request is now dispatched in my application, re-triggering multiple custom listeners,
I am now forced to hack in closure controllers somehow (since sub-request will trigger the listener IIRC)

Subrequests are a thing since years (and TwigBundle is using subrequests to render error pages since Symfony 2.0). If you want your listener to deal only with the master request, it is very easy to check for it:

(commit reverted)

Thanks for the info. 👍

I do understand the reasoning why someone wanted this to be the default behavior though - would the proposal above (adding a switch for this behavior to FrameworkBundle 4.x) be maybe a valid compromise for both lands?

introducing a config switch for this behavior (at least until 5.0 - could be enabled by default when TwigBundle exists, false otherwise)