Skip to content

Fix Wrong Exception on Cli run when Command Not found #40063

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

Open
wants to merge 1 commit into
base: 2.4-develop
Choose a base branch
from
Open
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
39 changes: 36 additions & 3 deletions lib/internal/Magento/Framework/Console/Cli.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ class Cli extends Console\Application
*/
private $initException;

/**
* GetCommands exception.
*
Comment on lines +66 to +67
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

The docblock comment 'GetCommands exception' should be more descriptive. Consider: 'Exception thrown during command list initialization.'

Suggested change
* GetCommands exception.
*
* Exception thrown during command list initialization.
*

Copilot uses AI. Check for mistakes.

* @var \Exception
*/
private $getCommandsException;

/**
* @var ObjectManagerInterface
*/
Expand Down Expand Up @@ -111,7 +118,7 @@ public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN')
$this->setCommandLoader($this->getCommandLoader());
}

/**
/**
* @inheritdoc
*
* @throws \Exception The exception in case of unexpected error
Expand All @@ -123,8 +130,28 @@ public function doRun(Console\Input\InputInterface $input, Console\Output\Output
$exitCode = parent::doRun($input, $output);
} catch (\Exception $e) {
$errorMessage = $e->getMessage() . PHP_EOL . $e->getTraceAsString();
$this->logger->error($errorMessage);
$this->initException = $e;

if ($this->getCommandsException) {
// No need to concatenate the exception message because it's already included in the $e->previous
// Example of the final exception :
// Exception during console commands initialization: Class "Vendor\Migration\Console\Command\ImportOrdersCommand" does not exist
// code: 0
// file: "./vendor/magento/framework/Console/Cli.php"
// line: 131
// -previous: Symfony\Component\Console\Exception\NamespaceNotFoundException^ {#3199
// #message: """
// There are no commands defined in the "c" namespace.\n
// \n
// Did you mean one of these?\n

$combinedErrorMessage = "Exception during console commands initialization: " .
$this->getCommandsException->getMessage() . PHP_EOL;
Comment on lines +147 to +148
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

[nitpick] The string concatenation spans multiple lines unnecessarily. Consider combining into a single line or using sprintf for better readability: sprintf('Exception during console commands initialization: %s%s', $this->getCommandsException->getMessage(), PHP_EOL)

Suggested change
$combinedErrorMessage = "Exception during console commands initialization: " .
$this->getCommandsException->getMessage() . PHP_EOL;
$combinedErrorMessage = sprintf(
'Exception during console commands initialization: %s%s',
$this->getCommandsException->getMessage(),
PHP_EOL
);

Copilot uses AI. Check for mistakes.

$this->initException = new \Exception($combinedErrorMessage, $e->getCode(), $e);
$this->logger->error($combinedErrorMessage);
} else {
$this->initException = $e;
$this->logger->error($errorMessage);
}
}

if ($this->initException) {
Expand All @@ -151,6 +178,11 @@ protected function getApplicationCommands()
{
$commands = [];
try {
if (class_exists(\Magento\Setup\Console\CommandList::class)) {
$setupCommandList = new \Magento\Setup\Console\CommandList($this->serviceManager);
$commands = array_merge($commands, $setupCommandList->getCommands());
}
Comment on lines +181 to +184
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please explain, why this change is needed?

Copy link
Contributor

@ihor-sviziev ihor-sviziev Jul 17, 2025

Choose a reason for hiding this comment

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

From reply in #40063 (comment):

We don't need these lines; it is probably due to a merge issue. You can
follow the logic. These lines don't change logic, just were touched by the
commit

These lines were added, so let's remove them if we don't need them


if ($this->objectManager->get(DeploymentConfig::class)->isAvailable()) {
Comment on lines +181 to 186
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

The setup command initialization has been moved outside the deployment config check, which changes the execution order. This should be documented or the reasoning should be clearer, as it may affect command availability in different deployment states.

Suggested change
if (class_exists(\Magento\Setup\Console\CommandList::class)) {
$setupCommandList = new \Magento\Setup\Console\CommandList($this->serviceManager);
$commands = array_merge($commands, $setupCommandList->getCommands());
}
if ($this->objectManager->get(DeploymentConfig::class)->isAvailable()) {
if ($this->objectManager->get(DeploymentConfig::class)->isAvailable()) {
if (class_exists(\Magento\Setup\Console\CommandList::class)) {
$setupCommandList = new \Magento\Setup\Console\CommandList($this->serviceManager);
$commands = array_merge($commands, $setupCommandList->getCommands());
}

Copilot uses AI. Check for mistakes.

/** @var CommandListInterface */
$commandList = $this->objectManager->create(CommandListInterface::class);
Expand All @@ -162,6 +194,7 @@ protected function getApplicationCommands()
$this->getVendorCommands($this->objectManager)
);
} catch (\Exception $e) {
$this->getCommandsException = $e;
$this->initException = $e;
}

Expand Down