Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bin/magento must return non zero on exceptions #3189

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
$productIds = $productCollection->getAllIds();
if (!count($productIds)) {
$output->writeln("<info>No product images to resize</info>");
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really an error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LeeSaferite I rather would have it as an error so you can choose to ignore it in your 'external' tooling

}

try {
Expand All @@ -99,10 +100,12 @@ protected function execute(InputInterface $input, OutputInterface $output)
}
} catch (\Exception $e) {
$output->writeln("<error>{$e->getMessage()}</error>");
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}

$output->write("\n");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like you are missing an explicit success return here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LeeSaferite nice catch, I'll update it, thanks.

$output->writeln("<info>Product images resized successfully</info>");
return \Magento\Framework\Console\Cli::RETURN_SUCCESS;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,6 @@ protected function execute(InputInterface $input, OutputInterface $output)
'Magento\Deploy\Model\Deployer',
['filesUtil' => $filesUtil, 'output' => $output, 'isDryRun' => $options[self::DRY_RUN_OPTION]]
);
$deployer->deploy($this->objectManagerFactory, $languages);
return $deployer->deploy($this->objectManagerFactory, $languages);
}
}
3 changes: 2 additions & 1 deletion app/code/Magento/Deploy/Console/Command/SetModeCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
if ($output->getVerbosity() >= OutputInterface::VERBOSITY_VERBOSE) {
$output->writeln($e->getTraceAsString());
}
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
}
}
8 changes: 7 additions & 1 deletion app/code/Magento/Deploy/Model/Deployer.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ function (AlternativeSourceInterface $alternative) {
*
* @param ObjectManagerFactory $omFactory
* @param array $locales
* @return void
* @return int
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.NPathComplexity)
*/
Expand Down Expand Up @@ -213,6 +213,12 @@ public function deploy(ObjectManagerFactory $omFactory, array $locales)
if (!$this->isDryRun) {
$this->versionStorage->save($version);
}
if ($this->errorCount > 0) {
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}

return \Magento\Framework\Console\Cli::RETURN_SUCCESS;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,12 @@ protected function execute(InputInterface $input, OutputInterface $output)
foreach ($failures as $message) {
$output->writeln(' - ' . $message);
}
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
} else {
$output->writeln('PASSED (' . count($runCommands) . ')');
}
return 0;
return \Magento\Framework\Console\Cli::RETURN_SUCCESS;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,12 @@ protected function execute(InputInterface $input, OutputInterface $output)
$output->write($result);
}

return;
return \Magento\Framework\Console\Cli::RETURN_SUCCESS;
} catch (\Exception $exception) {
$errorMessage = $exception->getMessage();
$output->writeln("<error>$errorMessage</error>");
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ protected function configure()
protected function execute(InputInterface $input, OutputInterface $output)
{
$indexers = $this->getIndexers($input);
$returnValue = \Magento\Framework\Console\Cli::RETURN_SUCCESS;
foreach ($indexers as $indexer) {
try {
$this->validateIndexerStatus($indexer);
Expand All @@ -64,11 +65,16 @@ protected function execute(InputInterface $input, OutputInterface $output)
);
} catch (LocalizedException $e) {
$output->writeln($e->getMessage());
// we must have an exit code higher than zero to indicate something was wrong
$returnValue = \Magento\Framework\Console\Cli::RETURN_FAILURE;
} catch (\Exception $e) {
$output->writeln($indexer->getTitle() . ' indexer process unknown error:');
$output->writeln($e->getMessage());
// we must have an exit code higher than zero to indicate something was wrong
$returnValue = \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
}
return $returnValue;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ protected function execute(InputInterface $input, OutputInterface $output)

$indexers = $this->getIndexers($input);

$returnValue = \Magento\Framework\Console\Cli::RETURN_SUCCESS;
foreach ($indexers as $indexer) {
try {
$previousStatus = $indexer->isScheduled() ? 'Update by Schedule' : 'Update on Save';
Expand All @@ -63,13 +64,17 @@ protected function execute(InputInterface $input, OutputInterface $output)
}
} catch (LocalizedException $e) {
$output->writeln($e->getMessage() . PHP_EOL);
// we must have an exit code higher than zero to indicate something was wrong
$returnValue = \Magento\Framework\Console\Cli::RETURN_FAILURE;
} catch (\Exception $e) {
$output->writeln($indexer->getTitle() . " indexer process unknown error:" . PHP_EOL);
$output->writeln($e->getMessage() . PHP_EOL);
// we must have an exit code higher than zero to indicate something was wrong
$returnValue = \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
}

return $this;
return $returnValue;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
$messages = array_merge($messages, $this->validate($themePaths));
if (!empty($messages)) {
$output->writeln($messages);
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
$messages = array_merge(
$messages,
Expand All @@ -209,7 +210,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
'<error>Unable to uninstall. Please resolve the following issues:</error>'
. PHP_EOL . implode(PHP_EOL, $messages)
);
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}

try {
Expand All @@ -230,6 +232,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
} catch (\Exception $e) {
$output->writeln('<error>' . $e->getMessage() . '</error>');
$output->writeln('<error>Please disable maintenance mode after you resolved above issues</error>');
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
}

Expand Down
4 changes: 3 additions & 1 deletion bin/magento
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use \Magento\Framework\App\ProductMetadata;
use \Magento\Framework\Composer\ComposerJsonFinder;
use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\Console\Cli;

if (PHP_SAPI !== 'cli') {
echo 'bin/magento must be run as a CLI application';
Expand All @@ -23,7 +24,7 @@ try {
$handler = new \Magento\Framework\App\ErrorHandler();
set_error_handler([$handler, 'handler']);
$productMetadata = new ProductMetadata(new ComposerJsonFinder(new DirectoryList(BP)));
$application = new Magento\Framework\Console\Cli('Magento CLI', $productMetadata->getVersion());
$application = new Cli('Magento CLI', $productMetadata->getVersion());
$application->run();
} catch (\Exception $e) {
while ($e) {
Expand All @@ -32,4 +33,5 @@ try {
echo "\n\n";
$e = $e->getPrevious();
}
exit(Cli::RETURN_FAILURE);
}
6 changes: 5 additions & 1 deletion lib/internal/Magento/Framework/Console/Cli.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,12 @@ class Cli extends SymfonyApplication
const INPUT_KEY_BOOTSTRAP = 'bootstrap';

/**
* @var \Zend\ServiceManager\ServiceManager
* Cli exit codes
*/
const RETURN_SUCCESS = 0;
const RETURN_FAILURE = 1;

/** @var \Zend\ServiceManager\ServiceManager */
private $serviceManager;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
'<error>Please check the path you provided. Dependencies report generator failed with error: ' .
$e->getMessage() . '</error>'
);
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
$messages = $this->validate($addresses);
if (!empty($messages)) {
$output->writeln('<error>' . implode('</error>' . PHP_EOL . '<error>', $messages));
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}

$this->maintenanceMode->set($this->isEnable());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
$messages = $this->validate($modules);
if (!empty($messages)) {
$output->writeln(implode(PHP_EOL, $messages));
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
try {
$modulesToChange = $this->getStatus()->getModulesToChange($isEnable, $modules);
} catch (\LogicException $e) {
$output->writeln('<error>' . $e->getMessage() . '</error>');
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
if (!empty($modulesToChange)) {
$force = $input->getOption(self::INPUT_KEY_FORCE);
Expand All @@ -93,7 +95,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
"<error>Unable to change status of modules because of the following constraints:</error>"
);
$output->writeln('<error>' . implode("</error>\n<error>", $constraints) . '</error>');
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
}
$this->setIsEnabled($isEnable, $modulesToChange, $output);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class AdminUserCreateCommand extends AbstractSetupCommand
* @var UserValidationRules
*/
private $validationRules;

/**
* @param InstallerFactory $installerFactory
* @param UserValidationRules $validationRules
Expand Down Expand Up @@ -58,7 +58,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
$errors = $this->validate($input);
if ($errors) {
$output->writeln('<error>' . implode('</error>' . PHP_EOL . '<error>', $errors) . '</error>');
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
$installer = $this->installerFactory->create(new ConsoleLogger($output));
$installer->installAdminUser($input->getOptions());
Expand Down
6 changes: 5 additions & 1 deletion setup/src/Magento/Setup/Console/Command/BackupCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,10 @@ protected function execute(InputInterface $input, OutputInterface $output)
if (!$this->deploymentConfig->isAvailable()
&& ($input->getOption(self::INPUT_KEY_MEDIA) || $input->getOption(self::INPUT_KEY_DB))) {
$output->writeln("<info>No information is available: the Magento application is not installed.</info>");
return;
// We need exit code higher than 0 here as an indication
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
$returnValue = \Magento\Framework\Console\Cli::RETURN_SUCCESS;
try {
$inputOptionProvided = false;
$output->writeln('<info>Enabling maintenance mode</info>');
Expand All @@ -143,10 +145,12 @@ protected function execute(InputInterface $input, OutputInterface $output)
}
} catch (\Exception $e) {
$output->writeln('<error>' . $e->getMessage() . '</error>');
$returnValue = \Magento\Framework\Console\Cli::RETURN_FAILURE;
} finally {
$output->writeln('<info>Disabling maintenance mode</info>');
$this->maintenanceMode->set(false);
}
return $returnValue;
}

/**
Expand Down
13 changes: 11 additions & 2 deletions setup/src/Magento/Setup/Console/Command/CronRunCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,15 @@ protected function configure()
protected function execute(InputInterface $input, OutputInterface $output)
{
if (!$this->checkRun()) {
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
try {
$this->status->toggleUpdateInProgress();
} catch (\RuntimeException $e) {
$this->status->add($e->getMessage());
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}

try {
Expand All @@ -106,6 +108,13 @@ protected function execute(InputInterface $input, OutputInterface $output)
} finally {
$this->status->toggleUpdateInProgress(false);
}

if ($this->status->isUpdateError()) {
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}

return \Magento\Framework\Console\Cli::RETURN_SUCCESS;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
{
if (!$this->deploymentConfig->isAvailable()) {
$output->writeln("<info>No information is available: the Magento application is not installed.</info>");
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
$installer = $this->installFactory->create(new ConsoleLogger($output));
$installer->installDataFixtures();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
{
if (!$this->deploymentConfig->isAvailable()) {
$output->writeln("<info>No information is available: the Magento application is not installed.</info>");
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
$installer = $this->installFactory->create(new ConsoleLogger($output));
$installer->installSchema();
Expand Down
2 changes: 2 additions & 0 deletions setup/src/Magento/Setup/Console/Command/DbStatusCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
'<info>Some modules use code versions newer or older than the database. ' .
"First update the module code, then run 'setup:upgrade'.</info>"
);
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
} else {
$output->writeln("<info>Run 'setup:upgrade' to update your DB schema and data.</info>");
}
Expand Down
5 changes: 4 additions & 1 deletion setup/src/Magento/Setup/Console/Command/DiCompileCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
foreach ($errors as $line) {
$output->writeln($line);
}
return;
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}

$modulePaths = $this->componentRegistrar->getPaths(ComponentRegistrar::MODULE);
Expand Down Expand Up @@ -199,6 +200,8 @@ function (OperationInterface $operation) use ($progressBar) {
$output->writeln('<info>Generated code and dependency injection configuration successfully.</info>');
} catch (OperationException $e) {
$output->writeln('<error>' . $e->getMessage() . '</error>');
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
if (!$input->getOption(self::SKIP_REINDEX_OPTION)) {
$fixtureModel->reindex($output);
}

$totalEndTime = microtime(true);
$totalResultTime = $totalEndTime - $totalStartTime;

$output->writeln('<info>Total execution time: ' . gmdate('H:i:s', $totalResultTime) . '</info>');
} catch (\Exception $e) {
$output->writeln('<error>' . $e->getMessage() . '</error>');
$output->writeln('<error>' . $e->getMessage() . '</error>');
// we must have an exit code higher than zero to indicate something was wrong
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
}
}
}
Loading