Problem/Motivation

Basically, MarkdownCommands causes this fatal error with Drush 13:

ambientimpact:~/.../Omnipedia/Web$ ddev drush status
AssertionError: Instead of using replacing Drush's logger, use $this->add() on DrushLoggerManager to add a custom logger. See https://github.com/drush-ops/drush/pull/5022 in /var/www/html/vendor/drush/drush/src/Commands/DrushCommands.php on line 76 #0 /var/www/html/vendor/drush/drush/src/Commands/DrushCommands.php(76): assert()
#1 /var/www/html/vendor/drush/drush/src/Runtime/ServiceManager.php(480): Drush\Commands\DrushCommands->logger()
#2 /var/www/html/vendor/drush/drush/src/Boot/DrupalBoot8.php(282): Drush\Runtime\ServiceManager->inflect()
#3 /var/www/html/vendor/drush/drush/src/Boot/DrupalBoot8.php(218): Drush\Boot\DrupalBoot8->addDrupalModuleDrushCommands()
#4 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(211): Drush\Boot\DrupalBoot8->bootstrapDrupalFull()
#5 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(397): Drush\Boot\BootstrapManager->doBootstrap()
#6 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(332): Drush\Boot\BootstrapManager->bootstrapMax()
#7 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(304): Drush\Boot\BootstrapManager->bootstrapToPhaseIndex()
#8 /var/www/html/vendor/drush/drush/src/Boot/BootstrapHook.php(36): Drush\Boot\BootstrapManager->bootstrapToPhase()
#9 /var/www/html/vendor/consolidation/annotated-command/src/Hooks/Dispatchers/InitializeHookDispatcher.php(44): Drush\Boot\BootstrapHook->initialize()
#10 /var/www/html/vendor/consolidation/annotated-command/src/Hooks/Dispatchers/InitializeHookDispatcher.php(36): Consolidation\AnnotatedCommand\Hooks\Dispatchers\InitializeHookDispatcher->doInitializeHook()
#11 /var/www/html/vendor/consolidation/annotated-command/src/Hooks/Dispatchers/InitializeHookDispatcher.php(29): Consolidation\AnnotatedCommand\Hooks\Dispatchers\InitializeHookDispatcher->callInitializeHook()
#12 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(145): Consolidation\AnnotatedCommand\Hooks\Dispatchers\InitializeHookDispatcher->initialize()
#13 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(376): Consolidation\AnnotatedCommand\CommandProcessor->initializeHook()
#14 /var/www/html/vendor/symfony/console/Command/Command.php(292): Consolidation\AnnotatedCommand\AnnotatedCommand->initialize()
#15 /var/www/html/vendor/symfony/console/Application.php(1096): Symfony\Component\Console\Command\Command->run()
#16 /var/www/html/vendor/symfony/console/Application.php(324): Symfony\Component\Console\Application->doRunCommand()
#17 /var/www/html/vendor/symfony/console/Application.php(175): Symfony\Component\Console\Application->doRun()
#18 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(110): Symfony\Component\Console\Application->run()
#19 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun()
#20 /var/www/html/vendor/drush/drush/drush.php(140): Drush\Runtime\Runtime->run()
#21 /var/www/html/vendor/bin/drush.php(119): include('...')
#22 {main}
AssertionError: Instead of using replacing Drush's logger, use $this->add() on DrushLoggerManager to add a custom logger. See https://github.com/drush-ops/drush/pull/5022 in assert() (line 76 of /var/www/html/vendor/drush/drush/src/Commands/DrushCommands.php).
 [warning] Drush command terminated abnormally.
Failed to run drush status: exit status 1

Steps to reproduce

Run drush status or any other Drush command; get above error.

Proposed resolution

It took a while to locate exactly what command was triggering the error until I found that MarkdownCommands does indeed override the logger in its constructor: $this->logger = $logger; - given that the entire class was apparently deprecated in 8.x-2.0 and was intended to be removed in 3.0.0 as part of #3103679: [PP-1][markdown] Drupal 9 Support, we might be able to remove it, but it looks like it's tied to some Composer stuff and I'm reluctant to pull on that thread because this would make more sense in a single issue to remove all deprecated that was supposed to be removed in 3.0.0. For now, we should wrap the assignment in a check for the logger() existing on $this and not assign it if it's present:

// Don't replace Drush's logger because it'll cause a fatal error on Drush
// 13 and newer.
//
// @see https://github.com/drush-ops/drush/pull/5022
if (!method_exists($this, 'logger')) {
  $this->logger = $logger;
}

Remaining tasks

Do the thing.

User interface changes

None.

API changes

Drush command removed.

Data model changes

None?

CommentFileSizeAuthor
#14 3483437-markdown--fix-drush-13.diff975 bytesrobloach

Issue fork markdown-3483437

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

ambient.impact created an issue. See original summary.

ambient.impact’s picture

Issue summary: View changes
ambient.impact’s picture

ambient.impact’s picture

Status: Active » Needs review

jroblero changed the visibility of the branch 3483437-drush-13-fatal-error to hidden.

mayeulk’s picture

Hi. Apparently I'm facing this bug with:
$ drush st
Drupal version : 10.3.7
DB driver : pgsql
PHP OS : Linux (Docker container)
PHP version : 8.3.11
Drush version : 13.3.2.0

I had to uninstall markdown module (which was not working anyway), through the admin GUI then with :
composer remove drupal/markdown
composer remove league/commonmark:^1
drush cache:rebuild

Is there a way that I can get both drush and markdown working?

malcomio’s picture

Status: Needs review » Active

There isn't a merge request or a patch, so the status shouldn't be Needs review.

The fork does have some change in it.

malcomio’s picture

Status: Active » Needs review

Have created MR 35

malcomio’s picture

Status: Needs review » Reviewed & tested by the community

This change seems to fix the problem as far as I can see.

I've seen different approaches in some other modules though:

https://git.drupalcode.org/project/digital_signage_framework/-/commit/c4...
https://git.drupalcode.org/project/do_username/-/merge_requests/13/diffs

I guess that the good thing about this approach is that it is backwards compatible

aaronmchale’s picture

Can confirm after applying patch from MR, also fixes issue for me.

moshe weitzman’s picture

I dont see the benefit of using a custom logger here. I suggest removing the logger related code from the constructor and create().

orkutmuratyilmaz’s picture

any chance for the merge?

robloach’s picture

Title: Fatal error with Drush 13 due to MarkdownCommands replacing logger » Fix fatal error with Drush 13 due to MarkdownCommands replacing logger
StatusFileSize
new975 bytes

Confirming. Here's the patch from @malcomio's MR for those needing it.

joelpittet made their first commit to this issue’s fork.

joelpittet’s picture

Status: Reviewed & tested by the community » Fixed

Thank you all I have merged this into 3.0.x-dev

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • joelpittet committed 8b35271a on 3.0.x authored by malcomio
    fix: #3483437 Fix fatal error with Drush 13 due to MarkdownCommands...
orkutmuratyilmaz’s picture

thanks for the fix:)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.