Added new ErrorController + Preview and enabling there the error renderer mechanism #33271

yceruto · 2019-08-21T07:10:23Z

Q	A
Branch?	4.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	yes (deps=high failure is normal)
Fixed tickets	-
License	MIT
Doc PR	TODO

After deprecating the ExceptionController in TwigBundle (refs #31398) the twig.exception_controller config key becomes useless as feature provided by TwigBundle, while the preview controller is taking more relevance for the error renderer mechanish.

Proposal

Deprecate the twig.exception_controller config key in favor of framework.error_controller with default ErrorController that activates the error renderer mechanism through the current ExceptionListener, meaning also that DebugHandlersListener::onKernelException method becomes useless too.
Deprecate the PreviewErrorController from TwigBundle in favor of similar in FrameworkBundle.

So you no longer need to install TwigBundle to create a custom error controller or check the preview output of an error renderer (included TwigHtmlErrorRenderer).

Btw this would fix #31398 (comment), removing here workaround in SecurityBundle.

TODO:

Update CHANGELOG & UPGRADE files
Add tests

WDYT?

fabpot

LGTM

src/Symfony/Bundle/FrameworkBundle/Controller/ErrorController.php

src/Symfony/Bundle/FrameworkBundle/Controller/PreviewErrorController.php

src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/ExceptionListenerPass.php

src/Symfony/Component/HttpKernel/Controller/ErrorController.php

yceruto · 2019-08-22T13:02:59Z

I'm introducing here a default exception controller that will use the error rendering mechanism. If you've a completely different error rendering system you'll can to replace the default controller configuring it on framework.error_controller and done.

That possibility is available with TwigBundle installed (deprecated here), now it'll be possible with FrameworkBundle only.

src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php

src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml

yceruto · 2019-08-24T12:39:10Z

Update

As there are two exception listeners when FrameworkBundle and TwigBundle are enabled, I've removed one conditionally: the twig.exception_listener service unless the option twig.exception_listener.controller contains a custom exception controller configured, in that case, I remove the exception_listener instead.

This will be the migration path:

twig:
-    exception_controller: 'App\Controller\ExceptionController'
+    exception_controller: null

framework:
+    error_controller: 'App\Controller\ExceptionController'

yceruto · 2019-08-24T12:49:08Z

~~Btw, with this new implementation we should revert symfony/recipes#640.~~ not after 5a12b6d

yceruto · 2019-08-24T12:56:56Z

src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/ExceptionListenerPass.php

@@ -27,13 +29,18 @@ public function process(ContainerBuilder $container)
            return;
        }

