diff --git a/electron/bridges/terminalWorkerManager.cjs b/electron/bridges/terminalWorkerManager.cjs index 9fa77044b7..f1a71613ed 100644 --- a/electron/bridges/terminalWorkerManager.cjs +++ b/electron/bridges/terminalWorkerManager.cjs @@ -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(); @@ -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 {} @@ -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); @@ -2055,6 +2070,7 @@ function createTerminalWorkerManager(options = {}) { supersedingSessionGenerations.clear(); closeAllUrgentInputPorts(); terminalInterceptorWarningListeners.clear(); + workerProcessErrorListeners.clear(); sessionOwnedListeners.clear(); sessionClosedListeners.clear(); workerExitListeners.clear(); @@ -2088,6 +2104,7 @@ function createTerminalWorkerManager(options = {}) { attachTerminalInterceptor, detachTerminalInterceptor, onTerminalInterceptorWarning, + onWorkerProcessError, onSessionOwned, onSessionClosed, onWorkerExit, diff --git a/electron/bridges/terminalWorkerManager.test.cjs b/electron/bridges/terminalWorkerManager.test.cjs index 086f409dd9..7fc6b3a7b7 100644 --- a/electron/bridges/terminalWorkerManager.test.cjs +++ b/electron/bridges/terminalWorkerManager.test.cjs @@ -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); +}); diff --git a/electron/main/registerBridges.cjs b/electron/main/registerBridges.cjs index e4aed54563..cebfab3624 100644 --- a/electron/main/registerBridges.cjs +++ b/electron/main/registerBridges.cjs @@ -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 { diff --git a/electron/terminalWorker/process.cjs b/electron/terminalWorker/process.cjs index 2dd4fc6b68..94d53980d3 100644 --- a/electron/terminalWorker/process.cjs +++ b/electron/terminalWorker/process.cjs @@ -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. @@ -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 }); + 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"); @@ -376,6 +428,7 @@ if (require.main === module) { module.exports = { createWorkerSender, + installWorkerProcessErrorGuards, createZmodemDownloadDirectorySelector, createZmodemUploadFileSelector, normalizeParentPortMessage, diff --git a/electron/terminalWorker/process.test.cjs b/electron/terminalWorker/process.test.cjs index 618cc12a5d..0579473b48 100644 --- a/electron/terminalWorker/process.test.cjs +++ b/electron/terminalWorker/process.test.cjs @@ -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"); @@ -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); +});