From 3f1080da8b072a6095978070e9506e5b00711740 Mon Sep 17 00:00:00 2001 From: Xinhong Zhou <59351302+zxhggg@users.noreply.github.com> Date: Sat, 29 Aug 2026 17:18:12 +0800 Subject: [PATCH 1/2] fix(terminal): guard the terminal worker against process-level errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal worker utilityProcess owns every terminal, SSH, SFTP, and port-forwarding session, but it installed no uncaughtException or unhandledRejection handler. Any stray async throw inside it — a socket that errors after its `once("error")` handler was consumed, a throw from a setTimeout callback, a rejected promise on a fire-and-forget IPC listener — exited the process with code 1. terminalWorkerManager.handleExit() then tore down *every* live session at once and reported "Terminal worker exited with code 1" to each of them. The main process already installs exactly these guards, for exactly this reason; processErrorGuards.cjs even says an ssh2 connection-level error is "never a reason to kill the entire multi-session app". Once sessions moved into the worker, the worker became the process that has to survive them. Install the same guards there, arming runtime protection immediately since the worker has no window lifecycle to wait on, and forward suppressed errors to the parent so main can record them. --- electron/terminalWorker/process.cjs | 53 ++++++++++++++++ electron/terminalWorker/process.test.cjs | 78 ++++++++++++++++++++++++ 2 files changed, 131 insertions(+) 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); +}); From b3b7b046c06f08cc143e484c8d3fe8c903b56153 Mon Sep 17 00:00:00 2001 From: Xinhong Zhou <59351302+zxhggg@users.noreply.github.com> Date: Sat, 29 Aug 2026 17:18:13 +0800 Subject: [PATCH 2/2] fix(terminal): record terminal worker crashes in the crash log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A worker exit dropped every session at once but left no forensic trace: handleExit() never reached crashLogBridge, and the only onWorkerExit listener was port forwarding's tunnel-status cleanup. Bug reports could therefore only carry the symptom ("Terminal worker exited with code 1"), never the cause — the worker's stderr goes to the main process's inherited stderr, which is discarded under the Windows GUI subsystem. Fan worker-process-error reports out to main and write both those and worker exits to the crash log. --- electron/bridges/terminalWorkerManager.cjs | 17 +++++++++++ .../bridges/terminalWorkerManager.test.cjs | 30 +++++++++++++++++++ electron/main/registerBridges.cjs | 19 ++++++++++++ 3 files changed, 66 insertions(+) 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 {