New ErrorRenderer FlattenException not compatible with old one Debug FlattenException

[alexander-schranz]

Symfony version(s) affected: 4.4-dev

Description

Want to move discussion from #33918 and #33916 to this issue.

The new ErrorRenderer FlattenException is not compatible with the old Debug FlattenException. And in some places you now get the new FlattenException which crashes current project codes.

How to reproduce

1. Case: Create a custom ExceptionController::showAction

Has symfony/debug installed:

use Symfony\Component\Debug\Exception\FlattenException;

public function showAction(Request $request, FlattenException $exception, DebugLoggerInterface $logger = null);

This works as expected in 4.3 and in 4.4 you will get an error Symfony\Component\ErrorRenderer\Exception\FlattenException given but expected Symfony\Component\Debug\Exception\FlattenException

2. Case: FlattenException without Debug installed

In 4.4-dev if a dependency has not debug actually installed the Debug class still exists because of a class_alias in the ErrorRenderer

use Symfony\Component\Debug\Exception\FlattenException;

FlattenException::create(new \Exception(), 200);

This will fail as the new FlattenException doesn't implement create function. If we rename to new from createFromThrowable to create this will the following would fail which is also possible currently:

use Symfony\Component\Debug\Exception\FlattenException;

FlattenException::createFromThrowable(new \Exception(), 200);

So both create and createFromThrowable should be available in 4.4.

Possible Solution

The Symfony\Component\ErrorRenderer\Exception\FlattenException should extend from the Symfony\Component\Debug\Exception\FlattenException to keep BC Compatibility in 4.x this can then be removed in 5.0.

Additional Context

I think its a similiar issue we had with the KernelBrowser and the Client here:

#31762 which was then later fixed in: #31881

[stof]

n 4.4-dev if a dependency has not debug actually installed the Debug class still exists because of a class_alias in the ErrorRenderer

I think the fact that we define a class alias is the actual issue here. Why are we doing that ?

[alexander-schranz]

@stof that would fix the second problem but the ExceptionController would still get the new FlattenException because of ExceptionListener here would still give a new FlattenException instead of the old one to the Controller

[yceruto]

We did that because the symfony/debug will be removed automatically (if it's a transitive dependecy). So any Debug\...\FlattenException typehint on project side would still work.

But actually I think the way we did it isn't totally right, because the class alias is created only when the ErrorRenderer\...\FlattenException is loaded, and that's okay if at compiler time there is no reference to Debug\...\FlattenException, e.g. when one have a custom exception controller typehinting the legacy class you'll get this error:

In RegisterControllerArgumentLocatorsPass.php line 173:
                                                                                                                                                                                                             
  Cannot determine controller argument for "App\Controller\LegacyExceptionController::showAction()": the $exception argument is type-hinted
  with the non-existent class or interface: "Symfony\Component\Debug\Exception\FlattenException".                                                                                                                                                                          

at this point the class alias doesn't exist.

(1) That makes me think the solution at least for this particular problem will be the one we did for ErrorHandler and Debug class: Create a stubs FlattenException class and mapping it directly inside the composer.json.

(2) Later, we need to fix the TwigBundle\...\ExceptionController where we can't use the new ErrorRenderer\...\FlattenException because a child class will face incompatible method signature if you've installed the symfony/debug (in this case there is no class alias at all).

(3) Next, we might need to deprecate passing the exception attribute to the configured controller in HttpKernel\...\ExceptionListener sending there the Debug\...\FlattenException to keep BC, and create a new e (or similar) attribute (instance of ErrorRenderer\...\FlattenException) to get rid of the legacy class/typehints. Consequently the new ErrorController must be updated to use the new attribute instead: public function __invoke(Request $request, FlattenException $e)

At least that's my reasoning from afar.

[yceruto]

About the FlattenException::create() method I think we should do something to make the migration go smoothly, but on the other hand, adding this method to a new component and also deprecating it is weird but possible, though there is an opinion from @nicolas-grekas about it #32873 (comment):

We provide no guarantee at the dependency level: if ppl use Debug but have it transitively, it's accepted policy to require them to make it an explicit dep as of the next version (minor included)
Said another way: the BC policy covers the API, not the deps.

Thus the solution to this problem would be requiring symfony/debug package.

[yceruto]

The Symfony\Component\ErrorRenderer\Exception\FlattenException should extend from the Symfony\Component\Debug\Exception\FlattenException to keep BC Compatibility in 4.x this can then be removed in 5.0.

That would deprecate the ErrorRenderer\...\FlattenException too as child class of the one deprecated, it wouldn't make sense to me.

[nicolas-grekas]

Thus the solution to this problem would be requiring symfony/debug package.

I think we should do this. Debug is so low level that we should make this as smooth as possible.

[yceruto]

@alexander-schranz could you please check if #33941 solves all the problems? Thanks!

[alexander-schranz]

@yceruto Tested. Fixes both cases 👍

[B-Galati]

@yceruto Hi! Just a question, what are the benefits to remove the original exception from the FlattenException class?