Skip to content

Commit c927c48

Browse files
fabpotfabpot
committed
bug symfony#25255 [Console][DI] Fail gracefully (nicolas-grekas)
This PR was merged into the 3.4 branch. Discussion ---------- [Console][DI] Fail gracefully | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony/flex#212, symfony#25280 | License | MIT | Doc PR | - I already experienced this issue a few times without spending time digging it: sometimes, you call `cache:clear`, and the command quits without any output, and with 255 status code. The reason is the `@include` in `Kernel`, which makes everything silent, especially fatal errors (thanks PHP...) So if the to-be-removed container is broken for some fatal reason, the failure is really bad. To fix that, here are two measures: - use `include_once` instead of `require_once` in the dumped container: that's OK there to actually not immediately load the file, any hard failure will happen later anyway, and any soft failure will allow the `cache:clear` command to complete (like when you remove a package) - register `Application::renderException()` as the main PHP exception handler, via `Debug::ErrorHandler` when it's available End result when it fails: ![image](https://user-images.githubusercontent.com/243674/33494543-e1d07202-d6c3-11e7-9677-bc2ae72fbba9.png) instead of a blank output. Commits ------- 4a5a3f5 [Console][DI] Fail gracefully
2 parents 9f1acc1 + 4a5a3f5 commit c927c48

File tree

8 files changed

+74
-38
lines changed

8 files changed

+74
-38
lines changed

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

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
use Symfony\Component\Console\Event\ConsoleTerminateEvent;
4141
use Symfony\Component\Console\Exception\CommandNotFoundException;
4242
use Symfony\Component\Console\Exception\LogicException;
43+
use Symfony\Component\Debug\ErrorHandler;
4344
use Symfony\Component\Debug\Exception\FatalThrowableError;
4445
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
4546

@@ -118,28 +119,39 @@ public function run(InputInterface $input = null, OutputInterface $output = null
118119
$output = new ConsoleOutput();
119120
}
120121

122+
$renderException = function ($e) use ($output) {
123+
if (!$e instanceof \Exception) {
124+
$e = class_exists(FatalThrowableError::class) ? new FatalThrowableError($e) : new \ErrorException($e->getMessage(), $e->getCode(), E_ERROR, $e->getFile(), $e->getLine());
125+
}
126+
if ($output instanceof ConsoleOutputInterface) {
127+
$this->renderException($e, $output->getErrorOutput());
128+
} else {
129+
$this->renderException($e, $output);
130+
}
131+
};
132+
if ($phpHandler = set_exception_handler($renderException)) {
133+
restore_exception_handler();
134+
if (!is_array($phpHandler) || !$phpHandler[0] instanceof ErrorHandler) {
135+
$debugHandler = true;
136+
} elseif ($debugHandler = $phpHandler[0]->setExceptionHandler($renderException)) {
137+
$phpHandler[0]->setExceptionHandler($debugHandler);
138+
}
139+
}
140+
121141
if (null !== $this->dispatcher && $this->dispatcher->hasListeners(ConsoleEvents::EXCEPTION)) {
122142
@trigger_error(sprintf('The "ConsoleEvents::EXCEPTION" event is deprecated since Symfony 3.3 and will be removed in 4.0. Listen to the "ConsoleEvents::ERROR" event instead.'), E_USER_DEPRECATED);
123143
}
124144

125145
$this->configureIO($input, $output);
126146

127147
try {
128-
$e = null;
129148
$exitCode = $this->doRun($input, $output);
130149
} catch (\Exception $e) {
131-
}
132-
133-
if (null !== $e) {
134150
if (!$this->catchExceptions) {
135151
throw $e;
136152
}
137153

138-
if ($output instanceof ConsoleOutputInterface) {
139-
$this->renderException($e, $output->getErrorOutput());
140-
} else {
141-
$this->renderException($e, $output);
142-
}
154+
$renderException($e);
143155

144156
$exitCode = $e->getCode();
145157
if (is_numeric($exitCode)) {
@@ -150,6 +162,12 @@ public function run(InputInterface $input = null, OutputInterface $output = null
150162
} else {
151163
$exitCode = 1;
152164
}
165+
} finally {
166+
if (!$phpHandler) {
167+
restore_exception_handler();
168+
} elseif (!$debugHandler) {
169+
$phpHandler[0]->setExceptionHandler(null);
170+
}
153171
}
154172

155173
if ($this->autoExit) {

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -217,12 +217,16 @@ public function dump(array $options = array())
217217
{$namespaceLine}
218218
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
219219
220-
if (!class_exists(\\Container{$hash}\\{$options['class']}::class, false)) {
221-
require __DIR__.'/Container{$hash}/{$options['class']}.php';
220+
if (\\class_exists(\\Container{$hash}\\{$options['class']}::class, false)) {
221+
// no-op
222+
} elseif (!include __DIR__.'/Container{$hash}/{$options['class']}.php') {
223+
touch(__DIR__.'/Container{$hash}.legacy');
224+
225+
return;
222226
}
223227
224-
if (!class_exists({$options['class']}::class, false)) {
225-
class_alias(\\Container{$hash}\\{$options['class']}::class, {$options['class']}::class, false);
228+
if (!\\class_exists({$options['class']}::class, false)) {
229+
\\class_alias(\\Container{$hash}\\{$options['class']}::class, {$options['class']}::class, false);
226230
}
227231
228232
return new \\Container{$hash}\\{$options['class']}();
@@ -428,13 +432,13 @@ private function addServiceInclude($cId, Definition $definition, \SplObjectStora
428432
}
429433

430434
foreach (array_diff_key(array_flip($lineage), $this->inlinedRequires) as $file => $class) {
431-
$code .= sprintf(" require_once %s;\n", $file);
435+
$code .= sprintf(" include_once %s;\n", $file);
432436
}
433437
}
434438

435439
foreach ($inlinedDefinitions as $def) {
436440
if ($file = $def->getFile()) {
437-
$code .= sprintf(" require_once %s;\n", $this->dumpValue($file));
441+
$code .= sprintf(" include_once %s;\n", $this->dumpValue($file));
438442
}
439443
}
440444

@@ -1233,7 +1237,7 @@ private function addInlineRequires()
12331237
foreach ($lineage as $file) {
12341238
if (!isset($this->inlinedRequires[$file])) {
12351239
$this->inlinedRequires[$file] = true;
1236-
$code .= sprintf(" require_once %s;\n", $file);
1240+
$code .= sprintf(" include_once %s;\n", $file);
12371241
}
12381242
}
12391243

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ protected function getLazyContextIgnoreInvalidRefService()
297297
*/
298298
protected function getMethodCall1Service()
299299
{
300-
require_once '%path%foo.php';
300+
include_once '%path%foo.php';
301301

302302
$this->services['method_call1'] = $instance = new \Bar\FooClass();
303303

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
200200
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
201201
// Returns the public 'method_call1' shared service.
202202

203-
require_once ($this->targetDirs[0].'/Fixtures/includes/foo.php');
203+
include_once ($this->targetDirs[0].'/Fixtures/includes/foo.php');
204204

205205
$this->services['method_call1'] = $instance = new \Bar\FooClass();
206206

@@ -473,12 +473,16 @@ class ProjectServiceContainer extends Container
473473

474474
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
475475

476-
if (!class_exists(\Container%s\ProjectServiceContainer::class, false)) {
477-
require __DIR__.'/Container%s/ProjectServiceContainer.php';
476+
if (\class_exists(\Container%s\ProjectServiceContainer::class, false)) {
477+
// no-op
478+
} elseif (!include __DIR__.'/Container%s/ProjectServiceContainer.php') {
479+
touch(__DIR__.'/Container%s.legacy');
480+
481+
return;
478482
}
479483

480-
if (!class_exists(ProjectServiceContainer::class, false)) {
481-
class_alias(\Container%s\ProjectServiceContainer::class, ProjectServiceContainer::class, false);
484+
if (!\class_exists(ProjectServiceContainer::class, false)) {
485+
\class_alias(\Container%s\ProjectServiceContainer::class, ProjectServiceContainer::class, false);
482486
}
483487

484488
return new \Container%s\ProjectServiceContainer();

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ protected function getLazyContextIgnoreInvalidRefService()
309309
*/
310310
protected function getMethodCall1Service()
311311
{
312-
require_once '%path%foo.php';
312+
include_once '%path%foo.php';
313313

314314
$this->services['method_call1'] = $instance = new \Bar\FooClass();
315315

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ public function __construct()
4646

4747
$this->aliases = array();
4848

49-
require_once $this->targetDirs[1].'/includes/HotPath/I1.php';
50-
require_once $this->targetDirs[1].'/includes/HotPath/P1.php';
51-
require_once $this->targetDirs[1].'/includes/HotPath/T1.php';
52-
require_once $this->targetDirs[1].'/includes/HotPath/C1.php';
49+
include_once $this->targetDirs[1].'/includes/HotPath/I1.php';
50+
include_once $this->targetDirs[1].'/includes/HotPath/P1.php';
51+
include_once $this->targetDirs[1].'/includes/HotPath/T1.php';
52+
include_once $this->targetDirs[1].'/includes/HotPath/C1.php';
5353
}
5454

5555
public function getRemovedIds()
@@ -105,8 +105,8 @@ protected function getC1Service()
105105
*/
106106
protected function getC2Service()
107107
{
108-
require_once $this->targetDirs[1].'/includes/HotPath/C2.php';
109-
require_once $this->targetDirs[1].'/includes/HotPath/C3.php';
108+
include_once $this->targetDirs[1].'/includes/HotPath/C2.php';
109+
include_once $this->targetDirs[1].'/includes/HotPath/C3.php';
110110

111111
return $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3()) && false ?: '_'});
112112
}
@@ -118,7 +118,7 @@ protected function getC2Service()
118118
*/
119119
protected function getC3Service()
120120
{
121-
require_once $this->targetDirs[1].'/includes/HotPath/C3.php';
121+
include_once $this->targetDirs[1].'/includes/HotPath/C3.php';
122122

123123
return $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3();
124124
}

‎src/Symfony/Component/HttpKernel/Kernel.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -582,15 +582,21 @@ protected function initializeContainer()
582582
$cacheDir = $this->warmupDir ?: $this->getCacheDir();
583583
$cache = new ConfigCache($cacheDir.'/'.$class.'.php', $this->debug);
584584
if ($fresh = $cache->isFresh()) {
585-
$this->container = require $cache->getPath();
585+
// Silence E_WARNING to ignore "include" failures - don't use "@" to prevent silencing fatal errors
586+
$errorLevel = error_reporting(\E_ALL ^ \E_WARNING);
587+
try {
588+
$this->container = include $cache->getPath();
589+
} finally {
590+
error_reporting($errorLevel);
591+
}
586592
$fresh = \is_object($this->container);
587593
}
588594
if (!$fresh) {
589595
if ($this->debug) {
590596
$collectedLogs = array();
591597
$previousHandler = set_error_handler(function ($type, $message, $file, $line) use (&$collectedLogs, &$previousHandler) {
592598
if (E_USER_DEPRECATED !== $type && E_DEPRECATED !== $type) {
593-
return $previousHandler ? $previousHandler($type, $message, $file, $line) : false;
599+
return $previousHandler ? $previousHandler($type & ~E_WARNING, $message, $file, $line) : E_WARNING === $type;
594600
}
595601

596602
if (isset($collectedLogs[$message])) {
@@ -617,23 +623,27 @@ protected function initializeContainer()
617623
'count' => 1,
618624
);
619625
});
626+
} else {
627+
$errorLevel = error_reporting(\E_ALL ^ \E_WARNING);
620628
}
621629

622630
try {
623631
$container = null;
624632
$container = $this->buildContainer();
625633
$container->compile();
634+
635+
$oldContainer = file_exists($cache->getPath()) && is_object($oldContainer = include $cache->getPath()) ? new \ReflectionClass($oldContainer) : false;
626636
} finally {
627637
if ($this->debug) {
628638
restore_error_handler();
629639

630640
file_put_contents($cacheDir.'/'.$class.'Deprecations.log', serialize(array_values($collectedLogs)));
631641
file_put_contents($cacheDir.'/'.$class.'Compiler.log', null !== $container ? implode("\n", $container->getCompiler()->getLog()) : '');
642+
} else {
643+
error_reporting($errorLevel);
632644
}
633645
}
634646

635-
$oldContainer = file_exists($cache->getPath()) && is_object($oldContainer = @include $cache->getPath()) ? new \ReflectionClass($oldContainer) : false;
636-
637647
$this->dumpContainer($cache, $container, $class, $this->getContainerBaseClass());
638648
$this->container = require $cache->getPath();
639649
}
@@ -649,13 +659,13 @@ protected function initializeContainer()
649659
// old container files are not removed immediately,
650660
// but on a next dump of the container.
651661
$oldContainerDir = dirname($oldContainer->getFileName());
652-
foreach (glob(dirname($oldContainerDir).'/*.legacyContainer') as $legacyContainer) {
653-
if ($oldContainerDir.'.legacyContainer' !== $legacyContainer && @unlink($legacyContainer)) {
662+
foreach (glob(dirname($oldContainerDir).'/*.legacy') as $legacyContainer) {
663+
if ($oldContainerDir.'.legacy' !== $legacyContainer && @unlink($legacyContainer)) {
654664
(new Filesystem())->remove(substr($legacyContainer, 0, -16));
655665
}
656666
}
657667

658-
touch($oldContainerDir.'.legacyContainer');
668+
touch($oldContainerDir.'.legacy');
659669
}
660670

661671
if ($this->container->has('cache_warmer')) {

‎src/Symfony/Component/HttpKernel/Tests/KernelTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ public function testKernelReset()
833833

834834
$this->assertTrue(get_class($kernel->getContainer()) !== $containerClass);
835835
$this->assertFileExists($containerFile);
836-
$this->assertFileExists(dirname($containerFile).'.legacyContainer');
836+
$this->assertFileExists(dirname($containerFile).'.legacy');
837837
}
838838

839839
public function testKernelPass()

0 commit comments

Comments
 (0)