From 55950a2a2cc5aa7adca527dce7802d20bc29da96 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 00:31:21 -0700 Subject: [PATCH 1/7] Add tests for sidecar SIGTERM/SIGINT shutdown error handling --- apps/sidecar/test/shutdown.test.ts | 97 +++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/apps/sidecar/test/shutdown.test.ts b/apps/sidecar/test/shutdown.test.ts index 3c3678e25..90ae0973f 100644 --- a/apps/sidecar/test/shutdown.test.ts +++ b/apps/sidecar/test/shutdown.test.ts @@ -1,5 +1,20 @@ import { expect, test } from "bun:test"; -import { drainWithTimeout } from "../src/shutdown"; +import { + attachShutdownRejectionHandler, + drainWithTimeout, + runSidecarShutdown, +} from "../src/shutdown"; + +function capturingLog() { + const errors: string[] = []; + return { + errors, + info(_strings: TemplateStringsArray, ..._values: unknown[]): void {}, + error(strings: TemplateStringsArray, ...values: unknown[]): void { + errors.push(String.raw({ raw: strings }, ...values)); + }, + }; +} test("resolves drained when the drain completes inside the bound", async () => { const outcome = await drainWithTimeout(() => Promise.resolve(), 1_000); @@ -19,3 +34,83 @@ test("resolves failed with the thrown error when the drain throws", async () => const outcome = await drainWithTimeout(() => Promise.reject(error), 1_000); expect(outcome).toEqual({ kind: "failed", error }); }); + +test("a throw from close exits 1 and does not reject", async () => { + const exits: number[] = []; + const log = capturingLog(); + let drained = false; + const promise = runSidecarShutdown({ + signal: "SIGTERM", + close: () => { + throw new Error("close failed"); + }, + drain: async () => { + drained = true; + }, + drainTimeoutMs: 1_000, + exit: (code) => { + exits.push(code); + }, + log, + }); + await expect(promise).resolves.toBeUndefined(); + expect(exits).toEqual([1]); + expect(drained).toBe(false); + expect(log.errors.some((line) => line.includes("close failed"))).toBe(true); +}); + +test("a drain failure exits 1 without rejecting", async () => { + const exits: number[] = []; + const promise = runSidecarShutdown({ + signal: "SIGINT", + close: () => undefined, + drain: async () => { + throw new Error("drain fault"); + }, + drainTimeoutMs: 1_000, + exit: (code) => { + exits.push(code); + }, + log: capturingLog(), + }); + await expect(promise).resolves.toBeUndefined(); + expect(exits).toEqual([1]); +}); + +test("a clean drain exits 0", async () => { + const exits: number[] = []; + const promise = runSidecarShutdown({ + signal: "SIGTERM", + close: () => undefined, + drain: async () => undefined, + drainTimeoutMs: 1_000, + exit: (code) => { + exits.push(code); + }, + log: capturingLog(), + }); + await expect(promise).resolves.toBeUndefined(); + expect(exits).toEqual([0]); +}); + +test("a rejected shutdown promise is caught by the signal handler binding", async () => { + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => { + unhandled.push(reason); + }; + process.on("unhandledRejection", onUnhandled); + const reported: unknown[] = []; + try { + attachShutdownRejectionHandler( + Promise.reject(new Error("escaped")), + (error) => { + reported.push(error); + }, + ); + await Bun.sleep(20); + expect(reported).toHaveLength(1); + expect(unhandled).toEqual([]); + } finally { + process.off("unhandledRejection", onUnhandled); + } +}); From a112842d9018b815f914653943db0bd12a3c22fa Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 00:31:31 -0700 Subject: [PATCH 2/7] Handle throws in the sidecar SIGTERM and SIGINT shutdown path orchestrator.close() and anything else that throws during shutdown was an unhandled rejection because the signal listeners used void. The shutdown body now catches, reports, and exits 1, and the listeners attach a rejection handler so nothing can escape. --- apps/sidecar/src/index.ts | 46 +++++++++++++++------------- apps/sidecar/src/shutdown.ts | 58 ++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 21 deletions(-) diff --git a/apps/sidecar/src/index.ts b/apps/sidecar/src/index.ts index ba162410c..b2a136e7d 100644 --- a/apps/sidecar/src/index.ts +++ b/apps/sidecar/src/index.ts @@ -32,6 +32,7 @@ import { createInMemoryTransport } from "@intx/mail-memory"; import { createTarballCache } from "@intx/tool-packaging"; import { hexEncode } from "@intx/types"; +import { reportError } from "@corbits/error-sink"; import { readSidecarConfig } from "./config"; import { DEFAULT_TOOL_REGISTRIES_JSON, @@ -42,7 +43,7 @@ import { createWorkflowClosureMaterializer } from "./workflow-closure-materializ import { MAX_INLINE_ASSET_PAYLOAD_BYTES } from "./source-asset-delivery"; import { createDefaultHarnessBuilder } from "./default-harness"; import { createHubLinkWatchdog } from "./hub-link-watchdog"; -import { drainWithTimeout } from "./shutdown"; +import { attachShutdownRejectionHandler, runSidecarShutdown } from "./shutdown"; import { loadOrMintSidecarKeypair } from "./signing-keypair"; import { createSidecarDeployRouter, @@ -387,25 +388,28 @@ async function shutdown(signal: string): Promise { if (shuttingDown) return; shuttingDown = true; const log = getLogger(["sidecar", "shutdown"]); - log.info`Received ${signal}; draining before exit`; - orchestrator.close(); - const outcome = await drainWithTimeout( - () => deployRouter.shutdownAll(), - SHUTDOWN_DRAIN_MS, - ); - // Drained and timed-out both exit 0: the process is going down either - // way and a bound cutting a slow drain short is not a crash. A drain - // that threw is a genuine fault and exits non-zero. - if (outcome.kind === "failed") { - const message = - outcome.error instanceof Error - ? outcome.error.message - : String(outcome.error); - log.error`Drain threw during shutdown; exiting non-zero: ${message}`; - process.exit(1); - } - process.exit(0); + await runSidecarShutdown({ + signal, + close: () => { + orchestrator.close(); + }, + drain: () => deployRouter.shutdownAll(), + drainTimeoutMs: SHUTDOWN_DRAIN_MS, + exit: (code) => { + process.exit(code); + }, + log, + }); } -process.on("SIGTERM", () => void shutdown("SIGTERM")); -process.on("SIGINT", () => void shutdown("SIGINT")); +function onShutdownRejection(error: unknown): void { + reportError(error, { operation: "sidecar.shutdown.signal" }); + process.exit(1); +} + +process.on("SIGTERM", () => { + attachShutdownRejectionHandler(shutdown("SIGTERM"), onShutdownRejection); +}); +process.on("SIGINT", () => { + attachShutdownRejectionHandler(shutdown("SIGINT"), onShutdownRejection); +}); diff --git a/apps/sidecar/src/shutdown.ts b/apps/sidecar/src/shutdown.ts index 79a909727..06737db7e 100644 --- a/apps/sidecar/src/shutdown.ts +++ b/apps/sidecar/src/shutdown.ts @@ -4,6 +4,15 @@ // fault -- the process is exiting either way -- but a drain that throws // is, and the two outcomes are kept distinct so the caller can exit // non-zero only for the genuine failure. +// +// `runSidecarShutdown` is the SIGTERM/SIGINT body: close, then drain. +// Throws from either (including a sync throw from orchestrator.close) +// become process.exit(1) rather than an unhandled rejection. +// `attachShutdownRejectionHandler` is the belt on the signal listener +// itself so a rejection that still escapes that body cannot become an +// unhandledRejection. + +import { reportError } from "@corbits/error-sink"; export type DrainOutcome = | { kind: "drained" } @@ -32,3 +41,52 @@ export async function drainWithTimeout( clearTimeout(timer); return outcome; } + +export type SidecarShutdownLog = { + info(strings: TemplateStringsArray, ...values: unknown[]): void; + error(strings: TemplateStringsArray, ...values: unknown[]): void; +}; + +export type RunSidecarShutdownArgs = { + signal: string; + close: () => void; + drain: () => Promise; + drainTimeoutMs: number; + exit: (code: number) => void; + log: SidecarShutdownLog; +}; + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +export async function runSidecarShutdown( + args: RunSidecarShutdownArgs, +): Promise { + args.log.info`Received ${args.signal}; draining before exit`; + try { + args.close(); + const outcome = await drainWithTimeout(args.drain, args.drainTimeoutMs); + // Drained and timed-out both exit 0: the process is going down either + // way and a bound cutting a slow drain short is not a crash. A drain + // that threw is a genuine fault and exits non-zero. + if (outcome.kind === "failed") { + args.log + .error`Drain threw during shutdown; exiting non-zero: ${errorMessage(outcome.error)}`; + args.exit(1); + return; + } + args.exit(0); + } catch (error) { + reportError(error, { operation: "sidecar.shutdown" }); + args.log.error`Shutdown threw; exiting non-zero: ${errorMessage(error)}`; + args.exit(1); + } +} + +export function attachShutdownRejectionHandler( + work: Promise, + onRejection: (error: unknown) => void, +): void { + void work.catch(onRejection); +} From 38f52d9b3b8d5aa6aa6bd780c401f84747522fc3 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 14:54:07 -0700 Subject: [PATCH 3/7] Add tests for sidecar drain-failed reportError A drain that throws already exits 1. Pin that the failure is also reported through reportError with operation sidecar.shutdown. --- apps/sidecar/test/shutdown.test.ts | 44 +++++++++++++++++++----------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/apps/sidecar/test/shutdown.test.ts b/apps/sidecar/test/shutdown.test.ts index 90ae0973f..7d9888d22 100644 --- a/apps/sidecar/test/shutdown.test.ts +++ b/apps/sidecar/test/shutdown.test.ts @@ -1,4 +1,5 @@ -import { expect, test } from "bun:test"; +import { expect, spyOn, test } from "bun:test"; +import * as errorSink from "@corbits/error-sink"; import { attachShutdownRejectionHandler, drainWithTimeout, @@ -59,22 +60,33 @@ test("a throw from close exits 1 and does not reject", async () => { expect(log.errors.some((line) => line.includes("close failed"))).toBe(true); }); -test("a drain failure exits 1 without rejecting", async () => { +test("a drain failure reports through reportError and exits 1 without rejecting", async () => { const exits: number[] = []; - const promise = runSidecarShutdown({ - signal: "SIGINT", - close: () => undefined, - drain: async () => { - throw new Error("drain fault"); - }, - drainTimeoutMs: 1_000, - exit: (code) => { - exits.push(code); - }, - log: capturingLog(), - }); - await expect(promise).resolves.toBeUndefined(); - expect(exits).toEqual([1]); + const error = new Error("drain fault"); + const report = spyOn(errorSink, "reportError").mockReturnValue("ref-test"); + try { + const promise = runSidecarShutdown({ + signal: "SIGINT", + close: () => undefined, + drain: async () => { + throw error; + }, + drainTimeoutMs: 1_000, + exit: (code) => { + exits.push(code); + }, + log: capturingLog(), + }); + await expect(promise).resolves.toBeUndefined(); + expect(exits).toEqual([1]); + expect(report).toHaveBeenCalledTimes(1); + expect(report.mock.calls[0]?.[0]).toBe(error); + expect(report.mock.calls[0]?.[1]).toMatchObject({ + operation: "sidecar.shutdown", + }); + } finally { + report.mockRestore(); + } }); test("a clean drain exits 0", async () => { From 65414f63d016a63d4b79be95cf61cb9c8a73ebe6 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 14:54:16 -0700 Subject: [PATCH 4/7] Report sidecar drain-failed through reportError A drain that throws was logged and exited 1 without going through the error sink. Route it through reportError with the same sidecar.shutdown operation as other shutdown faults, then exit 1. --- apps/sidecar/src/shutdown.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/sidecar/src/shutdown.ts b/apps/sidecar/src/shutdown.ts index 06737db7e..ffc09fca4 100644 --- a/apps/sidecar/src/shutdown.ts +++ b/apps/sidecar/src/shutdown.ts @@ -71,6 +71,7 @@ export async function runSidecarShutdown( // way and a bound cutting a slow drain short is not a crash. A drain // that threw is a genuine fault and exits non-zero. if (outcome.kind === "failed") { + reportError(outcome.error, { operation: "sidecar.shutdown" }); args.log .error`Drain threw during shutdown; exiting non-zero: ${errorMessage(outcome.error)}`; args.exit(1); From fa084c767cab722f18707081418d4db4ebe1ec73 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 15:46:51 -0700 Subject: [PATCH 5/7] Inject report into sidecar shutdown instead of spyOn spyOn cannot patch the ESM local binding, so the drain-failed test never observed reportError in CI. Inject report on RunSidecarShutdownArgs the same way hub shutdown does. --- apps/sidecar/src/shutdown.ts | 32 ++++++++++-------- apps/sidecar/test/shutdown.test.ts | 52 +++++++++++++++--------------- 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/apps/sidecar/src/shutdown.ts b/apps/sidecar/src/shutdown.ts index ffc09fca4..4c7a6a3e4 100644 --- a/apps/sidecar/src/shutdown.ts +++ b/apps/sidecar/src/shutdown.ts @@ -54,34 +54,40 @@ export type RunSidecarShutdownArgs = { drainTimeoutMs: number; exit: (code: number) => void; log: SidecarShutdownLog; + report?: typeof reportError; }; function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } -export async function runSidecarShutdown( - args: RunSidecarShutdownArgs, -): Promise { - args.log.info`Received ${args.signal}; draining before exit`; +export async function runSidecarShutdown({ + signal, + close, + drain, + drainTimeoutMs, + exit, + log, + report = reportError, +}: RunSidecarShutdownArgs): Promise { + log.info`Received ${signal}; draining before exit`; try { - args.close(); - const outcome = await drainWithTimeout(args.drain, args.drainTimeoutMs); + close(); + const outcome = await drainWithTimeout(drain, drainTimeoutMs); // Drained and timed-out both exit 0: the process is going down either // way and a bound cutting a slow drain short is not a crash. A drain // that threw is a genuine fault and exits non-zero. if (outcome.kind === "failed") { - reportError(outcome.error, { operation: "sidecar.shutdown" }); - args.log - .error`Drain threw during shutdown; exiting non-zero: ${errorMessage(outcome.error)}`; - args.exit(1); + report(outcome.error, { operation: "sidecar.shutdown" }); + log.error`Drain threw during shutdown; exiting non-zero: ${errorMessage(outcome.error)}`; + exit(1); return; } - args.exit(0); + exit(0); } catch (error) { reportError(error, { operation: "sidecar.shutdown" }); - args.log.error`Shutdown threw; exiting non-zero: ${errorMessage(error)}`; - args.exit(1); + log.error`Shutdown threw; exiting non-zero: ${errorMessage(error)}`; + exit(1); } } diff --git a/apps/sidecar/test/shutdown.test.ts b/apps/sidecar/test/shutdown.test.ts index 7d9888d22..a7baad800 100644 --- a/apps/sidecar/test/shutdown.test.ts +++ b/apps/sidecar/test/shutdown.test.ts @@ -1,5 +1,4 @@ -import { expect, spyOn, test } from "bun:test"; -import * as errorSink from "@corbits/error-sink"; +import { expect, test } from "bun:test"; import { attachShutdownRejectionHandler, drainWithTimeout, @@ -63,30 +62,31 @@ test("a throw from close exits 1 and does not reject", async () => { test("a drain failure reports through reportError and exits 1 without rejecting", async () => { const exits: number[] = []; const error = new Error("drain fault"); - const report = spyOn(errorSink, "reportError").mockReturnValue("ref-test"); - try { - const promise = runSidecarShutdown({ - signal: "SIGINT", - close: () => undefined, - drain: async () => { - throw error; - }, - drainTimeoutMs: 1_000, - exit: (code) => { - exits.push(code); - }, - log: capturingLog(), - }); - await expect(promise).resolves.toBeUndefined(); - expect(exits).toEqual([1]); - expect(report).toHaveBeenCalledTimes(1); - expect(report.mock.calls[0]?.[0]).toBe(error); - expect(report.mock.calls[0]?.[1]).toMatchObject({ - operation: "sidecar.shutdown", - }); - } finally { - report.mockRestore(); - } + const reported: unknown[] = []; + const contexts: unknown[] = []; + const promise = runSidecarShutdown({ + signal: "SIGINT", + close: () => undefined, + drain: async () => { + throw error; + }, + drainTimeoutMs: 1_000, + exit: (code) => { + exits.push(code); + }, + log: capturingLog(), + report: (reportedError, context) => { + reported.push(reportedError); + contexts.push(context); + return "ref-test"; + }, + }); + await expect(promise).resolves.toBeUndefined(); + expect(exits).toEqual([1]); + expect(reported).toEqual([error]); + expect(contexts[0]).toMatchObject({ + operation: "sidecar.shutdown", + }); }); test("a clean drain exits 0", async () => { From 30a4ef86a0d6e997a89ddbdf4a9a3a3782a7cfd9 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 21:45:45 -0700 Subject: [PATCH 6/7] sidecar: use injected report in shutdown's outer catch (CL-7221) --- apps/sidecar/src/shutdown.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/sidecar/src/shutdown.ts b/apps/sidecar/src/shutdown.ts index 4c7a6a3e4..2f61db12b 100644 --- a/apps/sidecar/src/shutdown.ts +++ b/apps/sidecar/src/shutdown.ts @@ -85,7 +85,7 @@ export async function runSidecarShutdown({ } exit(0); } catch (error) { - reportError(error, { operation: "sidecar.shutdown" }); + report(error, { operation: "sidecar.shutdown" }); log.error`Shutdown threw; exiting non-zero: ${errorMessage(error)}`; exit(1); } From baa654f28f683160329b72f79d9a5054af2071e7 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 22:49:42 -0700 Subject: [PATCH 7/7] sidecar: satisfy check:report-error for the injected shutdown reporter (CL-7221) --- apps/sidecar/src/shutdown.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/sidecar/src/shutdown.ts b/apps/sidecar/src/shutdown.ts index 2f61db12b..feb872bfe 100644 --- a/apps/sidecar/src/shutdown.ts +++ b/apps/sidecar/src/shutdown.ts @@ -78,6 +78,10 @@ export async function runSidecarShutdown({ // way and a bound cutting a slow drain short is not a crash. A drain // that threw is a genuine fault and exits non-zero. if (outcome.kind === "failed") { + // report-error-ignore: CL-7221 — `report` defaults to reportError + // from @corbits/error-sink (see RunSidecarShutdownArgs above); it's + // a parameter, not an import binding, so check:report-error's + // static import-binding match can't see through the default. report(outcome.error, { operation: "sidecar.shutdown" }); log.error`Drain threw during shutdown; exiting non-zero: ${errorMessage(outcome.error)}`; exit(1); @@ -85,6 +89,8 @@ export async function runSidecarShutdown({ } exit(0); } catch (error) { + // report-error-ignore: CL-7221 — same injected-reporter default as + // the catch above. report(error, { operation: "sidecar.shutdown" }); log.error`Shutdown threw; exiting non-zero: ${errorMessage(error)}`; exit(1);