Skip to content

Commit 170fece

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 a140f8f commit 170fece

File tree

5 files changed

+356
-10
lines changed

5 files changed

+356
-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,16 +12,19 @@
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,
2325
parseYarnClassicManifest,
2426
parseYarnClassicMetadata,
27+
parseYarnClassicError,
2528
parseYarnModernDependencies,
2629
} from './parsers';
2730

@@ -90,12 +93,30 @@ export interface PackageManagerDescriptor {
9093

9194
/** A function to parse the output of `getManifestCommand` for the full package metadata. */
9295
getRegistryMetadata: (stdout: string, logger?: Logger) => PackageMetadata | null;
96+
97+
/** A function to parse the output of stderr when a command fails. */
98+
getError?: (stderr: string, logger?: Logger) => ErrorInfo | null;
9399
};
100+
101+
/** A function that checks if a structured error represents a "package not found" error. */
102+
readonly isNotFound: (error: ErrorInfo) => boolean;
94103
}
95104

96105
/** A type that represents the name of a supported package manager. */
97106
export type PackageManagerName = keyof typeof SUPPORTED_PACKAGE_MANAGERS;
98107

108+
/** A set of error codes that are known to indicate a "package not found" error. */
109+
const NOT_FOUND_ERROR_CODES = new Set(['E404']);
110+
111+
/**
112+
* A shared function to check if a structured error represents a "package not found" error.
113+
* @param error The structured error to check.
114+
* @returns True if the error code is a known "not found" code, false otherwise.
115+
*/
116+
function isKnownNotFound(error: ErrorInfo): boolean {
117+
return NOT_FOUND_ERROR_CODES.has(error.code);
118+
}
119+
99120
/**
100121
* A map of supported package managers to their descriptors.
101122
* This is the single source of truth for all package-manager-specific
@@ -128,7 +149,9 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
128149
listDependencies: parseNpmLikeDependencies,
129150
getRegistryManifest: parseNpmLikeManifest,
130151
getRegistryMetadata: parseNpmLikeMetadata,
152+
getError: parseNpmLikeError,
131153
},
154+
isNotFound: isKnownNotFound,
132155
},
133156
yarn: {
134157
binary: 'yarn',
@@ -150,7 +173,9 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
150173
listDependencies: parseYarnModernDependencies,
151174
getRegistryManifest: parseNpmLikeManifest,
152175
getRegistryMetadata: parseNpmLikeMetadata,
176+
getError: parseNpmLikeError,
153177
},
178+
isNotFound: isKnownNotFound,
154179
},
155180
'yarn-classic': {
156181
binary: 'yarn',
@@ -169,13 +194,15 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
169194
getRegistryOptions: (registry: string) => ({ args: ['--registry', registry] }),
170195
versionCommand: ['--version'],
171196
listDependenciesCommand: ['list', '--depth=0', '--json'],
172-
getManifestCommand: ['info', '--json'],
197+
getManifestCommand: ['info', '--json', '--verbose'],
173198
requiresManifestVersionLookup: true,
174199
outputParsers: {
175200
listDependencies: parseYarnClassicDependencies,
176201
getRegistryManifest: parseYarnClassicManifest,
177202
getRegistryMetadata: parseYarnClassicMetadata,
203+
getError: parseYarnClassicError,
178204
},
205+
isNotFound: isKnownNotFound,
179206
},
180207
pnpm: {
181208
binary: 'pnpm',
@@ -197,7 +224,9 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
197224
listDependencies: parseNpmLikeDependencies,
198225
getRegistryManifest: parseNpmLikeManifest,
199226
getRegistryMetadata: parseNpmLikeMetadata,
227+
getError: parseNpmLikeError,
200228
},
229+
isNotFound: isKnownNotFound,
201230
},
202231
bun: {
203232
binary: 'bun',
@@ -218,7 +247,9 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
218247
listDependencies: parseNpmLikeDependencies,
219248
getRegistryManifest: parseNpmLikeManifest,
220249
getRegistryMetadata: parseNpmLikeMetadata,
250+
getError: parseNpmLikeError,
221251
},
252+
isNotFound: isKnownNotFound,
222253
},
223254
} satisfies Record<string, PackageManagerDescriptor>;
224255

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

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import { join } from 'node:path';
1616
import npa from 'npm-package-arg';
1717
import { maxSatisfying } from 'semver';
18-
import { PackageManagerError } from './error';
18+
import { ErrorInfo, PackageManagerError } from './error';
1919
import { Host } from './host';
2020
import { Logger } from './logger';
2121
import { PackageManagerDescriptor } from './package-manager-descriptor';
@@ -194,17 +194,45 @@ export class PackageManager {
194194

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

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

@@ -219,7 +247,7 @@ export class PackageManager {
219247
const message = `Failed to parse package manager output: ${
220248
e instanceof Error ? e.message : ''
221249
}`;
222-
throw new PackageManagerError(message, stdout, stderr, 0);
250+
throw new PackageManagerError(message, stdout, stderr, exitCode);
223251
}
224252
}
225253

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

Lines changed: 161 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
@@ -323,3 +324,162 @@ export function parseYarnClassicMetadata(stdout: string, logger?: Logger): Packa
323324
// Yarn classic wraps the metadata in a `data` property.
324325
return data.data;
325326
}
327+
328+
/**
329+
* Parses the stderr output of npm, pnpm, modern yarn, or bun to extract structured error information.
330+
*
331+
* This parser uses a multi-stage approach. It first attempts to parse the entire `stderr` as a
332+
* single JSON object, which is the standard for modern tools like pnpm, yarn, and bun. If JSON
333+
* parsing fails, it falls back to a line-by-line regex-based approach to handle the plain
334+
* text output from older versions of npm.
335+
*
336+
* Example JSON output (pnpm):
337+
* ```json
338+
* {
339+
* "code": "E404",
340+
* "summary": "Not Found - GET https://registry.npmjs.org/@angular%2fnon-existent - Not found",
341+
* "detail": "The requested resource '@angular/non-existent@*' could not be found or you do not have permission to access it."
342+
* }
343+
* ```
344+
*
345+
* Example text output (npm):
346+
* ```
347+
* npm error code E404
348+
* npm error 404 Not Found - GET https://registry.npmjs.org/@angular%2fnon-existent - Not found
349+
* ```
350+
*
351+
* @param stderr The standard error output of the command.
352+
* @param logger An optional logger instance.
353+
* @returns An `ErrorInfo` object if parsing is successful, otherwise `null`.
354+
*/
355+
export function parseNpmLikeError(stderr: string, logger?: Logger): ErrorInfo | null {
356+
logger?.debug(`Parsing npm-like error output...`);
357+
logStdout(stderr, logger); // Log stderr for debugging purposes
358+
359+
if (!stderr) {
360+
logger?.debug(' stderr is empty. No error found.');
361+
362+
return null;
363+
}
364+
365+
// Attempt to parse as JSON first (common for pnpm, modern yarn, bun)
366+
try {
367+
const jsonError = JSON.parse(stderr);
368+
if (
369+
jsonError &&
370+
typeof jsonError.code === 'string' &&
371+
(typeof jsonError.summary === 'string' || typeof jsonError.message === 'string')
372+
) {
373+
const summary = jsonError.summary || jsonError.message;
374+
logger?.debug(` Successfully parsed JSON error with code '${jsonError.code}'.`);
375+
376+
return {
377+
code: jsonError.code,
378+
summary,
379+
detail: jsonError.detail,
380+
};
381+
}
382+
} catch (e) {
383+
logger?.debug(` Failed to parse stderr as JSON: ${e}. Attempting regex fallback.`);
384+
// Fallback to regex for plain text errors (common for npm)
385+
}
386+
387+
// Regex for npm-like error codes (e.g., `npm ERR! code E404` or `npm error code E404`)
388+
const errorCodeMatch = stderr.match(/npm (ERR!|error) code (E\d{3}|[A-Z_]+)/);
389+
if (errorCodeMatch) {
390+
const code = errorCodeMatch[2]; // Capture group 2 is the actual error code
391+
let summary: string | undefined;
392+
393+
// Find the most descriptive summary line (the line after `npm ERR! code ...` or `npm error code ...`).
394+
for (const line of stderr.split('\n')) {
395+
if (line.startsWith('npm ERR!') && !line.includes(' code ')) {
396+
summary = line.replace('npm ERR! ', '').trim();
397+
break;
398+
} else if (line.startsWith('npm error') && !line.includes(' code ')) {
399+
summary = line.replace('npm error ', '').trim();
400+
break;
401+
}
402+
}
403+
404+
logger?.debug(` Successfully parsed text error with code '${code}'.`);
405+
406+
return {
407+
code,
408+
summary: summary || `Package manager error: ${code}`,
409+
};
410+
}
411+
412+
logger?.debug(' Failed to parse npm-like error. No structured error found.');
413+
414+
return null;
415+
}
416+
417+
/**
418+
* Parses the stderr output of yarn classic to extract structured error information.
419+
*
420+
* This parser first attempts to find an HTTP status code (e.g., 404, 401) in the verbose output.
421+
* If found, it returns a standardized error code (`E${statusCode}`).
422+
* If no HTTP status code is found, it falls back to parsing generic JSON error lines.
423+
*
424+
* Example verbose output (with HTTP status code):
425+
* ```json
426+
* {"type":"verbose","data":"Request \"https://registry.npmjs.org/@angular%2fnon-existent\" finished with status code 404."}
427+
* ```
428+
*
429+
* Example generic JSON error output:
430+
* ```json
431+
* {"type":"error","data":"Received invalid response from npm."}
432+
* ```
433+
*
434+
* @param stderr The standard error output of the command.
435+
* @param logger An optional logger instance.
436+
* @returns An `ErrorInfo` object if parsing is successful, otherwise `null`.
437+
*/
438+
export function parseYarnClassicError(stderr: string, logger?: Logger): ErrorInfo | null {
439+
logger?.debug(`Parsing yarn classic error output...`);
440+
logStdout(stderr, logger); // Log stderr for debugging purposes
441+
442+
if (!stderr) {
443+
logger?.debug(' stderr is empty. No error found.');
444+
445+
return null;
446+
}
447+
448+
// First, check for any HTTP status code in the verbose output.
449+
const statusCodeMatch = stderr.match(/finished with status code (\d{3})/);
450+
if (statusCodeMatch) {
451+
const statusCode = statusCodeMatch[1];
452+
logger?.debug(` Detected HTTP status code '${statusCode}' in verbose output.`);
453+
454+
return {
455+
code: `E${statusCode}`,
456+
summary: `Request failed with status code ${statusCode}.`,
457+
};
458+
}
459+
460+
// Fallback to the JSON error type if no HTTP status code is present.
461+
for (const line of stderr.split('\n')) {
462+
if (!line) {
463+
continue;
464+
}
465+
try {
466+
const json = JSON.parse(line);
467+
if (json.type === 'error' && typeof json.data === 'string') {
468+
const summary = json.data;
469+
logger?.debug(` Successfully parsed generic yarn classic error.`);
470+
471+
return {
472+
code: 'UNKNOWN_ERROR',
473+
summary,
474+
};
475+
}
476+
} catch (e) {
477+
// Ignore lines that are not valid JSON or not error types
478+
logger?.debug(` Ignoring non-JSON or non-error line: ${e}`);
479+
}
480+
}
481+
482+
logger?.debug(' Failed to parse yarn classic error. No structured error found.');
483+
484+
return null;
485+
}

0 commit comments

Comments
 (0)