Skip to content

Commit 4c53411

Browse files
committed
refactor(@angular/cli): implement structured error handling in package manager API
This commit introduces a structured error handling system into the package manager abstraction. Previously, the system treated most command failures generically, often assuming a package was simply "not found." This could mask the true root cause of issues such as network failures, registry authentication errors, or unexpected changes in a package manager's output. The new implementation enhances diagnostics and provides clearer, more actionable error messages by: - Introducing a standardized `ErrorInfo` interface to represent parsed error details. - Adding dedicated parsers to extract structured error information (JSON or text-based) from the stderr of npm, yarn, pnpm, and bun. - Making the core execution logic stricter. It now re-throws the original `PackageManagerError` if a command fails for an unknown reason, preventing silent failures. - Abstracting the "package not found" check into the `PackageManagerDescriptor`, removing hardcoded logic from the execution layer. - Improving the `yarn-classic` parser to use verbose logs, allowing it to accurately detect and report specific HTTP status codes (e.g., 401, 403, 404, 500).
1 parent 816b3d2 commit 4c53411

File tree

5 files changed

+348
-10
lines changed

5 files changed

+348
-10
lines changed

packages/angular/cli/src/package-managers/error.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,18 @@ export class PackageManagerError extends Error {
3535
super(message);
3636
}
3737
}
38+
39+
/**
40+
* Represents structured information about an error returned by a package manager command.
41+
* This is a data interface, not an `Error` subclass.
42+
*/
43+
export interface ErrorInfo {
44+
/** A specific error code (e.g. 'E404', 'EACCES'). */
45+
readonly code: string;
46+
47+
/** A short, human-readable summary of the error. */
48+
readonly summary: string;
49+
50+
/** An optional, detailed description of the error. */
51+
readonly detail?: string;
52+
}

packages/angular/cli/src/package-managers/package-manager-descriptor.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,17 @@
1212
* package-manager-specific commands, flags, and output parsing.
1313
*/
1414

15+
import { ErrorInfo } from './error';
1516
import { Logger } from './logger';
1617
import { PackageManifest, PackageMetadata } from './package-metadata';
1718
import { InstalledPackage } from './package-tree';
1819
import {
1920
parseNpmLikeDependencies,
21+
parseNpmLikeError,
2022
parseNpmLikeManifest,
2123
parseNpmLikeMetadata,
2224
parseYarnClassicDependencies,
25+
parseYarnClassicError,
2326
parseYarnLegacyManifest,
2427
parseYarnModernDependencies,
2528
} from './parsers';
@@ -86,12 +89,30 @@ export interface PackageManagerDescriptor {
8689

8790
/** A function to parse the output of `getManifestCommand` for the full package metadata. */
8891
getRegistryMetadata: (stdout: string, logger?: Logger) => PackageMetadata | null;
92+
93+
/** A function to parse the output of stderr when a command fails. */
94+
getError?: (stderr: string, logger?: Logger) => ErrorInfo | null;
8995
};
96+
97+
/** A function that checks if a structured error represents a "package not found" error. */
98+
readonly isNotFound: (error: ErrorInfo) => boolean;
9099
}
91100

92101
/** A type that represents the name of a supported package manager. */
93102
export type PackageManagerName = keyof typeof SUPPORTED_PACKAGE_MANAGERS;
94103

