Skip to content

Commit

Permalink
feature #33065 Deprecate things that prevent \Throwable from bubbling…
Browse files Browse the repository at this point in the history
… down (fancyweb)

This PR was merged into the 4.4 branch.

Discussion
----------

Deprecate things that prevent \Throwable from bubbling down

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes (todo update CHANGELOGS & UPGRADES)
| Tests pass?   | yes
| Fixed tickets | #32605
| License       | MIT
| Doc PR        | -

~The goal of this PR is to allow `\Throwable` forwarded from the`ErrorHandler::handleException()` method (cf #33038) to bubble down in our code base without having to convert them into exceptions.~

~WIP because I'm blocked by 2 things caused by `GetResponseForExceptionEvent::getThrowable()` returning a `\Throwable` instead of an `\Exception`.~

~1. This `\Throwable` end up in `DataCollectorInterface::collect()` (cf `ProfilerListener`). So that looks impossible to support in 4.4. What can we do?~
~2. The second blocker is `ExceptionListener::duplicateRequest()`. We are not gonna rename this method to support `\Throwable` I guess. What can we do?~

Since there are blockers to do it in 4.4, let's prepare for the future and deprecate the things that block us.

Commits
-------

abef506 Deprecate things that prevent \Throwable from bubbling down
  • Loading branch information
nicolas-grekas committed Nov 5, 2019
2 parents 4cd3dc8 + abef506 commit 72dd176
Show file tree
Hide file tree
Showing 35 changed files with 158 additions and 21 deletions.
20 changes: 20 additions & 0 deletions UPGRADE-4.4.md
Expand Up @@ -5,12 +5,15 @@ Cache
-----

* Added argument `$prefix` to `AdapterInterface::clear()`
* Marked the `CacheDataCollector` class as `@final`.

Console
-------

* Deprecated finding hidden commands using an abbreviation, use the full name instead
* Deprecated returning `null` from `Command::execute()`, return `0` instead
* Deprecated the `Application::renderException()` and `Application::doRenderException()` methods,
use `renderThrowable()` and `doRenderThrowable()` instead.

Debug
-----
Expand Down Expand Up @@ -65,6 +68,7 @@ DoctrineBridge
* Deprecated `RegistryInterface`, use `Doctrine\Common\Persistence\ManagerRegistry`.
* Added a new `getMetadataDriverClass` method to replace class parameters in `AbstractDoctrineExtension`. This method
will be abstract in Symfony 5 and must be declared in extending classes.
* Marked the `DoctrineDataCollector` class as `@final`.

Filesystem
----------
Expand All @@ -91,6 +95,7 @@ FrameworkBundle
* 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.
* Marked the `RouterDataCollector` class as `@final`.

HttpClient
----------
Expand Down Expand Up @@ -144,6 +149,12 @@ HttpKernel
current directory or with a glob pattern. The fallback directories have never been advocated
so you likely do not use those in any app based on the SF Standard or Flex edition.
* Getting the container from a non-booted kernel is deprecated
* Marked the `AjaxDataCollector`, `ConfigDataCollector`, `EventDataCollector`,
`ExceptionDataCollector`, `LoggerDataCollector`, `MemoryDataCollector`,
`RequestDataCollector` and `TimeDataCollector` classes as `@final`.
* Marked the `RouterDataCollector::collect()` method as `@final`.
* The `DataCollectorInterface::collect()` and `Profiler::collect()` methods third parameter signature
will be `\Throwable $exception = null` instead of `\Exception $exception = null` in Symfony 5.0.

Lock
----
Expand All @@ -164,6 +175,7 @@ Messenger
* [BC BREAK] Removed `$retryStrategyLocator` argument from `ConsumeMessagesCommand::__construct`.
* [BC BREAK] Removed `$senderClassOrAlias` argument from `RedeliveryStamp::__construct`.
* [BC BREAK] Removed `UnknownSenderException`.
* Marked the `MessengerDataCollector` class as `@final`.

Mime
----
Expand Down Expand Up @@ -216,6 +228,11 @@ Security
) {}
```

SecurityBundle
--------------

* Marked the `SecurityDataCollector` class as `@final`.

Serializer
----------

Expand All @@ -231,13 +248,15 @@ Translation

* Deprecated support for using `null` as the locale in `Translator`.
* Deprecated accepting STDIN implicitly when using the `lint:xliff` command, use `lint:xliff -` (append a dash) instead to make it explicit.
* Marked the `TranslationDataCollector` class as `@final`.

TwigBridge
----------

* Deprecated to pass `$rootDir` and `$fileLinkFormatter` as 5th and 6th argument respectively to the
`DebugCommand::__construct()` method, swap the variables position.
* Deprecated accepting STDIN implicitly when using the `lint:twig` command, use `lint:twig -` (append a dash) instead to make it explicit.
* Marked the `TwigDataCollector` class as `@final`.

TwigBundle
----------
Expand Down Expand Up @@ -342,6 +361,7 @@ Validator
* deprecated `ValidatorBuilder::setMetadataCache`, use `ValidatorBuilder::setMappingCache` instead.
* The `Range` constraint has a new message option `notInRangeMessage` that is used when both `min` and `max` values are set.
In case you are using custom translations make sure to add one for this new message.
* Marked the `ValidatorDataCollector` class as `@final`.

WebProfilerBundle
-----------------
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bridge/Doctrine/CHANGELOG.md
Expand Up @@ -8,6 +8,7 @@ CHANGELOG
* deprecated `RegistryInterface`, use `Doctrine\Common\Persistence\ManagerRegistry`
* added support for invokable event listeners
* added `getMetadataDriverClass` method to deprecate class parameters in service configuration files
* Marked the `DoctrineDataCollector` class as `@final`.

4.3.0
-----
Expand Down
Expand Up @@ -23,6 +23,8 @@
* DoctrineDataCollector.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class DoctrineDataCollector extends DataCollector
{
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bridge/Twig/CHANGELOG.md
Expand Up @@ -11,6 +11,7 @@ CHANGELOG
* deprecated accepting STDIN implicitly when using the `lint:twig` command, use `lint:twig -` (append a dash) instead to make it explicit.
* added `--show-deprecations` option to the `lint:twig` command
* added support for Bootstrap4 switches, use `switch-custom` as `label_attr` in a `CheckboxType`
* Marked the `TwigDataCollector` class as `@final`.

4.3.0
-----
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Bridge/Twig/DataCollector/TwigDataCollector.php
Expand Up @@ -25,6 +25,8 @@
* TwigDataCollector.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class TwigDataCollector extends DataCollector implements LateDataCollectorInterface
{
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Expand Up @@ -20,6 +20,7 @@ CHANGELOG
* [BC Break] The `framework.messenger.routing.senders` config key is not deep merged anymore.
* Added `secrets:*` commands and `%env(secret:...)%` processor to deal with secrets seamlessly.
* Made `framework.session.handler_id` accept a DSN
* Marked the `RouterDataCollector` class as `@final`.

4.3.0
-----
Expand Down
12 changes: 8 additions & 4 deletions src/Symfony/Bundle/FrameworkBundle/Console/Application.php
Expand Up @@ -207,11 +207,15 @@ private function renderRegistrationErrors(InputInterface $input, OutputInterface
(new SymfonyStyle($input, $output))->warning('Some commands could not be registered:');

foreach ($this->registrationErrors as $error) {
if (!$error instanceof \Exception) {
$error = new ErrorException($error);
}
if (method_exists($this, 'doRenderThrowable')) {
$this->doRenderThrowable($error, $output);
} else {
if (!$error instanceof \Exception) {
$error = new ErrorException($error);
}

$this->doRenderException($error, $output);
$this->doRenderException($error, $output);
}
}
}
}
Expand Up @@ -19,6 +19,8 @@
* RouterDataCollector.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class RouterDataCollector extends BaseRouterDataCollector
{
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@ CHANGELOG

* Added new `argon2id` encoder, undeprecated the `bcrypt` and `argon2i` ones (using `auto` is still recommended by default.)
* Deprecated the usage of "query_string" without a "search_dn" and a "search_password" config key in Ldap factories.
* Marked the `SecurityDataCollector` class as `@final`.

4.3.0
-----
Expand Down
Expand Up @@ -34,6 +34,8 @@

/**
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class SecurityDataCollector extends DataCollector implements LateDataCollectorInterface
{
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Cache/CHANGELOG.md
Expand Up @@ -11,6 +11,7 @@ CHANGELOG
* added `DeflateMarshaller` to compress serialized values
* removed support for phpredis 4 `compression`
* [BC BREAK] `RedisTagAwareAdapter` is not compatible with `RedisCluster` from `Predis` anymore, use `phpredis` instead
* Marked the `CacheDataCollector` class as `@final`.

4.3.0
-----
Expand Down
Expand Up @@ -21,6 +21,8 @@
/**
* @author Aaron Scherer <aequasi@gmail.com>
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*
* @final since Symfony 4.4
*/
class CacheDataCollector extends DataCollector implements LateDataCollectorInterface
{
Expand Down
64 changes: 59 additions & 5 deletions src/Symfony/Component/Console/Application.php
Expand Up @@ -128,13 +128,10 @@ public function run(InputInterface $input = null, OutputInterface $output = null
}

$renderException = function (\Throwable $e) use ($output) {
if (!$e instanceof \Exception) {
$e = class_exists(ErrorException::class) ? new ErrorException($e) : (class_exists(LegacyFatalThrowableError::class) ? new LegacyFatalThrowableError($e) : new \ErrorException($e->getMessage(), $e->getCode(), E_ERROR, $e->getFile(), $e->getLine()));
}
if ($output instanceof ConsoleOutputInterface) {
$this->renderException($e, $output->getErrorOutput());
$this->renderThrowable($e, $output->getErrorOutput());
} else {
$this->renderException($e, $output);
$this->renderThrowable($e, $output);
}
};
if ($phpHandler = set_exception_handler($renderException)) {
Expand Down Expand Up @@ -792,20 +789,77 @@ public static function getAbbreviations($names)

/**
* Renders a caught exception.
*
* @deprecated since Symfony 4.4, use "renderThrowable()" instead
*/
public function renderException(\Exception $e, OutputInterface $output)
{
@trigger_error(sprintf('The "%s::renderException()" method is deprecated since Symfony 4.4, use "renderThrowable()" instead.', __CLASS__), E_USER_DEPRECATED);

$output->writeln('', OutputInterface::VERBOSITY_QUIET);

$this->doRenderException($e, $output);

$this->finishRenderThrowableOrException($output);
}

public function renderThrowable(\Throwable $e, OutputInterface $output): void
{
if (__CLASS__ !== \get_class($this) && __CLASS__ === (new \ReflectionMethod($this, 'renderThrowable'))->getDeclaringClass()->getName() && __CLASS__ !== (new \ReflectionMethod($this, 'renderException'))->getDeclaringClass()->getName()) {
@trigger_error(sprintf('The "%s::renderException()" method is deprecated since Symfony 4.4, use "renderThrowable()" instead.', __CLASS__), E_USER_DEPRECATED);

if (!$e instanceof \Exception) {
$e = class_exists(ErrorException::class) ? new ErrorException($e) : (class_exists(LegacyFatalThrowableError::class) ? new LegacyFatalThrowableError($e) : new \ErrorException($e->getMessage(), $e->getCode(), E_ERROR, $e->getFile(), $e->getLine()));
}

$this->renderException($e, $output);

return;
}

$output->writeln('', OutputInterface::VERBOSITY_QUIET);

$this->doRenderThrowable($e, $output);

$this->finishRenderThrowableOrException($output);
}

private function finishRenderThrowableOrException(OutputInterface $output): void
{
if (null !== $this->runningCommand) {
$output->writeln(sprintf('<info>%s</info>', sprintf($this->runningCommand->getSynopsis(), $this->getName())), OutputInterface::VERBOSITY_QUIET);
$output->writeln('', OutputInterface::VERBOSITY_QUIET);
}
}

/**
* @deprecated since Symfony 4.4, use "doRenderThrowable()" instead
*/
protected function doRenderException(\Exception $e, OutputInterface $output)
{
@trigger_error(sprintf('The "%s::doRenderException()" method is deprecated since Symfony 4.4, use "doRenderThrowable()" instead.', __CLASS__), E_USER_DEPRECATED);

$this->doActuallyRenderThrowable($e, $output);
}

protected function doRenderThrowable(\Throwable $e, OutputInterface $output): void
{
if (__CLASS__ !== \get_class($this) && __CLASS__ === (new \ReflectionMethod($this, 'doRenderThrowable'))->getDeclaringClass()->getName() && __CLASS__ !== (new \ReflectionMethod($this, 'doRenderException'))->getDeclaringClass()->getName()) {
@trigger_error(sprintf('The "%s::doRenderException()" method is deprecated since Symfony 4.4, use "doRenderThrowable()" instead.', __CLASS__), E_USER_DEPRECATED);

if (!$e instanceof \Exception) {
$e = class_exists(ErrorException::class) ? new ErrorException($e) : (class_exists(LegacyFatalThrowableError::class) ? new LegacyFatalThrowableError($e) : new \ErrorException($e->getMessage(), $e->getCode(), E_ERROR, $e->getFile(), $e->getLine()));
}

$this->doRenderException($e, $output);

return;
}

$this->doActuallyRenderThrowable($e, $output);
}

private function doActuallyRenderThrowable(\Throwable $e, OutputInterface $output): void
{
do {
$message = trim($e->getMessage());
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Console/CHANGELOG.md
Expand Up @@ -11,6 +11,8 @@ CHANGELOG
* marked all dispatched event classes as `@final`
* added support for displaying table horizontally
* deprecated returning `null` from `Command::execute()`, return `0` instead
* Deprecated the `Application::renderException()` and `Application::doRenderException()` methods,
use `renderThrowable()` and `doRenderThrowable()` instead.

4.3.0
-----
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Component/HttpKernel/CHANGELOG.md
Expand Up @@ -15,6 +15,12 @@ CHANGELOG
* Marked all dispatched event classes as `@final`
* Added `ErrorController` to enable the preview and error rendering mechanism
* Getting the container from a non-booted kernel is deprecated.
* Marked the `AjaxDataCollector`, `ConfigDataCollector`, `EventDataCollector`,
`ExceptionDataCollector`, `LoggerDataCollector`, `MemoryDataCollector`,
`RequestDataCollector` and `TimeDataCollector` classes as `@final`.
* Marked the `RouterDataCollector::collect()` method as `@final`.
* The `DataCollectorInterface::collect()` and `Profiler::collect()` methods third parameter signature
will be `\Throwable $exception = null` instead of `\Exception $exception = null` in Symfony 5.0.

4.3.0
-----
Expand Down
Expand Up @@ -18,6 +18,8 @@
* AjaxDataCollector.
*
* @author Bart van den Burg <bart@burgov.nl>
*
* @final since Symfony 4.4
*/
class AjaxDataCollector extends DataCollector
{
Expand Down
Expand Up @@ -19,6 +19,8 @@

/**
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class ConfigDataCollector extends DataCollector implements LateDataCollectorInterface
{
Expand Down
Expand Up @@ -23,6 +23,8 @@
* EventDataCollector.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class EventDataCollector extends DataCollector implements LateDataCollectorInterface
{
Expand Down
Expand Up @@ -19,6 +19,8 @@
* ExceptionDataCollector.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class ExceptionDataCollector extends DataCollector
{
Expand Down
Expand Up @@ -21,6 +21,8 @@
* LogDataCollector.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class LoggerDataCollector extends DataCollector implements LateDataCollectorInterface
{
Expand Down
Expand Up @@ -18,6 +18,8 @@
* MemoryDataCollector.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class MemoryDataCollector extends DataCollector implements LateDataCollectorInterface
{
Expand Down
Expand Up @@ -22,6 +22,8 @@

/**
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class RequestDataCollector extends DataCollector implements EventSubscriberInterface, LateDataCollectorInterface
{
Expand Down
Expand Up @@ -33,6 +33,8 @@ public function __construct()

/**
* {@inheritdoc}
*
* @final since Symfony 4.4
*/
public function collect(Request $request, Response $response, \Exception $exception = null)
{
Expand Down
Expand Up @@ -19,6 +19,8 @@

/**
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.4
*/
class TimeDataCollector extends DataCollector implements LateDataCollectorInterface
{
Expand Down

0 comments on commit 72dd176

Please sign in to comment.