Skip to content

fix(console): write app command loading errors to stderr - #62718

Open
printminion-co wants to merge 1 commit into
nextcloud:masterfrom
IONOS-Productivity:mk/fix/occ-console-command-load-error-to-stderr
Open

fix(console): write app command loading errors to stderr#62718
printminion-co wants to merge 1 commit into
nextcloud:masterfrom
IONOS-Productivity:mk/fix/occ-console-command-load-error-to-stderr

Conversation

@printminion-co

@printminion-co printminion-co commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

When an app fails to load its console commands from info.xml, OC\Console\Application::loadCommands() reports the failure with $output->writeln() — i.e. stdout — while every other diagnostic in the same method already uses $output->getErrorOutput() (insufficient memory limit, "Nextcloud is not installed", "requires upgrade", maintenance-mode notices).

The command then runs normally and exits 0, so the message silently corrupts machine-readable output:

$ ./occ app:list --output=json
Connection refused
{"enabled":{"cloud_federation_api":"1.17.0", ...},"disabled":{...}}
$ echo $?
0

Anything piping occ <cmd> --output=json into a JSON parser breaks, and there is no non-zero exit code to detect it by. --no-warnings is not a workaround, since it sets VERBOSITY_QUIET and suppresses the payload too.

$output is typed ConsoleOutputInterface, so getErrorOutput() is guaranteed by the signature. Two call sites are affected: the app_api maintenance-mode block and the regular installed-apps loop. The logger->error() calls are unchanged, so the failure is still logged with its stack trace.

How this was hit

An app with the phpredis extension loaded but no Redis configured. RedisFactory::isAvailable() only checks extension_loaded('redis'), not whether a redis config block exists, so the app's queue factory took the Redis branch, fell back to 127.0.0.1:6379 and threw RedisException: Connection refused while its console commands were being constructed. Because enable_lazy_objects defaults to true, the constructor runs inside $this->application->add($c)Command::setApplication(), so the throw surfaces outside loadCommandsFromInfoXml()'s inner catch (ContainerExceptionInterface) and lands in the outer catch (\Throwable).

The app-side behaviour is arguably its own issue; the console bootstrap should not corrupt stdout regardless of which app fails to load.

Verification

Reproduced deterministically with a throwaway app whose info.xml names a nonexistent command class:

stdout jq stderr exit
before Console command '...' is un… parse error (5) empty 0
after {"enabled":{...} ok, 21 keys the error message 0

The error remains in nextcloud.log.

On tests

No test is included. There is currently no test for lib/private/Console/*, and grep -rn getErrorOutput tests/ returns nothing, so there is no existing harness or stdout-vs-stderr assertion precedent to extend. loadCommands() also does an unstubbable require_once core/register_command.php (~138 Server::get() calls, re-fetching the real IConfig rather than an injected mock), which makes a genuine unit test a DB-group test whose result is order-dependent because require_once runs once per process.

This change adds no logic — it only redirects an existing message from one stream to the other. Happy to add a build/integration scenario (CommandLine.php already captures stdout and stderr separately) or to extract that require_once behind an injected collaborator to make the class unit-testable, if a maintainer would prefer either.

Checklist

  • Code is properly formatted
  • Signed-off-by is present (added by the contributor)
  • Tests — see "On tests" above
  • Documentation has been updated or is not required

AI disclosure

This change and this description were drafted with AI assistance (Claude Code, claude-opus-5), then reviewed by the submitting contributor. Disclosed per the Nextcloud AI Contribution Policy.

@printminion-co
printminion-co requested a review from a team as a code owner July 31, 2026 08:10
@printminion-co
printminion-co requested review from Altahrim, come-nc, provokateurin and salmart-dev and removed request for a team July 31, 2026 08:10
@printminion-co
printminion-co force-pushed the mk/fix/occ-console-command-load-error-to-stderr branch from 1b5de99 to 281a0cc Compare July 31, 2026 08:14
When an app fails to load its commands from info.xml, the error was
written to stdout, while every other diagnostic in loadCommands() uses
$output->getErrorOutput(). The command itself then runs normally and
exits 0, so the message silently corrupts machine-readable output:

    $ ./occ app:list --output=json
    Connection refused
    {"enabled":{...},"disabled":{...}}
    $ echo $?
    0

Anything piping `occ <cmd> --output=json` into a JSON parser breaks, with
no non-zero exit code to detect it by.

Observed with notify_push on a setup that has the phpredis extension
loaded but no Redis configured: RedisFactory::isAvailable() only checks
whether the extension is loaded, so constructing the app's console
commands ends up calling pconnect() and throws RedisException.

--no-warnings is not a workaround for this, as it sets VERBOSITY_QUIET
and suppresses the payload too.

Route the message to the error output instead. It is still reported via
logger->error() exactly as before.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>

@artonge artonge left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Makes sense, cc @icewind1991 and @bigcat88 as original author of those lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants