Skip to content

fix(@angular/cli): strip terminal escape sequences in modern yarn error parser - #34614

Open
clydin wants to merge 2 commits into
angular:mainfrom
clydin:fix/yarn-modern-error-followup
Open

clydin wants to merge 2 commits into
angular:mainfrom
clydin:fix/yarn-modern-error-followup

Conversation

@clydin

@clydin clydin commented Oct 30, 2026

Copy link
Copy Markdown
Member

Modern Yarn formats error detail fields such as Response Code with ANSI color sequences and OSC 8 hyperlinks before passing them to StreamReport, which serializes the formatted strings verbatim into its JSON line output. When colors are enabled, such as by default on GitHub Actions even over non-TTY pipes or via FORCE_COLOR and .yarnrc.yml settings, escape codes appear around both the Response Code label and status number. As a result, parseYarnModernError failed to match the HTTP status code, fell back to the generic YN0035 code instead of E404 for missing packages, and leaked raw control sequences into the parsed summary and detail text.

Strip VT control characters from modern Yarn error messages prior to matching an anchored Response Code line, and ignore 2xx status codes for consistency with the Yarn Classic parser. In addition, split multi-line error payloads such as YN0001 exception stack traces so summary only retains the first line while remaining lines move into detail, fall back to UNKNOWN_ERROR when displayName is an empty string (when enableMessageNames is disabled), and return null directly when no JSON error lines are present instead of delegating to parseNpmLikeError on every successful command output.

…or parser

Modern Yarn formats error detail fields such as `Response Code` with ANSI
color sequences and OSC 8 hyperlinks before passing them to `StreamReport`,
which serializes the formatted strings verbatim into its JSON line output.
When colors are enabled, such as by default on GitHub Actions even over
non-TTY pipes or via `FORCE_COLOR` and `.yarnrc.yml` settings, escape
codes appear around both the `Response Code` label and status number. As a
result, `parseYarnModernError` failed to match the HTTP status code, fell
back to the generic `YN0035` code instead of `E404` for missing packages,
and leaked raw control sequences into the parsed summary and detail text.

Strip VT control characters from modern Yarn error messages prior to
matching an anchored `Response Code` line, and ignore 2xx status codes for
consistency with the Yarn Classic parser. In addition, split multi-line
error payloads such as `YN0001` exception stack traces so `summary` only
retains the first line while remaining lines move into `detail`, fall back
to `UNKNOWN_ERROR` when `displayName` is an empty string (when
`enableMessageNames` is disabled), and return `null` directly when no JSON
error lines are present instead of delegating to `parseNpmLikeError` on
every successful command output.
@clydin
clydin marked this pull request as ready for review October 30, 2026 19:30
@clydin clydin added the target: patch This PR is targeted for the next patch release label Oct 30, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the modern Yarn error parser to strip ANSI color codes and OSC 8 hyperlinks from error lines, improves HTTP status code extraction, and refines how error summaries and details are split. It also adds comprehensive unit tests to verify these changes. The review feedback suggests a robust fallback for the error summary to prevent it from being empty when the error data contains only whitespace.

Comment on lines +704 to +707
const [summary = '', ...firstDetailLines] = first.data
.split('\n')
.map((line) => line.trim())
.filter(Boolean);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If first.data is empty or contains only whitespace, the destructured summary will default to an empty string (''). To ensure a robust fallback and prevent empty error summaries, we can default summary to first.code (e.g., 'YN0035' or 'UNKNOWN_ERROR'), which is guaranteed to be a non-empty string.

  const [summary = first.code, ...firstDetailLines] = first.data\n    .split('\\n')\n    .map((line) => line.trim())\n    .filter(Boolean);

@clydin clydin added the action: review The PR is still awaiting reviews from at least one requested reviewer label Oct 30, 2026
@clydin
clydin requested a review from alan-agius4 October 30, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer area: @angular/cli target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant