Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): handle plugin errors from isolation correctly #22890

Merged
merged 1 commit into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions packages/nx/src/daemon/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,11 @@ async function handleMessage(socket, data: string) {
}

if (daemonIsOutdated()) {
await respondWithErrorAndExit(socket, `Lock files changed`, {
name: '',
message: 'LOCK-FILES-CHANGED',
});
await respondWithErrorAndExit(
socket,
`Lock files changed`,
new Error('LOCK-FILES-CHANGED')
);
}

resetInactivityTimeout(handleInactivityTimeout);
Expand Down
21 changes: 3 additions & 18 deletions packages/nx/src/daemon/socket-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { unlinkSync } from 'fs';
import { platform } from 'os';
import { join, resolve } from 'path';
import { DAEMON_SOCKET_PATH, socketDir } from './tmp-dir';
import { DaemonProjectGraphError } from '../project-graph/error-types';
import { createSerializableError } from '../utils/serializable-error';

export const isWindows = platform() === 'win32';

Expand All @@ -27,29 +27,14 @@ export function killSocketOrPath(): void {
} catch {}
}

// Include the original stack trace within the serialized error so that the client can show it to the user.
function serializeError(error: Error | null): string | null {
if (!error) {
return null;
}

if (error instanceof DaemonProjectGraphError) {
error.errors = error.errors.map((e) => JSON.parse(serializeError(e)));
}

return `{${Object.getOwnPropertyNames(error)
.map((k) => `"${k}": ${JSON.stringify(error[k])}`)
.join(',')}}`;
}

// Prepare a serialized project graph result for sending over IPC from the server to the client
export function serializeResult(
error: Error | null,
serializedProjectGraph: string | null,
serializedSourceMaps: string | null
): string | null {
// We do not want to repeat work `JSON.stringify`ing an object containing the potentially large project graph so merge as strings
return `{ "error": ${serializeError(
error
return `{ "error": ${JSON.stringify(
error ? createSerializableError(error) : error
)}, "projectGraph": ${serializedProjectGraph}, "sourceMaps": ${serializedSourceMaps} }`;
}
9 changes: 9 additions & 0 deletions packages/nx/src/project-graph/error-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,12 @@ export class DaemonProjectGraphError extends Error {
this.name = this.constructor.name;
}
}

export class LoadPluginError extends Error {
constructor(public plugin: string, cause: Error) {
super(`Could not load plugin ${plugin}`, {
cause,
});
this.name = this.constructor.name;
}
}
55 changes: 40 additions & 15 deletions packages/nx/src/project-graph/plugins/isolation/messaging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
import { PluginConfiguration } from '../../../config/nx-json';
import { CreateDependenciesContext, CreateNodesContext } from '../public-api';
import { LoadedNxPlugin } from '../internal-api';
import { Serializable } from 'child_process';

export interface PluginWorkerLoadMessage {
type: 'load';
Expand All @@ -26,7 +27,7 @@ export interface PluginWorkerLoadResult {
}
| {
success: false;
error: string;
error: Error;
};
}

Expand All @@ -49,7 +50,7 @@ export interface PluginWorkerCreateNodesResult {
}
| {
success: false;
error: string;
error: Error;
tx: string;
};
}
Expand All @@ -72,7 +73,7 @@ export interface PluginCreateDependenciesResult {
}
| {
success: false;
error: string;
error: Error;
tx: string;
};
}
Expand All @@ -96,7 +97,7 @@ export interface PluginWorkerProcessProjectGraphResult {
}
| {
success: false;
error: string;
error: Error;
tx: string;
};
}
Expand All @@ -113,6 +114,38 @@ export type PluginWorkerResult =
| PluginCreateDependenciesResult
| PluginWorkerProcessProjectGraphResult;

export function isPluginWorkerMessage(
message: Serializable
): message is PluginWorkerMessage {
return (
typeof message === 'object' &&
'type' in message &&
typeof message.type === 'string' &&
[
'load',
'createNodes',
'createDependencies',
'processProjectGraph',
].includes(message.type)
);
}

export function isPluginWorkerResult(
message: Serializable
): message is PluginWorkerResult {
return (
typeof message === 'object' &&
'type' in message &&
typeof message.type === 'string' &&
[
'load-result',
'createNodesResult',
'createDependenciesResult',
'processProjectGraphResult',
].includes(message.type)
);
}

type MaybePromise<T> = T | Promise<T>;

// The handler can return a message to be sent back to the process from which the message originated
Expand All @@ -126,28 +159,20 @@ type MessageHandlerReturn<T extends PluginWorkerMessage | PluginWorkerResult> =
export async function consumeMessage<
T extends PluginWorkerMessage | PluginWorkerResult
>(
raw: string | T,
raw: T,
handlers: {
[K in T['type']]: (
// Extract restricts the type of payload to the payload of the message with the type K
payload: Extract<T, { type: K }>['payload']
) => MessageHandlerReturn<T>;
}
) {
const message: T = typeof raw === 'string' ? JSON.parse(raw) : raw;
const message: T = raw;
const handler = handlers[message.type];
if (handler) {
const response = await handler(message.payload);
if (response) {
process.send!(createMessage(response));
process.send!(response);
}
} else {
throw new Error(`Unhandled message type: ${message.type}`);
}
}

export function createMessage(
message: PluginWorkerMessage | PluginWorkerResult
): string {
return JSON.stringify(message);
}
49 changes: 20 additions & 29 deletions packages/nx/src/project-graph/plugins/isolation/plugin-pool.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ChildProcess, fork } from 'child_process';
import { ChildProcess, Serializable, fork } from 'child_process';
import path = require('path');

import { PluginConfiguration } from '../../../config/nx-json';
Expand All @@ -7,7 +7,7 @@ import { PluginConfiguration } from '../../../config/nx-json';
// import { logger } from '../../utils/logger';

import { LoadedNxPlugin, nxPluginCache } from '../internal-api';
import { PluginWorkerResult, consumeMessage, createMessage } from './messaging';
import { consumeMessage, isPluginWorkerResult } from './messaging';

const cleanupFunctions = new Set<() => void>();

Expand Down Expand Up @@ -45,7 +45,7 @@ export function loadRemoteNxPlugin(
...(isWorkerTypescript ? ['-r', 'ts-node/register'] : []),
],
});
worker.send(createMessage({ type: 'load', payload: { plugin, root } }));
worker.send({ type: 'load', payload: { plugin, root } });

// logger.verbose(`[plugin-worker] started worker: ${worker.pid}`);

Expand Down Expand Up @@ -103,14 +103,11 @@ function createWorkerHandler(
) {
let pluginName: string;

return function (message: string) {
const parsed = JSON.parse(message);
// logger.verbose(
// `[plugin-pool] received message: ${parsed.type} from ${
// pluginName ?? worker.pid
// }`
// );
consumeMessage<PluginWorkerResult>(parsed, {
return function (message: Serializable) {
if (!isPluginWorkerResult(message)) {
return;
}
return consumeMessage(message, {
'load-result': (result) => {
if (result.success) {
const { name, createNodesPattern } = result;
Expand All @@ -124,12 +121,10 @@ function createWorkerHandler(
(configFiles, ctx) => {
const tx = pluginName + ':createNodes:' + performance.now();
return registerPendingPromise(tx, pending, () => {
worker.send(
createMessage({
type: 'createNodes',
payload: { configFiles, context: ctx, tx },
})
);
worker.send({
type: 'createNodes',
payload: { configFiles, context: ctx, tx },
});
});
},
]
Expand All @@ -139,12 +134,10 @@ function createWorkerHandler(
const tx =
pluginName + ':createDependencies:' + performance.now();
return registerPendingPromise(tx, pending, () => {
worker.send(
createMessage({
type: 'createDependencies',
payload: { context: ctx, tx },
})
);
worker.send({
type: 'createDependencies',
payload: { context: ctx, tx },
});
});
}
: undefined,
Expand All @@ -153,12 +146,10 @@ function createWorkerHandler(
const tx =
pluginName + ':processProjectGraph:' + performance.now();
return registerPendingPromise(tx, pending, () => {
worker.send(
createMessage({
type: 'processProjectGraph',
payload: { graph, ctx, tx },
})
);
worker.send({
type: 'processProjectGraph',
payload: { graph, ctx, tx },
});
});
}
: undefined,
Expand Down
34 changes: 24 additions & 10 deletions packages/nx/src/project-graph/plugins/isolation/plugin-worker.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import { consumeMessage, PluginWorkerMessage } from './messaging';
import { consumeMessage, isPluginWorkerMessage } from './messaging';
import { LoadedNxPlugin } from '../internal-api';
import { loadNxPlugin } from '../loader';
import { runCreateNodesInParallel } from '../utils';
import { Serializable } from 'child_process';
import { createSerializableError } from '../../../utils/serializable-error';

global.NX_GRAPH_CREATION = true;

let plugin: LoadedNxPlugin;

process.on('message', async (message: string) => {
consumeMessage<PluginWorkerMessage>(message, {
process.on('message', async (message: Serializable) => {
if (!isPluginWorkerMessage(message)) {
return;
}
return consumeMessage(message, {
load: async ({ plugin: pluginConfiguration, root }) => {
process.chdir(root);
try {
Expand All @@ -31,9 +35,7 @@ process.on('message', async (message: string) => {
type: 'load-result',
payload: {
success: false,
error: `Could not load plugin ${plugin} \n ${
e instanceof Error ? e.stack : ''
}`,
error: createSerializableError(e),
},
};
}
Expand All @@ -48,7 +50,11 @@ process.on('message', async (message: string) => {
} catch (e) {
return {
type: 'createNodesResult',
payload: { success: false, error: e.stack, tx },
payload: {
success: false,
error: createSerializableError(e),
tx,
},
};
}
},
Expand All @@ -62,7 +68,11 @@ process.on('message', async (message: string) => {
} catch (e) {
return {
type: 'createDependenciesResult',
payload: { success: false, error: e.stack, tx },
payload: {
success: false,
error: createSerializableError(e),
tx,
},
};
}
},
Expand All @@ -76,7 +86,11 @@ process.on('message', async (message: string) => {
} catch (e) {
return {
type: 'processProjectGraphResult',
payload: { success: false, error: e.stack, tx },
payload: {
success: false,
error: createSerializableError(e),
tx,
},
};
}
},
Expand Down