CL-7221 sidecars top-level SIGTERM/SIGINT handler has no error

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<void> {
   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);
+});

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<void>;
+  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<void> {
+  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<void>,
+  onRejection: (error: unknown) => void,
+): void {
+  void work.catch(onRejection);
+}

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);
+  }
+});