broken commands kill occ

[butonic]

If an app ships with an incompatible command the occ command will be broken and unusable, eg:

An unhandled exception has been thrown:
ArgumentCountError: Too few arguments to function OCA\User_LDAP\Command\Search::__construct(), 0 passed in /var/www/owncloud/lib/private/Console/Application.php on line 170 and exactly 3 expected in /var/www/owncloud/apps/user_ldap/lib/Command/Search.php:50
Stack trace:
#0 /var/www/owncloud/lib/private/Console/Application.php(170): OCA\User_LDAP\Command\Search->__construct()
#1 /var/www/owncloud/lib/private/Console/Application.php(108): OC\Console\Application->loadCommandsFromInfoXml(Array)
#2 /var/www/owncloud/console.php(105): OC\Console\Application->loadCommands(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#3 /var/www/owncloud/occ(11): require_once('/var/www/ownclo...')
#4 {main}

core/lib/private/Console/Application.php

Lines 164 to 178 in 3bff5c2

	private function loadCommandsFromInfoXml($commands) {
	foreach ($commands as $command) {
	try {
	$c = \OC::$server->query($command);
	} catch (QueryException $e) {
	if (\class_exists($command)) {
	$c = new $command();
	} else {
	throw new \Exception("Console command '$command' is unknown and could not be loaded");
	}
	}
	$this->application->add($c);
	}
	}

• add ILogger to DI,
• log an error
• skip to the next command
• add tests ... currently not covered AFAICT

[ownclouders]

GitMate.io thinks possibly related issues are #13402 (... broken), #24494 (occ command to clear all locks), #13976 (Kill OC_Request), #25964 (occ-.htaccess generation breaks .htaccess), and #30177 (Kill occ command maintenance:createjs).

[Kanishksgithub]

i want to work on this could you briefly define exactly what i have to do in this issue

[DeepDiver1975]

i want to work on this could you briefly define exactly what i have to do in this issue

instead of throwing the exception write a log statement - this way the loop will finish and all other commands are available ....

[Kanishksgithub]

could you assign me i want to work on this issue

[Kanishksgithub]

i want to fix this bug

[DeepDiver1975]

could you assign me i want to work on this issue

Just go for it an submit your pull request. THX a lot! 🙏

[DeepDiver1975]

Classification: confirmed-bug (still reproducible on master)

🤖 Automated issue analysis by Claude Code review agent.

This is still present and unfixed on current master. The offending code in lib/private/Console/Application.php is essentially unchanged since 2016:

private function loadCommandsFromInfoXml($commands) {
    foreach ($commands as $command) {
        try {
            $c = \OC::$server->query($command);
        } catch (QueryException $e) {
            if (\class_exists($command)) {
                $c = new $command();          // <-- can throw \Error
            } else {
                throw new \Exception("Console command '$command' is unknown and could not be loaded");
            }
        }
        $this->application->addCommand($c);
    }
}

Root cause. When a command class declares constructor dependencies that the DI container cannot resolve, \OC::$server->query($command) throws QueryException, which is caught. The fallback then calls new $command() with no arguments. For a command whose constructor has required parameters (e.g. OCA\User_LDAP\Command\Search::__construct() expecting 3 args, as in the report) this raises an ArgumentCountError (a \Error/\Throwable, not a QueryException or \Exception). That error is not caught here, propagates up through loadCommands(), and aborts the entire occ bootstrap — so a single broken app command makes all commands unusable.

Two distinct problems remain in this method:

1. new $command() is unguarded against \Error/\Throwable thrown by the constructor (the actual crash in the report).
2. The else branch throw new \Exception(...) also aborts the whole loop rather than skipping the one bad command.

The maintainer's earlier guidance still applies: instead of throwing/letting it bubble, log an error and continue to the next command so the rest of occ stays available. The original checklist is still open:

• add ILogger to DI (the constructor currently takes only IConfig, EventDispatcherInterface, IRequest — no logger is injected)
• log an error
• skip to the next command
• add tests (this path is still uncovered)

Suggested shape of the fix: wrap the per-command load (query + new $command() fallback + addCommand) in a try { ... } catch (\Throwable $e) { $this->logger->logException($e, ['app' => 'core']); continue; } so one incompatible command is logged and skipped instead of killing occ.

This is a junior-friendly, well-scoped fix. No PR is being opened automatically.

[DeepDiver1975]

🤖 Follow-up: opened a fix — #41642. It wraps each command load in try/catch (\Throwable) so a broken command is logged via ILogger and skipped instead of killing occ; the existing query/new $command()/unknown-class path and the success behavior are unchanged. Includes a regression test.