Skip to content

Commit df8da55

Browse files
committed
fix: enhance process terminated errors and handle more edge cases in critial errors
1 parent f9c7c17 commit df8da55

File tree

4 files changed

+41
-2
lines changed

4 files changed

+41
-2
lines changed

src/cli/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ process.on("uncaughtException", err => {
2222
});
2323

2424
process.on("unhandledRejection", (reason, p) => {
25+
// This flag lets other unhandledRejection handlers know that we already processed it on Testplane side.
26+
// Currently we use this to avoid duplicate error logging and force shutdown in HTML Reporter.
27+
(global as Record<string, unknown>)["__TESTPLANE_INTERNAL_UNHANDLED_REJECTION_PROCESSED"] = true;
2528
if (shouldIgnoreUnhandledRejection(reason as Error)) {
2629
logger.warn(`Unhandled Rejection "${reason}" in testplane:master:${process.pid} was ignored`);
2730
return;

src/runner/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,9 @@ export class MainRunner extends RunnableEmitter {
208208

209209
this.activeBrowserRunners.forEach(runner => runner.cancel(error));
210210

211-
this.workers?.cancel();
211+
this.workers?.cancel().catch(() => {
212+
/* we can just ignore the error thrown, because we don't care about cleanup at this point */
213+
});
212214
}
213215

214216
registerWorkers<T extends ReadonlyArray<string>>(workerFilepath: string, exportedMethods: T): RegisterWorkers<T> {

src/utils/processor.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,24 @@
22

33
const _ = require("lodash");
44
const { WORKER_UNHANDLED_REJECTION } = require("../constants/process-messages");
5+
const debug = require("debug")("testplane:worker:processor");
56
const logger = require("./logger");
67
const ipc = require("./ipc");
78
const { shouldIgnoreUnhandledRejection } = require("./errors");
89
const { utilInspectSafe } = require("./secret-replacer");
910
const { preloadWebdriverIO, preloadMochaReader } = require("./preload-utils.js");
1011

12+
process.on("uncaughtException", err => {
13+
if (err.code === "EPIPE" || err.code === "ERR_IPC_CHANNEL_CLOSED") {
14+
debug(
15+
"The following error was ignored in worker, because we tried to send message to master process, but it has already exited",
16+
);
17+
debug(err);
18+
return;
19+
}
20+
throw err;
21+
});
22+
1123
process.on("unhandledRejection", (reason, p) => {
1224
if (shouldIgnoreUnhandledRejection(reason)) {
1325
logger.warn(`Unhandled Rejection "${reason}" in testplane:worker:${process.pid} was ignored`);

src/utils/workers-registry.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ const {
1717
TEST_ASSIGNED_TO_WORKER,
1818
} = require("../constants/process-messages");
1919
const { isRunInNodeJsEnv } = require("./config");
20+
const { utilInspectSafe } = require("./secret-replacer");
21+
const { NEW_ISSUE_LINK } = require("../constants/help");
2022

2123
const extractErrorFromWorkerMessage = data => {
2224
if (data.error) {
@@ -76,7 +78,27 @@ module.exports = class WorkersRegistry extends EventEmitter {
7678
if (this._ended) {
7779
return Promise.reject(new Error(`Can't execute method '${methodName}' because worker farm ended.`));
7880
}
79-
return promisify(this._workerFarm.execute)(workerFilepath, methodName, args);
81+
const stack = new Error().stack;
82+
return promisify(this._workerFarm.execute)(workerFilepath, methodName, args).catch(error => {
83+
if (error.name === "ProcessTerminatedError") {
84+
const workerCallError = new Error(
85+
`Testplane tried to run method '${methodName}' with args ${utilInspectSafe(
86+
args,
87+
)} in worker, but failed to do so.\n` +
88+
`Most likely this happened due to a critical error in the worker like unhandled promise rejection or the worker process was terminated unexpectedly.\n` +
89+
`Check surrounding logs for more details on the cause. If you believe this should not have happened, let us know: ${NEW_ISSUE_LINK}\n\n`,
90+
);
91+
try {
92+
workerCallError.stack = workerCallError.name + stack.split("\n").slice(1).join("\n");
93+
} catch {
94+
/* */
95+
}
96+
workerCallError.cause = error;
97+
98+
throw workerCallError;
99+
}
100+
throw error;
101+
});
80102
};
81103
}
82104

0 commit comments

Comments
 (0)