Skip to content

Commit 81f48b8

Browse files
fabpotfabpot
committed
bug symfony#22435 [Console] Fix dispatching throwables from ConsoleEvents::COMMAND (nicolas-grekas)
This PR was merged into the 2.7 branch. Discussion ---------- [Console] Fix dispatching throwables from ConsoleEvents::COMMAND | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Exceptions thrown by `ConsoleEvents::COMMAND` listeners should be trigger `ConsoleEvents::EXCEPTION` events. (best reviewed [ignoring whitespaces](https://github.com/symfony/symfony/pull/22435/files?w=1)) Commits ------- 6d6b04a [Console] Fix dispatching throwables from ConsoleEvents::COMMAND
2 parents 93b7530 + 6d6b04a commit 81f48b8

File tree

2 files changed

+85
-64
lines changed

2 files changed

+85
-64
lines changed

‎src/Symfony/Component/Console/Application.php

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,17 @@ public function run(InputInterface $input = null, OutputInterface $output = null
119119
$this->configureIO($input, $output);
120120

121121
try {
122+
$e = null;
122123
$exitCode = $this->doRun($input, $output);
123-
} catch (\Exception $e) {
124+
} catch (\Exception $x) {
125+
$e = $x;
126+
} catch (\Throwable $x) {
127+
$e = new FatalThrowableError($x);
128+
}
129+
130+
if (null !== $e) {
124131
if (!$this->catchExceptions) {
125-
throw $e;
132+
throw $x;
126133
}
127134

128135
if ($output instanceof ConsoleOutputInterface) {
@@ -184,6 +191,7 @@ public function doRun(InputInterface $input, OutputInterface $output)
184191
$input = new ArrayInput(array('command' => $this->defaultCommand));
185192
}
186193

194+
$this->runningCommand = null;
187195
// the command name MUST be the first element of the input
188196
$command = $this->find($name);
189197

@@ -841,47 +849,41 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
841849
}
842850

843851
if (null === $this->dispatcher) {
844-
try {
845-
return $command->run($input, $output);
846-
} catch (\Exception $e) {
847-
throw $e;
848-
} catch (\Throwable $e) {
849-
throw new FatalThrowableError($e);
850-
}
852+
return $command->run($input, $output);
851853
}
852854

853855
$event = new ConsoleCommandEvent($command, $input, $output);
854-
$this->dispatcher->dispatch(ConsoleEvents::COMMAND, $event);
856+
$e = null;
857+
858+
try {
859+
$this->dispatcher->dispatch(ConsoleEvents::COMMAND, $event);
855860

856-
if ($event->commandShouldRun()) {
857-
try {
858-
$e = null;
861+
if ($event->commandShouldRun()) {
859862
$exitCode = $command->run($input, $output);
860-
} catch (\Exception $x) {
861-
$e = $x;
862-
} catch (\Throwable $x) {
863-
$e = new FatalThrowableError($x);
863+
} else {
864+
$exitCode = ConsoleCommandEvent::RETURN_CODE_DISABLED;
864865
}
865-
if (null !== $e) {
866-
$event = new ConsoleExceptionEvent($command, $input, $output, $e, $e->getCode());
867-
$this->dispatcher->dispatch(ConsoleEvents::EXCEPTION, $event);
868-
869-
if ($e !== $event->getException()) {
870-
$x = $e = $event->getException();
871-
}
872-
873-
$event = new ConsoleTerminateEvent($command, $input, $output, $e->getCode());
874-
$this->dispatcher->dispatch(ConsoleEvents::TERMINATE, $event);
866+
} catch (\Exception $e) {
867+
} catch (\Throwable $e) {
868+
}
869+
if (null !== $e) {
870+
$x = $e instanceof \Exception ? $e : new FatalThrowableError($e);
871+
$event = new ConsoleExceptionEvent($command, $input, $output, $x, $x->getCode());
872+
$this->dispatcher->dispatch(ConsoleEvents::EXCEPTION, $event);
875873

876-
throw $x;
874+
if ($x !== $event->getException()) {
875+
$e = $event->getException();
877876
}
878-
} else {
879-
$exitCode = ConsoleCommandEvent::RETURN_CODE_DISABLED;
877+
$exitCode = $e->getCode();
880878
}
881879

882880
$event = new ConsoleTerminateEvent($command, $input, $output, $exitCode);
883881
$this->dispatcher->dispatch(ConsoleEvents::TERMINATE, $event);
884882

883+
if (null !== $e) {
884+
throw $e;
885+
}
886+
885887
return $event->getExitCode();
886888
}
887889

‎src/Symfony/Component/Console/Tests/ApplicationTest.php

Lines changed: 53 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -977,15 +977,28 @@ public function testRunDispatchesAllEventsWithException()
977977
$this->assertContains('before.foo.caught.after.', $tester->getDisplay());
978978
}
979979

980-
public function testRunWithError()
980+
public function testRunDispatchesAllEventsWithExceptionInListener()
981981
{
982-
if (method_exists($this, 'expectException')) {
983-
$this->expectException('Exception');
984-
$this->expectExceptionMessage('dymerr');
985-
} else {
986-
$this->setExpectedException('Exception', 'dymerr');
987-
}
982+
$dispatcher = $this->getDispatcher();
983+
$dispatcher->addListener('console.command', function () {
984+
throw new \RuntimeException('foo');
985+
});
988986

987+
$application = new Application();
988+
$application->setDispatcher($dispatcher);
989+
$application->setAutoExit(false);
990+
991+
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output) {
992+
$output->write('foo.');
993+
});
994+
995+
$tester = new ApplicationTester($application);
996+
$tester->run(array('command' => 'foo'));
997+
$this->assertContains('before.caught.after.', $tester->getDisplay());
998+
}
999+
1000+
public function testRunWithError()
1001+
{
9891002
$application = new Application();
9901003
$application->setAutoExit(false);
9911004
$application->setCatchExceptions(false);
@@ -997,7 +1010,13 @@ public function testRunWithError()
9971010
});
9981011

9991012
$tester = new ApplicationTester($application);
1000-
$tester->run(array('command' => 'dym'));
1013+
1014+
try {
1015+
$tester->run(array('command' => 'dym'));
1016+
$this->fail('Error expected.');
1017+
} catch (\Error $e) {
1018+
$this->assertSame('dymerr', $e->getMessage());
1019+
}
10011020
}
10021021

