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..feb872bfe 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,65 @@ 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; + report?: typeof reportError; +}; + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +export async function runSidecarShutdown({ + signal, + close, + drain, + drainTimeoutMs, + exit, + log, + report = reportError, +}: RunSidecarShutdownArgs): Promise { + log.info`Received ${signal}; draining before exit`; + try { + 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") { + // 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); + return; + } + 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); + } +} + +export function attachShutdownRejectionHandler( + work: Promise, + onRejection: (error: unknown) => void, +): void { + void work.catch(onRejection); +} diff --git a/apps/sidecar/test/shutdown.test.ts b/apps/sidecar/test/shutdown.test.ts index 3c3678e25..a7baad800 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,95 @@ 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 reports through reportError and exits 1 without rejecting", async () => { + const exits: number[] = []; + const error = new Error("drain fault"); + 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 () => { + 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); + } +});