Skip to content

Commit

Permalink
fix(core): show better project graph errors (#29525)
Browse files Browse the repository at this point in the history 
## 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)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #

(cherry picked from commit c060b3a)
  • Loading branch information
@AgentEnder @FrozenPandaz
AgentEnder authored and FrozenPandaz committed Jan 15, 2025
1 parent bd35bfe commit 311204a
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 54 deletions.
95 changes: 78 additions & 17 deletions packages/nx/src/project-graph/error-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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');
}

/**
Expand All @@ -67,7 +88,7 @@ export class ProjectGraphError extends Error {
}

getErrors() {
return this.#errors;
return this.errors;
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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}`;
Expand Down
3 changes: 0 additions & 3 deletions packages/nx/src/project-graph/project-graph.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
22 changes: 2 additions & 20 deletions packages/nx/src/project-graph/utils/project-configuration-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
isProjectWithNoNameError,
isAggregateCreateNodesError,
AggregateCreateNodesError,
formatAggregateCreateNodesError,
} from '../error-types';
import { CreateNodesResult } from '../plugins/public-api';
import { isGlobPattern } from '../../utils/globs';
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions packages/nx/src/utils/handle-errors.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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 () => {
Expand Down
22 changes: 10 additions & 12 deletions packages/nx/src/utils/handle-errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
Expand All @@ -59,13 +54,16 @@ export async function handleErrors(
}
}

function formatErrorStackAndCause<T extends Error>(error: T): string[] {
function formatErrorStackAndCause<T extends Error>(
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(),
]
Expand Down

0 comments on commit 311204a

Please sign in to comment.