Fosrestbundle: Compatibility Symfony 4.3

[maxhelias]

The class EventListener/ExceptionListener.php extends from Symfony\Component\HttpKernel\EventListener\ExceptionListener and this last is marked final since Symfony 4.3.

The "Symfony\Component\HttpKernel\EventListener\ExceptionListener" class is considered final since Symfony 4.3. It may change without further notice as of its next major version. You should not extend it from "FOS\RestBundle\EventListener\ExceptionListener".

[pdugas]

+1 for a fix. Just hit this after doing some updating.

[maxhelias]

I'm trying to fix myself but I do not know what the ZONE_ATTRIBUTE attribute does. Because the rest of the class is identical to the parent.
So I don't know whether to make a separate listener or rather decorate the parent class.

[sarcher]

I am taking a look at this, but I'm not sure that it is a candidate for decoration, since the parent class calls duplicateRequest() internally, and that method is protected so there's no way to prevent the parent from sending a FlattenException to the controller.

So if it can't be extended and can't be decorated, the only alternative is to make a fully separate listener that duplicates some or all of the code from the Symfony one. I can't say I fully understand why we don't want a FlattenException in the controller but for backwards compatibility it should stay as a regular \Exception there anyway, so I think re-creating the existing Symfony listener seems to be the only way to go here (and should be fully backwards compatible).

If anybody else is watching and has some advice, though, I'd take it. I'm going to proceed with that for now though.

[killerwolf]

@maxhelias zone narrows listeners to only API request.

exemple :
when requesting api.mydomain.com/* all Listener are active and are inactive all other subdomain.

for this to work, you need to specify your zone with host, patterns.

fos_rest:
  zone:
    - { host: 'api.mydomain.com' }

here is the complete documentation
https://symfony.com/doc/master/bundles/FOSRestBundle/3-listener-support.html#zone-listener

[killerwolf]

The only difference between ExceptionListener from FOSRESTBundle and The one provided by symfony/http-kernel is the FlattenException in the duplicateRequest function.

is this usefull ?

[sarcher]

@killerwolf That is correct, the difference is that duplicateRequest() is overloaded to provide a FlattenException. The problem is that the ExceptionController class itself takes an \Exception, and the FlattenException class does not extend the base PHP \Exception class.

Since Symfony's class is now final, it can't be extended, and since the duplicateRequest() method is protected, it can't just be decorated either.

I can't find any indication of why we can't take a FlattenException here but it doesn't really matter, because changing the controller would be a BC break. We could remove the type hint on the controller and check for one or the other manually there, but I feel there must have been a reason that a FlattenException wasn't wanted to begin with (probably because we are serializing the exception anyway, we'd want any object data contained within, although I am not certain).

One way or another it's a bit of a mess, I made it work locally by just duplicating most of Symfony's class, but that's not a great solution. I think we need one of the maintainers to offer some direction here.

[fabpot]

@yceruto Can you have a look at this one?

[yceruto]

Well, here it's necessary "conditionally" change the _controller and exception attributes of the subrequest fired by the native ExceptionListener.

One solution can be decorating it and create a new listener (the same class can be used) for KernelEvents::REQUEST event to change the_controller and exception attributes before the next controller resolution. For optimization this new listener can be added dynamically only when needed as well, e.g. into the same onKernelException() method.

Save the original exception in onKernelException as the FlattenException won't have this reference. And, in the request listener make sure it's a subrequest and the current exception attribute is instanceof FlattenException then change the values.

This class is marked @internal so the parent class and duplicateRequest() method would be allowed to be removed, without the need to create a separate class.

Could that does work for you?

[xabbuh]

@yceruto Do you think #2018 would be enough?

[yceruto]

@xabbuh in part, still the exception attribute needs to be changed.

I will try an alternative this week.

[yceruto]

See https://github.com/FriendsOfSymfony/FOSRestBundle/pull/2025

[maxhelias]

Thanks @yceruto for fixing this 🙌