Skip to content

Commit 433f2ca

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 433f2ca

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

0 commit comments

Comments
 (0)