104+
/** A set of error codes that are known to indicate a "package not found" error. */
105+
const NOT_FOUND_ERROR_CODES = new Set(['E404']);
106+
107+
/**
108+
* A shared function to check if a structured error represents a "package not found" error.
109+
* @param error The structured error to check.
110+
* @returns True if the error code is a known "not found" code, false otherwise.
111+
*/
112+
function isKnownNotFound(error: ErrorInfo): boolean {
113+
return NOT_FOUND_ERROR_CODES.has(error.code);
114+
}
115+
95116
/**
96117
* A map of supported package managers to their descriptors.
97118
* This is the single source of truth for all package-manager-specific
@@ -124,7 +145,9 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
124145
listDependencies: parseNpmLikeDependencies,
125146
getRegistryManifest: parseNpmLikeManifest,
126147
getRegistryMetadata: parseNpmLikeMetadata,
148+
getError: parseNpmLikeError,
127149
},
150+
isNotFound: isKnownNotFound,
128151
},
129152
yarn: {
130153
binary: 'yarn',
@@ -146,7 +169,9 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
146169
listDependencies: parseYarnModernDependencies,
147170
getRegistryManifest: parseNpmLikeManifest,
148171
getRegistryMetadata: parseNpmLikeMetadata,
172+
getError: parseNpmLikeError,
149173
},
174+
isNotFound: isKnownNotFound,
150175
},
151176
'yarn-classic': {
152177
binary: 'yarn',
@@ -165,12 +190,14 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
165190
getRegistryOptions: (registry: string) => ({ args: ['--registry', registry] }),
166191
versionCommand: ['--version'],
167192
listDependenciesCommand: ['list', '--depth=0', '--json'],
168-
getManifestCommand: ['info', '--json'],
193+
getManifestCommand: ['info', '--json', '--verbose'],
169194
outputParsers: {
170195
listDependencies: parseYarnClassicDependencies,
171196
getRegistryManifest: parseYarnLegacyManifest,
172197
getRegistryMetadata: parseNpmLikeMetadata,
198+
getError: parseYarnClassicError,
173199
},
200+
isNotFound: isKnownNotFound,
174201
},
175202
pnpm: {
176203
binary: 'pnpm',
@@ -192,7 +219,9 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
192219
listDependencies: parseNpmLikeDependencies,
193220
getRegistryManifest: parseNpmLikeManifest,
194221
getRegistryMetadata: parseNpmLikeMetadata,
222+
getError: parseNpmLikeError,
195223
},
224+
isNotFound: isKnownNotFound,
196225
},
197226
bun: {
198227
binary: 'bun',
@@ -214,7 +243,9 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
214243
listDependencies: parseNpmLikeDependencies,
215244
getRegistryManifest: parseNpmLikeManifest,
216245
getRegistryMetadata: parseNpmLikeMetadata,
246+
getError: parseNpmLikeError,
217247
},
248+
isNotFound: isKnownNotFound,
218249
},
219250
} satisfies Record<string, PackageManagerDescriptor>;
220251

packages/angular/cli/src/package-managers/package-manager.ts

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import { join } from 'node:path';
1616
import npa from 'npm-package-arg';
17-
import { PackageManagerError } from './error';
17+
import { ErrorInfo, PackageManagerError } from './error';
1818
import { Host } from './host';
1919
import { Logger } from './logger';
2020
import { PackageManagerDescriptor } from './package-manager-descriptor';
@@ -191,17 +191,45 @@ export class PackageManager {
191191

192192
let stdout;
193193
let stderr;
194+
let exitCode: number | null = null;
195+
let parsedError: ErrorInfo | null = null;
196+
194197
try {
195198
({ stdout, stderr } = await this.#run(args, runOptions));
196199
} catch (e) {
197-
if (e instanceof PackageManagerError && typeof e.exitCode === 'number' && e.exitCode !== 0) {
198-
// Some package managers exit with a non-zero code when the package is not found.
199-
if (cache && cacheKey) {
200-
cache.set(cacheKey, null);
201-
}
200+
if (!(e instanceof PackageManagerError && typeof e.exitCode === 'number')) {
201+
throw e;
202+
}
203+
204+
exitCode = e.exitCode;
205+
stdout = e.stdout;
206+
stderr = e.stderr;
207+
208+
// Only try to parse errors if exit code is non-zero.
209+
if (exitCode !== 0) {
210+
parsedError = this.descriptor.outputParsers.getError?.(stderr, this.options.logger) ?? null;
211+
}
202212

203-
return null;
213+
if (parsedError) {
214+
this.options.logger?.debug(
215+
`[${this.descriptor.binary}] Structured error (code: ${parsedError.code}): ${parsedError.summary}`,
216+
);
217+
218+
// Special case for 'not found' errors (e.g., E404). Return null for these.
219+
if (this.descriptor.isNotFound(parsedError)) {
220+
if (cache && cacheKey) {
221+
cache.set(cacheKey, null);
222+
}
223+
224+
return null;
225+
} else {
226+
// For all other structured errors, re-throw a more informative error.
227+
throw new PackageManagerError(parsedError.summary, stdout, stderr, exitCode);
228+
}
204229
}
230+
231+
// If no structured error was parsed, rethrow the original PackageManagerError
232+
// (this also covers cases where exitCode is 0 but runCommand still threw for other reasons).
205233
throw e;
206234
}
207235

@@ -216,7 +244,7 @@ export class PackageManager {
216244
const message = `Failed to parse package manager output: ${
217245
e instanceof Error ? e.message : ''
218246
}`;
219-
throw new PackageManagerError(message, stdout, stderr, 0);
247+
throw new PackageManagerError(message, stdout, stderr, exitCode);
220248
}
221249
}
222250

packages/angular/cli/src/package-managers/parsers.ts

Lines changed: 153 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* into their own file improves modularity and allows for focused testing.
1313
*/
1414

