Skip to content

Commit 7ae01cc

Browse files
AgentEnderFrozenPandaz
authored andcommitted
fix(core): ensure process is kept alive when plugin communication in 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 # (cherry picked from commit 5176a1e)
1 parent be69f8c commit 7ae01cc

File tree

1 file changed

+28
-21
lines changed

1 file changed

+28
-21
lines changed

packages/nx/src/project-graph/plugins/isolation/plugin-pool.ts

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,15 @@ const MINUTES = 10;
2828

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

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

76-
const loadTimeout = MAX_MESSAGE_WAIT
77-
? setTimeout(() => {
78-
rej(
79-
new Error(
80-
`Loading "${plugin}" timed out after ${MINUTES} minutes. ${PLUGIN_TIMEOUT_HINT_TEXT}`
81-
)
82-
);
83-
}, MAX_MESSAGE_WAIT)
84-
: undefined;
85-
84+
const loadTimeout = setTimeout(() => {
85+
rej(
86+
new Error(
87+
`Loading "${
88+
typeof plugin === 'string' ? plugin : plugin.plugin
89+
}" timed out after ${MINUTES} minutes. ${PLUGIN_TIMEOUT_HINT_TEXT}`
90+
)
91+
);
92+
}, MAX_MESSAGE_WAIT);
8693
socket.on(
8794
'data',
8895
consumeMessagesFromSocket(
@@ -263,21 +270,21 @@ function registerPendingPromise(
263270
operation: string;
264271
}
265272
): Promise<any> {
266-
let resolver, rejector, timeout;
273+
let resolver: (x: unknown) => void,
274+
rejector: (e: Error | unknown) => void,
275+
timeout: NodeJS.Timeout;
267276

268277
const promise = new Promise((res, rej) => {
269278
rejector = rej;
270279
resolver = res;
271280

272-
timeout = MAX_MESSAGE_WAIT
273-
? setTimeout(() => {
274-
rej(
275-
new Error(
276-
`${context.plugin} timed out after ${MINUTES} minutes during ${context.operation}. ${PLUGIN_TIMEOUT_HINT_TEXT}`
277-
)
278-
);
279-
}, MAX_MESSAGE_WAIT)
280-
: undefined;
281+
timeout = setTimeout(() => {
282+
rej(
283+
new Error(
284+
`${context.plugin} timed out after ${MINUTES} minutes during ${context.operation}. ${PLUGIN_TIMEOUT_HINT_TEXT}`
285+
)
286+
);
287+
}, MAX_MESSAGE_WAIT);
281288

282289
callback();
283290
}).finally(() => {

0 commit comments

Comments
 (0)