A commands logger is ambigious

[eiriksm]

Describe the bug
The command class uses the LoggerAwareTrait from the psr/log package (plus the interface DrushLoggerManager). This trait has the method setLogger which accepts a LoggerInterface instance. So far, this is very straight forward and just what I expect.

However, many commands you try to run will try to get the logger via the ::logger method. This method is required to either return a DrushLoggerManager or null.

This means that if you try to set a logger on a drush command class that is not a DrushLoggerManager (for example, one of the psr/log compatible loggers from monolog) you will get a type error (example taken from setting a monolog logger as the logger):

Drush\Commands\DrushCommands::logger(): Return value must be of type ?Drush\Log\DrushLoggerManager, Drupal\monolog\Logger\Logger returned

This is a bit confusing, even if understandable (for example, seems we need the ::success method). On the one hand, we are implementing the "contract" specified in DrushLoggerManager but for someone following this contract, it will not work.

To Reproduce

<?php

use Drush\Commands\core\QueueCommands;

$command = QueueCommands::create(\Drupal::getContainer());
$command->setLogger(\Drupal::logger('drush'));
$command->run('some_queue_you_have');

Expected behavior
It would run the queue, and log to my logger.

Actual behavior

TypeError: Drush\Commands\DrushCommands::logger(): Return value must be of type ?Drush\Log\DrushLoggerManager, Drupal\monolog\Logger\Logger returned in /var/www/html/vendor/drush/drush/src/Commands/DrushCommands.php on line 72 #0 /var/www/html/vendor/drush/drush/src/Commands/core/QueueCommands.php(101): Drush\Commands\DrushCommands->logger()
#1 /var/www/html/test.php(7): Drush\Commands\core\QueueCommands->run('some_queue_you_have')
#2 /var/www/html/vendor/drush/drush/src/Commands/core/PhpCommands.php(105): include('/var/www/html/t...')
#3 [internal function]: Drush\Commands\core\PhpCommands->script(Array, Array)
#4 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(276): call_user_func_array(Array, Array)
#5 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#6 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#7 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(391): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#8 /var/www/html/vendor/symfony/console/Command/Command.php(326): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#9 /var/www/html/vendor/symfony/console/Application.php(1096): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#10 /var/www/html/vendor/symfony/console/Application.php(324): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 /var/www/html/vendor/symfony/console/Application.php(175): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(110): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#13 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun(Array, Object(Symfony\Component\Console\Output\ConsoleOutput))
#14 /var/www/html/vendor/drush/drush/drush.php(139): Drush\Runtime\Runtime->run(Array)
#15 /var/www/html/vendor/drush/drush/drush(4): require('/var/www/html/v...')
#16 /var/www/html/vendor/bin/drush(119): include('/var/www/html/v...')
#17 {main}

Workaround
I guess I could somehow extend DrushLoggerManager for the purpose of logging the output. Or run drush within a subprocess and fetch the output.

System Configuration

Q	A
Drush version?	11.x/12.x (but specifically the error comes from 12.4.3.0)
Drupal version?	10.2.1
PHP version	8.1.26
OS?	Linux

Additional information
Not sure if this is fixable in a good way. But maybe we should not use the ::setLogger method from the psr/log package to avoid the confusion?

[obriat]

I didn't dive into the code, but I just found the method name "logger" confusing as I expect that it should at least send the message to the loggerchanel so I can find it into my defined log target (file, dblog, syslog, ...).

[weitzman]

See this PR and especially these lines https://github.com/drush-ops/drush/pull/5022/files#diff-99d5f4f635c30767edca2077f00a804bec2c283565da05f4a864e9d45295d690R37-R54

I dont think this is documented anywhere yet.

[paul-m]

I think the most basic level of documentation would be to change the signature of LoggerAwareTrait so that it only takes DrushLoggerManager and never LoggerInterface, because it kinda works, and kinda doesn't as-is, leading to broken code.