Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions UPGRADE-4.4.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,25 @@ TwigBridge
TwigBundle
----------

* Deprecated default value `twig.controller.exception::showAction` of the `twig.exception_controller` configuration option,
set it to `null` instead. This will also change the default error response format according to https://tools.ietf.org/html/rfc7807
for `json`, `xml`, `atom` and `txt` formats:
* Deprecated `twig.exception_controller` configuration option, set it to "null" and use `framework.error_controller` instead:

Before:
```yaml
twig:
exception_controller: 'App\Controller\MyExceptionController'
```

After:
```yaml
twig:
exception_controller: null

framework:
error_controller: 'App\Controller\MyExceptionController'
```

The new default exception controller will also change the error response content according to
https://tools.ietf.org/html/rfc7807 for `json`, `xml`, `atom` and `txt` formats:

Before:
```json
Expand All @@ -240,7 +256,8 @@ TwigBundle
}
```

* Deprecated the `ExceptionController` and all built-in error templates, use the error renderer mechanism of the `ErrorRenderer` component
* Deprecated the `ExceptionController` and `PreviewErrorController` controllers, use `ErrorController` from the HttpKernel component instead
* Deprecated all built-in error templates, use the error renderer mechanism of the `ErrorRenderer` component
* Deprecated loading custom error templates in non-html formats. Custom HTML error pages based on Twig keep working as before:

Before (`templates/bundles/TwigBundle/Exception/error.jsonld.twig`):
Expand Down
4 changes: 2 additions & 2 deletions UPGRADE-5.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,8 @@ TwigBundle
* The default value (`false`) of the `twig.strict_variables` configuration option has been changed to `%kernel.debug%`.
* The `transchoice` tag and filter have been removed, use the `trans` ones instead with a `%count%` parameter.
* Removed support for legacy templates directories `src/Resources/views/` and `src/Resources/<BundleName>/views/`, use `templates/` and `templates/bundles/<BundleName>/` instead.
* The default value (`twig.controller.exception::showAction`) of the `twig.exception_controller` configuration option has been changed to `null`.
* Removed `ExceptionController` class and all built-in error templates
* The `twig.exception_controller` configuration option has been removed, use `framework.error_controller` instead.
* Removed `ExceptionController`, `PreviewErrorController` classes and all built-in error templates

TwigBridge
----------
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ CHANGELOG
* Deprecated `routing.loader.service`, use `routing.loader.container` instead.
* Not tagging service route loaders with `routing.route_loader` has been deprecated.
* Overriding the methods `KernelTestCase::tearDown()` and `WebTestCase::tearDown()` without the `void` return-type is deprecated.

* Added new `error_controller` configuration to handle system exceptions

4.3.0
-----

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
use Symfony\Component\Mailer\Mailer;
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Component\PropertyInfo\PropertyInfoExtractorInterface;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Symfony\Component\Serializer\Serializer;
use Symfony\Component\Translation\Translator;
use Symfony\Component\Validator\Validation;
Expand Down Expand Up @@ -84,6 +83,9 @@ public function getConfigTreeBuilder()
->beforeNormalization()->ifString()->then(function ($v) { return [$v]; })->end()
->prototype('scalar')->end()
->end()
->scalarNode('error_controller')
->defaultValue('error_controller')
->end()
->end()
;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ public function load(array $configs, ContainerBuilder $container)
$container->setParameter('kernel.http_method_override', $config['http_method_override']);
$container->setParameter('kernel.trusted_hosts', $config['trusted_hosts']);
$container->setParameter('kernel.default_locale', $config['default_locale']);
$container->setParameter('kernel.error_controller', $config['error_controller']);

