added catching of php7 fatal exceptions in application #20110 #20111
Conversation
- added new private method to application, which would process the exception and return the exit code / throw it - added another catch into command run method, which would ensure that under PHP7 all exceptions are being catched as well
|
Doesn't #19813 fix the issue? |
|
partially. when no dispatcher is present - the fatal exceptions will not be catched anyways. though i liked the approach. i could adjust this PR so it creates a FatalThrowableError object and works with that. this way we'd avoid creating another method |
| if ($output instanceof ConsoleOutputInterface) { | ||
| $this->renderException($e, $output->getErrorOutput()); | ||
| } else { | ||
| $this->renderException($e, $output); |
There was a problem hiding this comment.
The phpdoc of renderException with @param \Exception $e is not correct anymore and needs to be updated too.
There was a problem hiding this comment.
done some changes. took the approach of #19813
now should be just fine
- implemented compatibilit with PHP7 fatal errors - using FatalThrowableError for BC with PHP5
| if (!$this->catchExceptions) { | ||
| throw $e; | ||
| throw $exception; |
There was a problem hiding this comment.
$e should be thrown here, not $exception (no wrapper)
There was a problem hiding this comment.
you mean FatalThrowableError should not be here at all?
There was a problem hiding this comment.
yes, when rethrowing, the original exception should be thrown
|
done. now the changes have become as simple and straight forward as possible |
|
Let's change the type hint (doc comment) on the renderException method also (Throwable instead of Exception), that would be consistent with what is done here |
|
Here is the patch I propose to finish this PR: --- a/src/Symfony/Component/Console/Application.php
+++ b/src/Symfony/Component/Console/Application.php
@@ -102,8 +102,6 @@ class Application
* @param OutputInterface $output An Output instance
*
* @return int 0 if everything went fine, or an error code
- *
- * @throws \Exception When doRun returns Exception
*/
public function run(InputInterface $input = null, OutputInterface $output = null)
{
@@ -634,7 +632,7 @@ class Application
/**
* Renders a caught exception.
*
- * @param \Exception $e An exception instance
+ * @param \Throwable $e An exception instance
* @param OutputInterface $output An OutputInterface instance
*/
public function renderException($e, $output)
@@ -831,8 +829,6 @@ class Application
* @param OutputInterface $output An Output instance
*
* @return int 0 if everything went fine, or an error code
- *
- * @throws \Exception when the command being run threw an exception
*/
protected function doRunCommand(Command $command, InputInterface $input, OutputInterface $output)
{Note also that it should be merged on 2.7! |
|
@bozerkins Can you take @nicolas-grekas comments into account? |
…adus) This PR was squashed before being merged into the 2.7 branch (closes #20736). Discussion ---------- [Console] fixed PHP7 Errors when not using Dispatcher | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17257, #20110, #20111 | License | MIT | Doc PR | n/a Original fix, #19813, works only when there is event dispatcher available. This PR fix the issue also for scenario without event dispatcher. Closes #20110 issue and #20111 PR connected to it. Closing #17257 , as everywhere the error is converted to exception and it should be handled Commits ------- 899fa79 [Console] fixed PHP7 Errors when not using Dispatcher
2.82.7