From 311204a8b4e4a5805ddf60c3df5058c4438eb63f Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Tue, 14 Jan 2025 15:28:58 -0500 Subject: [PATCH] fix(core): show better project graph errors (#29525) ## Current Behavior Sub-errors are hidden when any project graph error is encountered. This is detrimental, as things like "missing comma in JSON" get hidden and make people think that Nx is broken, when in fact their config files are invalid. ## Expected Behavior Sub errors are shown regardless of verbose logging (but including their stack trace if verbose logging is enabled) ### Without Verbose ![image](https://github.com/user-attachments/assets/3a96d07e-3f0a-4eb7-8629-0c02c6912746) ### With Verbose ![image](https://github.com/user-attachments/assets/41b83e19-e6b1-471c-80ca-004b8f56d8f2) ## Related Issue(s) Fixes # (cherry picked from commit c060b3ae7f107d4b682583d598adfc0f382b06dd) --- packages/nx/src/project-graph/error-types.ts | 95 +++++++++++++++---- .../nx/src/project-graph/project-graph.ts | 3 - .../utils/project-configuration-utils.ts | 22 +---- packages/nx/src/utils/handle-errors.spec.ts | 7 +- packages/nx/src/utils/handle-errors.ts | 22 ++--- 5 files changed, 95 insertions(+), 54 deletions(-) diff --git a/packages/nx/src/project-graph/error-types.ts b/packages/nx/src/project-graph/error-types.ts index 1f61deb951d4b..093bcff466a7f 100644 --- a/packages/nx/src/project-graph/error-types.ts +++ b/packages/nx/src/project-graph/error-types.ts @@ -7,20 +7,11 @@ import { ProjectGraph } from '../config/project-graph'; import { CreateNodesFunctionV2 } from './plugins/public-api'; export class ProjectGraphError extends Error { - readonly #errors: Array< - | AggregateCreateNodesError - | MergeNodesError - | CreateMetadataError - | ProjectsWithNoNameError - | MultipleProjectsWithSameNameError - | ProcessDependenciesError - | WorkspaceValidityError - >; readonly #partialProjectGraph: ProjectGraph; readonly #partialSourceMaps: ConfigurationSourceMaps; constructor( - errors: Array< + private readonly errors: Array< | AggregateCreateNodesError | MergeNodesError | ProjectsWithNoNameError @@ -32,16 +23,46 @@ export class ProjectGraphError extends Error { partialProjectGraph: ProjectGraph, partialSourceMaps: ConfigurationSourceMaps ) { - super( - `Failed to process project graph. Run "nx reset" to fix this. Please report the issue if you keep seeing it.` - ); + const messageFragments = ['Failed to process project graph.']; + const mergeNodesErrors = []; + const unknownErrors = []; + for (const e of errors) { + if ( + // Known errors that are self-explanatory + isAggregateCreateNodesError(e) || + isCreateMetadataError(e) || + isProcessDependenciesError(e) || + isProjectsWithNoNameError(e) || + isMultipleProjectsWithSameNameError(e) || + isWorkspaceValidityError(e) + ) { + } else if ( + // Known error type, but unlikely to be caused by the user + isMergeNodesError(e) + ) { + mergeNodesErrors.push(e); + } else { + unknownErrors.push(e); + } + } + if (mergeNodesErrors.length > 0) { + messageFragments.push( + `This type of error most likely points to an issue within Nx. Please report it.` + ); + } + if (unknownErrors.length > 0) { + messageFragments.push( + `If the error cause is not obvious from the below error messages, running "nx reset" may fix it. Please report the issue if you keep seeing it.` + ); + } + super(messageFragments.join(' ')); this.name = this.constructor.name; - this.#errors = errors; + this.errors = errors; this.#partialProjectGraph = partialProjectGraph; this.#partialSourceMaps = partialSourceMaps; - this.stack = `${this.message}\n ${errors + this.stack = errors .map((error) => indentString(formatErrorStackAndCause(error), 2)) - .join('\n')}`; + .join('\n'); } /** @@ -67,7 +88,7 @@ export class ProjectGraphError extends Error { } getErrors() { - return this.#errors; + return this.errors; } } @@ -242,6 +263,36 @@ export class AggregateCreateNodesError extends Error { } } +export function formatAggregateCreateNodesError( + error: AggregateCreateNodesError, + pluginName: string +) { + const errorBodyLines = [ + `${ + error.errors.length > 1 ? `${error.errors.length} errors` : 'An error' + } occurred while processing files for the ${pluginName} plugin.`, + ]; + const errorStackLines = []; + + const innerErrors = error.errors; + for (const [file, e] of innerErrors) { + if (file) { + errorBodyLines.push(` - ${file}: ${e.message}`); + errorStackLines.push(` - ${file}: ${e.stack}`); + } else { + errorBodyLines.push(` - ${e.message}`); + errorStackLines.push(` - ${e.stack}`); + } + if (e.stack && process.env.NX_VERBOSE_LOGGING === 'true') { + const innerStackTrace = ' ' + e.stack.split('\n')?.join('\n '); + errorStackLines.push(innerStackTrace); + } + } + + error.stack = errorStackLines.join('\n'); + error.message = errorBodyLines.join('\n'); +} + export class MergeNodesError extends Error { file: string; pluginName: string; @@ -292,6 +343,16 @@ export class ProcessDependenciesError extends Error { this.stack = `${this.message}\n ${cause.stack.split('\n').join('\n ')}`; } } + +function isProcessDependenciesError(e: unknown): e is ProcessDependenciesError { + return ( + e instanceof ProcessDependenciesError || + (typeof e === 'object' && + 'name' in e && + e?.name === ProcessDependenciesError.name) + ); +} + export class WorkspaceValidityError extends Error { constructor(public message: string) { message = `Configuration Error\n${message}`; diff --git a/packages/nx/src/project-graph/project-graph.ts b/packages/nx/src/project-graph/project-graph.ts index 2188c65a773c5..69445fd276a56 100644 --- a/packages/nx/src/project-graph/project-graph.ts +++ b/packages/nx/src/project-graph/project-graph.ts @@ -178,9 +178,6 @@ export function handleProjectGraphError(opts: { exitOnError: boolean }, e) { const isVerbose = process.env.NX_VERBOSE_LOGGING === 'true'; if (e instanceof ProjectGraphError) { let title = e.message; - if (isVerbose) { - title += ' See errors below.'; - } const bodyLines = isVerbose ? [e.stack] diff --git a/packages/nx/src/project-graph/utils/project-configuration-utils.ts b/packages/nx/src/project-graph/utils/project-configuration-utils.ts index b83315d691ca4..d779132c06b91 100644 --- a/packages/nx/src/project-graph/utils/project-configuration-utils.ts +++ b/packages/nx/src/project-graph/utils/project-configuration-utils.ts @@ -28,6 +28,7 @@ import { isProjectWithNoNameError, isAggregateCreateNodesError, AggregateCreateNodesError, + formatAggregateCreateNodesError, } from '../error-types'; import { CreateNodesResult } from '../plugins/public-api'; import { isGlobPattern } from '../../utils/globs'; @@ -392,31 +393,12 @@ export async function createProjectConfigurations( workspaceRoot: root, }) .catch((e: Error) => { - const errorBodyLines = [ - `An error occurred while processing files for the ${pluginName} plugin.`, - ]; const error: AggregateCreateNodesError = isAggregateCreateNodesError(e) ? // This is an expected error if something goes wrong while processing files. e : // This represents a single plugin erroring out with a hard error. new AggregateCreateNodesError([[null, e]], []); - - const innerErrors = error.errors; - for (const [file, e] of innerErrors) { - if (file) { - errorBodyLines.push(` - ${file}: ${e.message}`); - } else { - errorBodyLines.push(` - ${e.message}`); - } - if (e.stack) { - const innerStackTrace = - ' ' + e.stack.split('\n')?.join('\n '); - errorBodyLines.push(innerStackTrace); - } - } - - error.stack = errorBodyLines.join('\n'); - + formatAggregateCreateNodesError(error, pluginName); // This represents a single plugin erroring out with a hard error. errors.push(error); // The plugin didn't return partial results, so we return an empty array. diff --git a/packages/nx/src/utils/handle-errors.spec.ts b/packages/nx/src/utils/handle-errors.spec.ts index bc9a6877fa1d1..456459f06bea5 100644 --- a/packages/nx/src/utils/handle-errors.spec.ts +++ b/packages/nx/src/utils/handle-errors.spec.ts @@ -25,9 +25,11 @@ describe('handleErrors', () => { const body = bodyLines.join('\n'); expect(body).toContain('cause message'); expect(body).toContain('test-plugin'); + // --verbose is active, so we should see the stack trace + expect(body).toMatch(/\s+at.*handle-errors.spec.ts/); }); - it('should only display wrapper error if not verbose', async () => { + it('should not display stack trace if not verbose', async () => { const spy = jest.spyOn(output, 'error').mockImplementation(() => {}); await handleErrors(false, async () => { const cause = new Error('cause message'); @@ -41,7 +43,8 @@ describe('handleErrors', () => { const { bodyLines, title } = spy.mock.calls[0][0]; const body = bodyLines.join('\n'); - expect(body).not.toContain('cause message'); + expect(body).toContain('cause message'); + expect(body).not.toMatch(/\s+at.*handle-errors.spec.ts/); }); it('should display misc errors that do not have a cause', async () => { diff --git a/packages/nx/src/utils/handle-errors.ts b/packages/nx/src/utils/handle-errors.ts index 2b15d9bce71fb..4685cf2a05d34 100644 --- a/packages/nx/src/utils/handle-errors.ts +++ b/packages/nx/src/utils/handle-errors.ts @@ -26,23 +26,18 @@ export async function handleErrors( ) { title += ' ' + projectGraphError.cause.message + '.'; } - if (isVerbose) { - title += ' See errors below.'; - } - - const bodyLines = isVerbose - ? formatErrorStackAndCause(projectGraphError) - : ['Pass --verbose to see the stacktraces.']; output.error({ title, - bodyLines: bodyLines, + bodyLines: isVerbose + ? formatErrorStackAndCause(projectGraphError, isVerbose) + : projectGraphError.getErrors().map((e) => e.message), }); } else { const lines = (err.message ? err.message : err.toString()).split('\n'); const bodyLines: string[] = lines.slice(1); if (isVerbose) { - bodyLines.push(...formatErrorStackAndCause(err)); + bodyLines.push(...formatErrorStackAndCause(err, isVerbose)); } else if (err.stack) { bodyLines.push('Pass --verbose to see the stacktrace.'); } @@ -59,13 +54,16 @@ export async function handleErrors( } } -function formatErrorStackAndCause(error: T): string[] { +function formatErrorStackAndCause( + error: T, + verbose: boolean +): string[] { return [ - error.stack || error.message, + verbose ? error.stack || error.message : error.message, ...(error.cause && typeof error.cause === 'object' ? [ 'Caused by:', - 'stack' in error.cause + verbose && 'stack' in error.cause ? error.cause.stack.toString() : error.cause.toString(), ]