Skip to content

Commit

Permalink
fix(core): ensure process is kept alive when plugin communication in …
Browse files Browse the repository at this point in the history
…progress (#28948)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
When using NX_PLUGIN_NO_TIMEOUTS there is not a reference counted as the
setTimeout is not called. This could theoretically result in node
killing the process if the only thing that was keeping it alive was the
plugin call.

## Expected Behavior
A ref is tracked either way to prevent the process from getting dropped.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #
  • Loading branch information
AgentEnder authored Nov 15, 2024
1 parent 3763baf commit 5176a1e
Showing 1 changed file with 28 additions and 21 deletions.
49 changes: 28 additions & 21 deletions packages/nx/src/project-graph/plugins/isolation/plugin-pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,15 @@ const MINUTES = 10;

const MAX_MESSAGE_WAIT =
process.env.NX_PLUGIN_NO_TIMEOUTS === 'true'
? undefined
? // Registering a timeout prevents the process from exiting
// if the call to a plugin happens to be the only thing
// keeping the process alive. As such, even if timeouts are disabled
// we need to register one. 2147483647 is the max timeout
// that Node.js allows, and is equivalent to 24.8 days....
// This does mean that the NX_PLUGIN_NO_TIMEOUTS env var
// would still timeout after 24.8 days, but that seems
// like a reasonable compromise.
2147483647
: 1000 * 60 * MINUTES; // 10 minutes

interface PendingPromise {
Expand Down Expand Up @@ -73,16 +81,15 @@ export async function loadRemoteNxPlugin(
});
// logger.verbose(`[plugin-worker] started worker: ${worker.pid}`);

const loadTimeout = MAX_MESSAGE_WAIT
? setTimeout(() => {
rej(
new Error(
`Loading "${plugin}" timed out after ${MINUTES} minutes. ${PLUGIN_TIMEOUT_HINT_TEXT}`
)
);
}, MAX_MESSAGE_WAIT)
: undefined;

const loadTimeout = setTimeout(() => {
rej(
new Error(
`Loading "${
typeof plugin === 'string' ? plugin : plugin.plugin
}" timed out after ${MINUTES} minutes. ${PLUGIN_TIMEOUT_HINT_TEXT}`
)
);
}, MAX_MESSAGE_WAIT);
socket.on(
'data',
consumeMessagesFromSocket(
Expand Down Expand Up @@ -263,21 +270,21 @@ function registerPendingPromise(
operation: string;
}
): Promise<any> {
let resolver, rejector, timeout;
let resolver: (x: unknown) => void,
rejector: (e: Error | unknown) => void,
timeout: NodeJS.Timeout;

const promise = new Promise((res, rej) => {
rejector = rej;
resolver = res;

timeout = MAX_MESSAGE_WAIT
? setTimeout(() => {
rej(
new Error(
`${context.plugin} timed out after ${MINUTES} minutes during ${context.operation}. ${PLUGIN_TIMEOUT_HINT_TEXT}`
)
);
}, MAX_MESSAGE_WAIT)
: undefined;
timeout = setTimeout(() => {
rej(
new Error(
`${context.plugin} timed out after ${MINUTES} minutes during ${context.operation}. ${PLUGIN_TIMEOUT_HINT_TEXT}`
)
);
}, MAX_MESSAGE_WAIT);

callback();
}).finally(() => {
Expand Down

0 comments on commit 5176a1e

Please sign in to comment.