15+
import { ErrorInfo } from './error';
1516
import { Logger } from './logger';
1617
import { PackageManifest, PackageMetadata } from './package-metadata';
1718
import { InstalledPackage } from './package-tree';
@@ -106,7 +107,7 @@ export function parseNpmLikeDependencies(
106107
* Parses the output of `yarn list` (classic).
107108
*
108109
* The expected output is a JSON stream (JSONL), where each line is a JSON object.
109-
* The relevant object has a `type` of `'tree'`.
110+
* The relevant object has a `type` of `'tree'` with a `data` property.
110111
* Yarn classic does not provide a path, so the `path` property will be `undefined`.
111112
*
112113
* ```json
@@ -289,3 +290,154 @@ export function parseYarnLegacyManifest(stdout: string, logger?: Logger): Packag
289290
// Yarn classic wraps the manifest in a `data` property.
290291
return data.data ?? data;
291292
}
293+
294+
/**
295+
* Parses the stderr output of npm, pnpm, modern yarn, or bun to extract structured error information.
296+
*
297+
* This parser first attempts to parse the stderr as JSON, which is common for pnpm, modern yarn, and bun.
298+
* If JSON parsing fails, it falls back to a regex-based approach to extract common npm error codes
299+
* (e.g., E404) from the plain text output.
300+
*
301+
* Example JSON output (pnpm):
302+
* ```json
303+
* {
304+
* "code": "E404",
305+
* "summary": "Not Found - GET https://registry.npmjs.org/@angular%2fnon-existent - Not found",
306+
* "detail": "The requested resource '@angular/non-existent@*' could not be found or you do not have permission to access it."
307+
* }
308+
* ```
309+
*
310+
* Example text output (npm):
311+
* ```
312+
* npm ERR! code E404
313+
* npm ERR! 404 Not Found - GET https://registry.npmjs.org/@angular%2fnon-existent - Not found
314+
* ```
315+
*
316+
* @param stderr The standard error output of the command.
317+
* @param logger An optional logger instance.
318+
* @returns A `StructuredError` object if parsing is successful, otherwise `null`.
319+
*/
320+
export function parseNpmLikeError(stderr: string, logger?: Logger): ErrorInfo | null {
321+
logger?.debug(`Parsing npm-like error output...`);
322+
logStdout(stderr, logger); // Log stderr for debugging purposes
323+
324+
if (!stderr) {
325+
logger?.debug(' stderr is empty. No error found.');
326+
327+
return null;
328+
}
329+
330+
// Attempt to parse as JSON first (common for pnpm, modern yarn, bun)
331+
try {
332+
const jsonError = JSON.parse(stderr);
333+
if (
334+
jsonError &&
335+
typeof jsonError.code === 'string' &&
336+
(typeof jsonError.summary === 'string' || typeof jsonError.message === 'string')
337+
) {
338+
const summary = jsonError.summary || jsonError.message;
339+
logger?.debug(` Successfully parsed JSON error with code '${jsonError.code}'.`);
340+
341+
return {
342+
code: jsonError.code,
343+
summary: summary,
344+
detail: jsonError.detail,
345+
};
346+
}
347+
} catch (e) {
348+
logger?.debug(` Failed to parse stderr as JSON: ${e}. Attempting regex fallback.`);
349+
// Fallback to regex for plain text errors (common for npm)
350+
}
351+
352+
// Regex for npm-like error codes (e.g., `npm ERR! code E404` or `npm error code E404`)
353+
const errorCodeMatch = stderr.match(/npm (ERR!|error) code (E\d{3}|[A-Z_]+)/);
354+
if (errorCodeMatch) {
355+
const code = errorCodeMatch[2]; // Capture group 2 is the actual error code
356+
let summary: string | undefined;
357+
358+
// Find the most descriptive summary line (the line after `npm ERR! code ...` or `npm error code ...`).
359+
for (const line of stderr.split('\n')) {
360+
if (line.startsWith('npm ERR!') && !line.includes(' code ')) {
361+
summary = line.replace('npm ERR! ', '').trim();
362+
break;
363+
} else if (line.startsWith('npm error') && !line.includes(' code ')) {
364+
summary = line.replace('npm error ', '').trim();
365+
break;
366+
}
367+
}
368+
369+
logger?.debug(` Successfully parsed text error with code '${code}'.`);
370+
371+
return {
372+
code,
373+
summary: summary || `Package manager error: ${code}`,
374+
};
375+
}
376+
377+
logger?.debug(' Failed to parse npm-like error. No structured error found.');
378+
379+
return null;
380+
}
381+
382+
/**
383+
* Parses the stderr output of yarn classic to extract structured error information.
384+
*
385+
* Yarn classic typically outputs JSON lines for errors, with a 'type' of 'error'.
386+
*
387+
* Example JSON output:
388+
* ```json
389+
* {"type":"error","data":"An unexpected error occurred: "https://registry.npmjs.org/@angular%2fnon-existent: Not found""}
390+
* ```
391+
*
392+
* @param stderr The standard error output of the command.
393+
* @param logger An optional logger instance.
394+
* @returns A `StructuredError` object if parsing is successful, otherwise `null`.
395+
*/
396+
export function parseYarnClassicError(stderr: string, logger?: Logger): ErrorInfo | null {
397+
logger?.debug(`Parsing yarn classic error output...`);
398+
logStdout(stderr, logger); // Log stderr for debugging purposes
399+
400+
if (!stderr) {
401+
logger?.debug(' stderr is empty. No error found.');
402+
403+
return null;
404+
}
405+
406+
// First, check for any HTTP status code in the verbose output.
407+
const statusCodeMatch = stderr.match(/finished with status code (\d{3})/);
408+
if (statusCodeMatch) {
409+
const statusCode = statusCodeMatch[1];
410+
logger?.debug(` Detected HTTP status code '${statusCode}' in verbose output.`);
411+
412+
return {
413+
code: `E${statusCode}`,
414+
summary: `Request failed with status code ${statusCode}.`,
415+
};
416+
}
417+
418+
// Fallback to the JSON error type if no HTTP status code is present.
419+
for (const line of stderr.split('\n')) {
420+
if (!line) {
421+
continue;
422+
}
423+
try {
424+
const json = JSON.parse(line);
425+
if (json.type === 'error' && typeof json.data === 'string') {
426+
const message = json.data;
427+
logger?.debug(` Successfully parsed generic yarn classic error.`);
428+
429+
return {
430+
code: 'UNKNOWN_ERROR',
431+
summary: message,
432+
};
433+
}
434+
} catch (e) {
435+
// Ignore lines that are not valid JSON or not error types
436+
logger?.debug(` Ignoring non-JSON or non-error line: ${e}`);
437+
}
438+
}
439+
440+
logger?.debug(' Failed to parse yarn classic error. No structured error found.');
441+
442+
return null;
443+
}

0 commit comments

Comments
 (0)