10031022
/**
@@ -1087,32 +1106,6 @@ public function testTerminalDimensions()
10871106
$this->assertSame(array($width, 80), $application->getTerminalDimensions());
10881107
}
10891108

1090-
protected function getDispatcher($skipCommand = false)
1091-
{
1092-
$dispatcher = new EventDispatcher();
1093-
$dispatcher->addListener('console.command', function (ConsoleCommandEvent $event) use ($skipCommand) {
1094-
$event->getOutput()->write('before.');
1095-
1096-
if ($skipCommand) {
1097-
$event->disableCommand();
1098-
}
1099-
});
1100-
$dispatcher->addListener('console.terminate', function (ConsoleTerminateEvent $event) use ($skipCommand) {
1101-
$event->getOutput()->writeln('after.');
1102-
1103-
if (!$skipCommand) {
1104-
$event->setExitCode(113);
1105-
}
1106-
});
1107-
$dispatcher->addListener('console.exception', function (ConsoleExceptionEvent $event) {
1108-
$event->getOutput()->write('caught.');
1109-
1110-
$event->setException(new \LogicException('caught.', $event->getExitCode(), $event->getException()));
1111-
});
1112-
1113-
return $dispatcher;
1114-
}
1115-
11161109
public function testSetRunCustomDefaultCommand()
11171110
{
11181111
$command = new \FooCommand();
@@ -1151,6 +1144,32 @@ public function testCanCheckIfTerminalIsInteractive()
11511144
$inputStream = $application->getHelperSet()->get('question')->getInputStream();
11521145
$this->assertEquals($tester->getInput()->isInteractive(), @posix_isatty($inputStream));
11531146
}
1147+
1148+
protected function getDispatcher($skipCommand = false)
1149+
{
1150+
$dispatcher = new EventDispatcher();
1151+
$dispatcher->addListener('console.command', function (ConsoleCommandEvent $event) use ($skipCommand) {
1152+
$event->getOutput()->write('before.');
1153+
1154+
if ($skipCommand) {
1155+
$event->disableCommand();
1156+
}
1157+
});
1158+
$dispatcher->addListener('console.terminate', function (ConsoleTerminateEvent $event) use ($skipCommand) {
1159+
$event->getOutput()->writeln('after.');
1160+
1161+
if (!$skipCommand) {
1162+
$event->setExitCode(ConsoleCommandEvent::RETURN_CODE_DISABLED);
1163+
}
1164+
});
1165+
$dispatcher->addListener('console.exception', function (ConsoleExceptionEvent $event) {
1166+
$event->getOutput()->write('caught.');
1167+
1168+
$event->setException(new \LogicException('caught.', $event->getExitCode(), $event->getException()));
1169+
});
1170+
1171+
return $dispatcher;
1172+
}
11541173
}
11551174

11561175
class CustomApplication extends Application

0 commit comments

Comments
 (0)