-        // register the exception controller only if Twig is enabled and required dependencies do exist
-        if (!class_exists('Symfony\Component\ErrorRenderer\Exception\FlattenException') || !interface_exists('Symfony\Component\EventDispatcher\EventSubscriberInterface')) {


I've removed all interface_exists() check here becase both interfaces are currently installed as hard deps.

yceruto · 2019-08-24T12:59:26Z

This is ready for review!

src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/ExceptionListenerPass.php

fabpot · 2019-09-05T11:15:12Z

Thank you @yceruto.

… the error renderer mechanism (yceruto) This PR was merged into the 4.4 branch. Discussion ---------- Added new ErrorController + Preview and enabling there the error renderer mechanism | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes (deps=high failure is normal) | Fixed tickets | - | License | MIT | Doc PR | TODO After deprecating the `ExceptionController` in TwigBundle (refs #31398) the `twig.exception_controller` config key becomes useless as feature provided by TwigBundle, while the preview controller is taking more relevance for the error renderer mechanish. **Proposal** * Deprecate the `twig.exception_controller` config key in favor of `framework.error_controller` with default `ErrorController` that activates the error renderer mechanism through the current `ExceptionListener`, meaning also that `DebugHandlersListener::onKernelException` method becomes useless too. * Deprecate the `PreviewErrorController` from TwigBundle in favor of similar in FrameworkBundle. So you no longer need to install TwigBundle to create a custom error controller or check the preview output of an error renderer (included `TwigHtmlErrorRenderer`). Btw this would fix #31398 (comment), removing here workaround in SecurityBundle. TODO: - [x] Update CHANGELOG & UPGRADE files - [x] Add tests WDYT? Commits ------- b79532a Add ErrorController to preview and render errors

nicolas-grekas · 2019-09-05T12:46:48Z

src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/ExceptionListenerPass.php

@@ -27,13 +29,18 @@ public function process(ContainerBuilder $container)
            return;
        }

-        // register the exception controller only if Twig is enabled and required dependencies do exist
-        if (!class_exists('Symfony\Component\ErrorRenderer\Exception\FlattenException') || !interface_exists('Symfony\Component\EventDispatcher\EventSubscriberInterface')) {
+        // to be removed in 5.0


@yceruto can you please send a PR on master to cleanup this?

This PR was merged into the 5.0-dev branch. Discussion ---------- [TwigBundle] Remove legacy code | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | TODO See #33271 Commits ------- abb3258 Remove legacy code

yceruto added HttpKernel FrameworkBundle TwigBundle labels Aug 21, 2019

yceruto added this to the next milestone Aug 21, 2019

carsonbot added Status: Needs Review Feature Deprecation labels Aug 21, 2019

yceruto added Status: Needs Work and removed Status: Needs Review labels Aug 21, 2019

fabpot requested changes Aug 21, 2019

View reviewed changes

yceruto force-pushed the error_controller branch from da77722 to f7495ec Compare August 21, 2019 13:08

yceruto changed the title ~~Add ErrorController and PreviewErrorController to FrameworkBundle~~ Added new ErrorController + Preview and enabling there the error renderer mechanism Aug 21, 2019

yceruto force-pushed the error_controller branch 2 times, most recently from e64481f to 9bc74f9 Compare August 21, 2019 17:32

yceruto commented Aug 21, 2019

View reviewed changes

src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/ExceptionListenerPass.php Show resolved Hide resolved

yceruto added Status: Needs Review and removed Status: Needs Work labels Aug 21, 2019

Tobion reviewed Aug 21, 2019

View reviewed changes

src/Symfony/Component/HttpKernel/Controller/ErrorController.php Show resolved Hide resolved

Tobion reviewed Aug 22, 2019

View reviewed changes

src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php Outdated Show resolved Hide resolved

Tobion reviewed Aug 22, 2019

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml Show resolved Hide resolved

yceruto added Status: Needs Work and removed Status: Needs Review labels Aug 23, 2019

yceruto force-pushed the error_controller branch from 26eb151 to 49b23f1 Compare August 24, 2019 12:28

yceruto force-pushed the error_controller branch 2 times, most recently from 1449e0f to 2841e59 Compare August 24, 2019 12:46

yceruto commented Aug 24, 2019

View reviewed changes

yceruto removed the Status: Needs Work label Aug 24, 2019

yceruto added the Status: Needs Review label Aug 24, 2019

Tobion reviewed Aug 24, 2019

View reviewed changes

src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/ExceptionListenerPass.php Outdated Show resolved Hide resolved

yceruto force-pushed the error_controller branch from 71d7bdd to 5a12b6d Compare August 24, 2019 19:34

yceruto force-pushed the error_controller branch 5 times, most recently from 83755cb to 7cce955 Compare September 2, 2019 21:02

Add ErrorController to preview and render errors

b79532a

yceruto force-pushed the error_controller branch from 7cce955 to b79532a Compare September 2, 2019 21:04

yceruto mentioned this pull request Sep 5, 2019

Exception is logged 3 times on 4.4@dev #33468

Closed

fabpot approved these changes Sep 5, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Sep 5, 2019

fabpot mentioned this pull request Sep 5, 2019

Added new ErrorController + Preview and enabling there the error render… symfony/symfony-docs#12273

Closed

fabpot merged commit b79532a into symfony:4.4 Sep 5, 2019

yceruto deleted the error_controller branch September 5, 2019 12:06

nicolas-grekas reviewed Sep 5, 2019

View reviewed changes

yceruto mentioned this pull request Sep 5, 2019

[TwigBundle] Remove legacy code #33476

Merged

yceruto mentioned this pull request Sep 5, 2019

Add errors preview routes to the framework config symfony/recipes#645

Merged

derrabus mentioned this pull request Sep 5, 2019

Tests fail because expected exceptions are intercepted by ExceptionController #33478

Closed

nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019

This was referenced Nov 12, 2019

Release v4.4.0-BETA1 #34338

Merged

Release v5.0.0-BETA1 #34339

Merged

xabbuh mentioned this pull request Jan 23, 2020

4.3: Unable to find file "@FrameworkBundle/Resources/config/routing/errors.xml" #35442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added new ErrorController + Preview and enabling there the error renderer mechanism #33271

Added new ErrorController + Preview and enabling there the error renderer mechanism #33271

yceruto commented Aug 21, 2019 •

edited

Loading

fabpot left a comment

yceruto commented Aug 22, 2019 •

edited

Loading

yceruto commented Aug 24, 2019 •

edited

Loading

yceruto commented Aug 24, 2019 •

edited

Loading

yceruto Aug 24, 2019 •

edited

Loading

yceruto commented Aug 24, 2019

fabpot commented Sep 5, 2019

nicolas-grekas Sep 5, 2019

yceruto Sep 5, 2019

Added new ErrorController + Preview and enabling there the error renderer mechanism #33271

Added new ErrorController + Preview and enabling there the error renderer mechanism #33271

Conversation

yceruto commented Aug 21, 2019 • edited Loading

fabpot left a comment

Choose a reason for hiding this comment

yceruto commented Aug 22, 2019 • edited Loading

yceruto commented Aug 24, 2019 • edited Loading

yceruto commented Aug 24, 2019 • edited Loading

yceruto Aug 24, 2019 • edited Loading

Choose a reason for hiding this comment

yceruto commented Aug 24, 2019

fabpot commented Sep 5, 2019

nicolas-grekas Sep 5, 2019

Choose a reason for hiding this comment

yceruto Sep 5, 2019

Choose a reason for hiding this comment

yceruto commented Aug 21, 2019 •

edited

Loading

yceruto commented Aug 22, 2019 •

edited

Loading

yceruto commented Aug 24, 2019 •

edited

Loading

yceruto commented Aug 24, 2019 •

edited

Loading

yceruto Aug 24, 2019 •

edited

Loading