Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions electron/bridges/terminalWorkerManager.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ function createTerminalWorkerManager(options = {}) {
const urgentInputPorts = new Map();
const outputTaps = new Set();
const terminalInterceptorWarningListeners = new Set();
const workerProcessErrorListeners = new Set();
const sessionOwnedListeners = new Set();
const sessionClosedListeners = new Set();
const workerExitListeners = new Set();
Expand Down Expand Up @@ -1432,6 +1433,14 @@ function createTerminalWorkerManager(options = {}) {
}
return;
}
if (message.kind === "worker-process-error") {
// The worker survived this one (its process guards suppressed it), but
// main still owns the crash log, so record it there for bug reports.
for (const listener of [...workerProcessErrorListeners]) {
try { listener(message); } catch {}
}
return;
}
if (message.kind === "renderer-event") {
for (const listener of [...workerRendererEventListeners]) {
try { listener(message); } catch {}
Expand Down Expand Up @@ -2003,6 +2012,12 @@ function createTerminalWorkerManager(options = {}) {
return Object.freeze({ dispose: () => terminalInterceptorWarningListeners.delete(listener) });
}

function onWorkerProcessError(listener) {
if (typeof listener !== "function") throw new TypeError("Worker process error listener is required");
workerProcessErrorListeners.add(listener);
return Object.freeze({ dispose: () => workerProcessErrorListeners.delete(listener) });
}

function onSessionOwned(listener) {
if (typeof listener !== "function") throw new TypeError("Terminal session owner listener is required");
sessionOwnedListeners.add(listener);
Expand Down Expand Up @@ -2055,6 +2070,7 @@ function createTerminalWorkerManager(options = {}) {
supersedingSessionGenerations.clear();
closeAllUrgentInputPorts();
terminalInterceptorWarningListeners.clear();
workerProcessErrorListeners.clear();
sessionOwnedListeners.clear();
sessionClosedListeners.clear();
workerExitListeners.clear();
Expand Down Expand Up @@ -2088,6 +2104,7 @@ function createTerminalWorkerManager(options = {}) {
attachTerminalInterceptor,
detachTerminalInterceptor,
onTerminalInterceptorWarning,
onWorkerProcessError,
onSessionOwned,
onSessionClosed,
onWorkerExit,
Expand Down
30 changes: 30 additions & 0 deletions electron/bridges/terminalWorkerManager.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -4944,3 +4944,33 @@ test("worker exit notifies host lifecycle listeners for every active session", a
{ sessionId: "local-2", reason: "worker-exit" },
]);
});

test("worker process error reports are fanned out to main-process listeners", () => {
const child = new FakeChild();
const manager = createTerminalWorkerManager({
utilityProcess: {
fork() {
return child;
},
},
workerScriptPath: "/worker.cjs",
});
const seen = [];
const subscription = manager.onWorkerProcessError((report) => seen.push(report));
void manager.request("netcatty:test", {});

child.emit("message", {
kind: "worker-process-error",
source: "uncaughtException",
message: "Keepalive timeout",
level: "client-timeout",
});

assert.equal(seen.length, 1);
assert.equal(seen[0].source, "uncaughtException");
assert.equal(seen[0].message, "Keepalive timeout");

subscription.dispose();
child.emit("message", { kind: "worker-process-error", message: "second" });
assert.equal(seen.length, 1);
});
19 changes: 19 additions & 0 deletions electron/main/registerBridges.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,25 @@ function createBridgeRegistrar(context) {
: null;
registerTransportIdleTtlSettingsSync(ipcMain, terminalWorkerManager);
if (terminalWorkerManager) {
// A worker crash silently drops every live session at once. Record it in
// the crash log so bug reports carry the cause instead of only the
// "Terminal worker exited with code 1" symptom.
terminalWorkerManager.onWorkerExit?.((error) => {
try {
crashLogBridge.captureError("terminal-worker-exit", error);
} catch { /* never throw from crash logging */ }
});
terminalWorkerManager.onWorkerProcessError?.((report) => {
try {
const err = new Error(report?.message || "Terminal worker process error");
if (report?.stack) err.stack = report.stack;
crashLogBridge.captureError("terminal-worker-process-error", err, {
origin: report?.source,
...(report?.code ? { code: report.code } : {}),
...(report?.level ? { level: report.level } : {}),
});
} catch { /* never throw from crash logging */ }
});
const { registerPluginShutdown } = require("../plugins/shutdownCoordinator.cjs");
registerPluginShutdown(async () => {
try {
Expand Down
53 changes: 53 additions & 0 deletions electron/terminalWorker/process.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ const path = require("node:path");
const { randomUUID } = require("node:crypto");
const { createTerminalWorkerRuntime } = require("./runtime.cjs");
const tempDirBridge = require("../bridges/tempDirBridge.cjs");
const {
createProcessErrorController,
installProcessErrorHandlers,
} = require("../bridges/processErrorGuards.cjs");

// The worker owns SSH sessions in the default runtime path. Install the same
// DH compatibility shim as the main process before loading ssh2-backed bridges.
Expand Down Expand Up @@ -223,12 +227,60 @@ function registerPortForwardingWorkerBridge(ipcMain) {
portForwardingBridge.registerHandlers(ipcMain);
}

/**
* The worker owns every terminal, SSH, SFTP, and port-forwarding session in the
* default runtime path, so an unguarded async throw here is not one bad
* session -- it exits the utilityProcess with code 1 and terminalWorkerManager's
* handleExit() drops *every* live session at once.
*
* The main process installs these same guards for exactly this reason (see the
* "never a reason to kill the entire multi-session app" note in
* processErrorGuards.cjs). Once sessions moved into the worker, the worker
* became the process that has to survive them.
*/
function installWorkerProcessErrorGuards(parentPort, processObject = process) {
const controller = createProcessErrorController({
captureError(source, err) {
try {
parentPort?.postMessage?.({
kind: "worker-process-error",
source,
message: err?.message ? String(err.message) : String(err),
...(typeof err?.stack === "string" ? { stack: err.stack } : {}),
...(err?.code ? { code: String(err.code) } : {}),
...(err?.level ? { level: String(err.level) } : {}),
});
} catch {
// The parent port is already gone; the worker is being torn down.
}
},
onFatalError(err) {
// Unreachable while runtime protection is armed below, but never let a
// classification change silently reintroduce the mass-disconnect bug.
console.error("[TerminalWorker] Fatal process error:", err);
},
logError(...args) {
console.error(...args);
},
logWarn(...args) {
console.warn(...args);
},
});
// The worker has no window lifecycle: it is live the moment it is forked.
// Arm runtime protection immediately, otherwise classifyProcessError() reads
// every runtime error as a pre-startup failure and still lets the process die.
controller.completeMainWindowStartup({ windowShown: true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Arm process protection only after worker startup

If a generic exception occurs while loading or initializing any bridge between this call and runtime.start(), the controller already classifies it as a post-startup error and suppresses it. Node then unwinds main() without reaching the parentPort.on("message", ...) registration, while the utility process remains alive; because terminalWorkerManager.request() has no timeout, terminal requests remain pending indefinitely instead of the worker exiting and being replaced. Keep startup errors fatal and arm runtime protection only after runtime.start() succeeds.

Useful? React with 👍 / 👎.

return installProcessErrorHandlers(processObject, controller);
}

function main() {
const parentPort = process.parentPort;
if (!parentPort) {
throw new Error("Terminal worker requires process.parentPort");
}

installWorkerProcessErrorGuards(parentPort);

const sessions = new Map();
const sftpClients = new Map();
const { createTerminalDataPipeline } = require("./terminalDataPipeline.cjs");
Expand Down Expand Up @@ -376,6 +428,7 @@ if (require.main === module) {

module.exports = {
createWorkerSender,
installWorkerProcessErrorGuards,
createZmodemDownloadDirectorySelector,
createZmodemUploadFileSelector,
normalizeParentPortMessage,
Expand Down
78 changes: 78 additions & 0 deletions electron/terminalWorker/process.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ const assert = require("node:assert/strict");
const crypto = require("node:crypto");
const test = require("node:test");

const { EventEmitter } = require("node:events");

const {
createZmodemDownloadDirectorySelector,
createZmodemUploadFileSelector,
installWorkerProcessErrorGuards,
normalizeParentPortMessage,
registerExternalSessionHandlers,
} = require("./process.cjs");
Expand Down Expand Up @@ -217,3 +220,78 @@ test("external plugin sessions stream auto-save logs through output and lifecycl
true,
);
});

test("worker process guards keep a stray ssh2 transport error from killing every session", () => {
const parentPort = createParentPort();
const fakeProcess = new EventEmitter();
const uninstall = installWorkerProcessErrorGuards(parentPort, fakeProcess);

const err = new Error("Keepalive timeout");
err.level = "client-timeout";
// Before the guards existed this reached Node's default handler and exited
// the utilityProcess with code 1, dropping every live session at once.
assert.doesNotThrow(() => fakeProcess.emit("uncaughtException", err));

uninstall();
});

test("worker process guards suppress generic runtime errors instead of exiting", () => {
const parentPort = createParentPort();
const fakeProcess = new EventEmitter();
const uninstall = installWorkerProcessErrorGuards(parentPort, fakeProcess);

// The worker is armed as "runtime started" the moment it is forked, so even
// an error with no network classification must not take the process down.
assert.doesNotThrow(() => fakeProcess.emit("uncaughtException", new Error("boom")));
assert.doesNotThrow(() => fakeProcess.emit("unhandledRejection", new Error("rejected")));

const reports = parentPort.messages.filter((m) => m.kind === "worker-process-error");
assert.equal(reports.length, 2);
assert.deepEqual(
reports.map((r) => r.source),
["uncaughtException", "unhandledRejection"],
);
assert.equal(reports[0].message, "boom");
assert.ok(typeof reports[0].stack === "string" && reports[0].stack.length > 0);

uninstall();
});

test("worker process guards report socket error codes to the parent", () => {
const parentPort = createParentPort();
const fakeProcess = new EventEmitter();
const uninstall = installWorkerProcessErrorGuards(parentPort, fakeProcess);

const err = new Error("socket hang up");
err.code = "ECONNRESET";
fakeProcess.emit("uncaughtException", err);

const report = parentPort.messages.find((m) => m.kind === "worker-process-error");
assert.ok(report);
assert.equal(report.code, "ECONNRESET");

uninstall();
});

test("worker process guards ignore benign stream teardown without reporting", () => {
const parentPort = createParentPort();
const fakeProcess = new EventEmitter();
const uninstall = installWorkerProcessErrorGuards(parentPort, fakeProcess);

const err = new Error("write EPIPE");
err.code = "EPIPE";
assert.doesNotThrow(() => fakeProcess.emit("uncaughtException", err));
assert.equal(parentPort.messages.filter((m) => m.kind === "worker-process-error").length, 0);

uninstall();
});

test("worker process guards can be uninstalled", () => {
const parentPort = createParentPort();
const fakeProcess = new EventEmitter();
const uninstall = installWorkerProcessErrorGuards(parentPort, fakeProcess);
uninstall();

assert.equal(fakeProcess.listenerCount("uncaughtException"), 0);
assert.equal(fakeProcess.listenerCount("unhandledRejection"), 0);
});