if (!$container->hasParameter('debug.file_link_format')) {
if (!$container->hasParameter('templating.helper.code.file_link_format')) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@
<argument>%kernel.debug%</argument>
<argument type="service" id="debug.file_link_formatter" />
<argument>%kernel.debug%</argument>
<argument>%kernel.charset%</argument>
<argument type="service" id="error_renderer" on-invalid="null" />
</service>

<service id="debug.file_link_formatter" class="Symfony\Component\HttpKernel\Debug\FileLinkFormatter">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8" ?>

<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing https://symfony.com/schema/routing/routing-1.0.xsd">

<route id="_preview_error" path="/{code}.{_format}">
<default key="_controller">error_controller::preview</default>
<default key="_format">html</default>
<requirement key="code">\d+</requirement>
</route>
</routes>
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
<xsd:attribute name="secret" type="xsd:string" />
<xsd:attribute name="default-locale" type="xsd:string" />
<xsd:attribute name="test" type="xsd:boolean" />
<xsd:attribute name="error-controller" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="form">
Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,19 @@
<service id="disallow_search_engine_index_response_listener" class="Symfony\Component\HttpKernel\EventListener\DisallowRobotsIndexingListener">
<tag name="kernel.event_subscriber" />
</service>

<service id="error_controller" class="Symfony\Component\HttpKernel\Controller\ErrorController" public="true">
<argument type="service" id="http_kernel" />
<argument>%kernel.error_controller%</argument>
<argument type="service" id="error_renderer" />
</service>

<service id="exception_listener" class="Symfony\Component\HttpKernel\EventListener\ExceptionListener">
<tag name="kernel.event_subscriber" />
<tag name="monolog.logger" channel="request" />
<argument>%kernel.error_controller%</argument>
<argument type="service" id="logger" on-invalid="null" />
<argument>%kernel.debug%</argument>
</service>
</services>
</container>
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function registerContainerConfiguration(LoaderInterface $loader)

public function setAnnotatedClassCache(array $annotatedClasses)
{
$annotatedClasses = array_diff($annotatedClasses, ['Symfony\Bundle\WebProfilerBundle\Controller\ExceptionController', 'Symfony\Bundle\TwigBundle\Controller\ExceptionController']);
$annotatedClasses = array_diff($annotatedClasses, ['Symfony\Bundle\WebProfilerBundle\Controller\ExceptionController', 'Symfony\Bundle\TwigBundle\Controller\ExceptionController', 'Symfony\Bundle\TwigBundle\Controller\PreviewErrorController']);

parent::setAnnotatedClassCache($annotatedClasses);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor
'transports' => [],
'enabled' => !class_exists(FullStack::class) && class_exists(Mailer::class),
],
'error_controller' => 'error_controller',
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ framework:

twig:
strict_variables: '%kernel.debug%'
exception_controller: ~
exception_controller: null # to be removed in 5.0
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ framework:

twig:
strict_variables: '%kernel.debug%'
exception_controller: ~
exception_controller: null # to be removed in 5.0
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ class MissingUserProviderTest extends AbstractWebTestCase
{
public function testUserProviderIsNeeded()
{
$this->expectException('Symfony\Component\Config\Definition\Exception\InvalidConfigurationException');
$this->expectExceptionMessage('"default" firewall requires a user provider but none was defined.');
$client = $this->createClient(['test_case' => 'MissingUserProvider', 'root_config' => 'config.yml']);
$client = $this->createClient(['test_case' => 'MissingUserProvider', 'root_config' => 'config.yml', 'debug' => true]);

$client->request('GET', '/', [], [], [
'PHP_AUTH_USER' => 'username',
'PHP_AUTH_PW' => 'pa$$word',
]);

$response = $client->getResponse();
$this->assertSame(500, $response->getStatusCode());
$this->stringContains('"default" firewall requires a user provider but none was defined.', $response->getContent());
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@
return [
new Symfony\Bundle\SecurityBundle\SecurityBundle(),
new Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
new Symfony\Bundle\TwigBundle\TwigBundle(),
new Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLoginBundle\JsonLoginBundle(),
];
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
imports:
- { resource: ./../config/default.yml }
- { resource: ./../config/framework.yml }

security:
encoders:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
imports:
- { resource: ./../config/default.yml }
- { resource: ./../config/framework.yml }

security:
encoders:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
twig:
debug: '%kernel.debug%'
strict_variables: '%kernel.debug%'
exception_controller: Symfony\Bundle\SecurityBundle\Tests\Functional\app\ExceptionController
exception_controller: null # to be removed in 5.0
5 changes: 3 additions & 2 deletions src/Symfony/Bundle/TwigBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ CHANGELOG
* marked the `TemplateIterator` as `internal`
* added HTML comment to beginning and end of `exception_full.html.twig`
* added a new `TwigHtmlErrorRenderer` for `html` format, integrated with the `ErrorRenderer` component
* deprecated `ExceptionController` class and all built-in error templates in favor of the new error renderer mechanism
* deprecated default value `twig.controller.exception::showAction` of `twig.exception_controller` configuration option, set it to `null` instead
* deprecated `ExceptionController` and `PreviewErrorController` controllers, use `ErrorController` from the `HttpKernel` component instead
* deprecated all built-in error templates in favor of the new error renderer mechanism
* deprecated `twig.exception_controller` configuration option, set it to "null" and use `framework.error_controller` configuration instead

4.2.0
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use Twig\Error\LoaderError;
use Twig\Loader\ExistsLoaderInterface;

@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.4, use the ErrorRenderer component instead.', ExceptionController::class), E_USER_DEPRECATED);
@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.4, use "%s" instead.', ExceptionController::class, \Symfony\Component\HttpKernel\Controller\ErrorController::class), E_USER_DEPRECATED);

/**
* ExceptionController renders error or exception pages for a given
Expand All @@ -28,7 +28,7 @@
* @author Fabien Potencier <fabien@symfony.com>
* @author Matthias Pigulla <mp@webfactory.de>
*
* @deprecated since Symfony 4.4, use the ErrorRenderer component instead.
* @deprecated since Symfony 4.4, use Symfony\Component\HttpKernel\Controller\ErrorController instead.
*/
class ExceptionController
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,39 +11,35 @@

namespace Symfony\Bundle\TwigBundle\Controller;

use Symfony\Component\ErrorRenderer\ErrorRenderer;
use Symfony\Component\ErrorRenderer\Exception\FlattenException;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\HttpKernelInterface;

@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.4, use the "%s" instead.', PreviewErrorController::class, \Symfony\Component\HttpKernel\Controller\ErrorController::class), E_USER_DEPRECATED);

/**
* PreviewErrorController can be used to test error pages.
*
* It will create a test exception and forward it to another controller.
*
* @author Matthias Pigulla <mp@webfactory.de>
*
* @deprecated since Symfony 4.4, use the Symfony\Component\HttpKernel\Controller\ErrorController instead.
*/
class PreviewErrorController
{
protected $kernel;
protected $controller;
private $errorRenderer;

public function __construct(HttpKernelInterface $kernel, $controller, ErrorRenderer $errorRenderer = null)
public function __construct(HttpKernelInterface $kernel, $controller)
{
$this->kernel = $kernel;
$this->controller = $controller;
$this->errorRenderer = $errorRenderer;
}

public function previewErrorPageAction(Request $request, $code)
{
$exception = FlattenException::createFromThrowable(new \Exception('Something has intentionally gone wrong.'), $code, ['X-Debug' => false]);

if (null === $this->controller && null !== $this->errorRenderer) {
return new Response($this->errorRenderer->render($exception, $request->getPreferredFormat()), $code);
}
$exception = FlattenException::createFromThrowable(new \Exception('Something has intentionally gone wrong.'), $code);

/*
* This Request mimics the parameters set by
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
* Registers the Twig exception listener if Twig is registered as a templating engine.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @internal
*/
class ExceptionListenerPass implements CompilerPassInterface
{
Expand All @@ -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')) {
Copy link
Member Author

@yceruto yceruto Aug 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// to be removed in 5.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #33476

// register the exception listener only if it's currently used, else use the provided by FrameworkBundle
if (null === $container->getParameter('twig.exception_listener.controller') && $container->hasDefinition('exception_listener')) {
$container->removeDefinition('twig.exception_listener');
} elseif ($container->hasParameter('templating.engines')) {
$engines = $container->getParameter('templating.engines');
if (!\in_array('twig', $engines)) {
$container->removeDefinition('twig.exception_listener');
} else {
$container->removeDefinition('exception_listener');

if ($container->hasParameter('templating.engines')) {
$engines = $container->getParameter('templating.engines');
if (!\in_array('twig', $engines, true)) {
$container->removeDefinition('twig.exception_listener');
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,18 @@ public function getConfigTreeBuilder()
->children()
->scalarNode('exception_controller')
->defaultValue(static function () {
@trigger_error('Relying on the default value ("twig.controller.exception::showAction") of the "twig.exception_controller" configuration option is deprecated since Symfony 4.4, set it to "null" explicitly instead, which will be the new default in 5.0.', E_USER_DEPRECATED);
@trigger_error('The "twig.exception_controller" configuration key has been deprecated in Symfony 4.4, set it to "null" and use "framework.error_controller" configuration key instead.', E_USER_DEPRECATED);

return 'twig.controller.exception::showAction';
})
->validate()
->ifTrue(static function ($v) { return null !== $v; })
->then(static function ($v) {
@trigger_error('The "twig.exception_controller" configuration key has been deprecated in Symfony 4.4, set it to "null" and use "framework.error_controller" configuration key instead.', E_USER_DEPRECATED);

return $v;
})
->end()
->end()
->end()
;
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@
<argument>%twig.exception_listener.controller%</argument>
<argument type="service" id="logger" on-invalid="null" />
<argument>%kernel.debug%</argument>
<deprecated>The "%service_id%" service is deprecated since Symfony 4.4.</deprecated>
</service>

<service id="twig.controller.exception" class="Symfony\Bundle\TwigBundle\Controller\ExceptionController" public="true">
Expand All @@ -145,7 +146,7 @@
<service id="twig.controller.preview_error" class="Symfony\Bundle\TwigBundle\Controller\PreviewErrorController" public="true">
<argument type="service" id="http_kernel" />
<argument>%twig.exception_listener.controller%</argument>
<argument type="service" id="error_renderer" on-invalid="null" />
<deprecated>The "%service_id%" service is deprecated since Symfony 4.4.</deprecated>
</service>

<service id="twig.configurator.environment" class="Symfony\Bundle\TwigBundle\DependencyInjection\Configurator\EnvironmentConfigurator">
Expand Down
Loading