We currently print nice, plain error messages for AJAX requests but HTML laden ones for CLI requests. Attached patch makes CLI errors print like AJAX except for an additional strip_tags which cleans them up even more. The strip_tags is not desireable for AJAX so we make own conditional for CLI.

CommentFileSizeAuthor
#10 cli.diff747 bytesmoshe weitzman
#4 cli.diff737 bytesmoshe weitzman
#3 cli.diff737 bytesmoshe weitzman
cli.diff711 bytesmoshe weitzman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

#564242: Non-interactive install prints HTML exception messages into error log was marked a duplicate of this bug.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Needs review » Needs work

I was wrong in an earlier IRC discussion with Moshe. We actually want to use the ! placeholder here (as in '!type: !message in !function (line !line of !file)'), because we want to output the result in plain-text to CLI.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs work » Needs review
FileSize
cli.diff737 bytes

Right. Since $error has @ in it already, whats needed is an html_entity_decode before strip_tags. Patch attached.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
FileSize
cli.diff737 bytes

swapped entity_decode and strip_tags for correctness.

Status: Needs review » Needs work

The last submitted patch, cli.diff, failed testing.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs work » Needs review

#4: cli.diff queued for re-testing.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

All tests have passed.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Hm. There's getting to be quite a lot of special-casing in this function. :\ I can see why this is needed, though. Committed to HEAD.

I'm curious, while your nose is in this part of the code, do you have a suggestion for fixing #642160: Make debug() message work better (usable), which is closely related?

Status: Fixed » Closed (fixed)

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

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Priority: Normal » Critical
Status: Closed (fixed) » Reviewed & tested by the community
FileSize
cli.diff747 bytes

This issue managed to get committed and then undone by #325169: Move error/exception handler higher up in the bootstrap process. Here is a re-roll.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

#10: cli.diff queued for re-testing.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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