Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
added catching of php7 fatal exceptions in application #20110#20111
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
added catching of php7 fatal exceptions in application #20110#20111
Uh oh!
There was an error while loading.Please reload this page.
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
nicolas-grekas commentedSep 30, 2016
Doesn't#19813 fix the issue? |
bozerkins commentedSep 30, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 ($outputinstanceof ConsoleOutputInterface) { | ||
| $this->renderException($e,$output->getErrorOutput()); | ||
| }else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The phpdoc ofrenderException with@param \Exception $e is not correct anymore and needs to be updated too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 ($exception) { | ||
| if (!$this->catchExceptions) { | ||
| throw$e; | ||
| throw$exception; |
nicolas-grekasOct 3, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
$e should be thrown here, not$exception (no wrapper)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
you mean FatalThrowableError should not be here at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
yes, when rethrowing, the original exception should be thrown
bozerkins commentedOct 3, 2016
done. now the changes have become as simple and straight forward as possible |
nicolas-grekas commentedOct 5, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 |
nicolas-grekas commentedOct 6, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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! |
fabpot commentedNov 10, 2016
@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/aOriginal 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 handledCommits-------899fa79 [Console] fixed PHP7 Errors when not using Dispatcher
Uh oh!
There was an error while loading.Please reload this page.
2.82.7