From e56ddd10c6e6635eae4e253c901b58be7508c729 Mon Sep 17 00:00:00 2001
From: Gergo Magyar <gergomagyar@icloud.com>
Date: Sun, 21 Jun 2026 08:55:43 +0000
Subject: [PATCH] fix(server): report analyze-worker errors over IPC, never
 swallow (#2264 P3)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The worker's send() swallowed IPC failures (and a prior pass logged them to
stderr). Per review, all worker errors must be reported back to the parent over
the existing IPC channel (send({ type: 'error' })) and nothing silently dropped.

- send() no longer catches: a dead channel (ERR_IPC_CHANNEL_CLOSED) throws
  instead of being swallowed.
- Every handler (uncaughtException, unhandledRejection, SIGTERM, the analysis
  message handler) reports its error via send() in try and schedules process.exit
  in finally, so a throw from send() can no longer skip the exit and wedge the
  worker — the P3 'schedule the exit so it always fires' fix, without a swallow.
- SIGTERM cleanup failures are now reported to the parent instead of an empty
  catch {}.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm
---
 gitnexus/src/server/analyze-worker.ts | 77 ++++++++++++++++-----------
 1 file changed, 47 insertions(+), 30 deletions(-)

diff --git a/gitnexus/src/server/analyze-worker.ts b/gitnexus/src/server/analyze-worker.ts
index b00c66acf..6dd1f4e21 100644
--- a/gitnexus/src/server/analyze-worker.ts
+++ b/gitnexus/src/server/analyze-worker.ts
@@ -44,34 +44,48 @@ export interface ErrorMessage {
 export type WorkerMessage = ProgressMessage | CompleteMessage | ErrorMessage;
 
 function send(msg: WorkerMessage) {
-  try {
-    process.send?.(msg);
-  } catch {
-    // The parent may have already disconnected the IPC channel
-    // (ERR_IPC_CHANNEL_CLOSED). Swallow so a failed notification can't escape the
-    // message/SIGTERM handlers and skip their scheduled process.exit (#2264 review
-    // P3) — a vanished child is treated as a failure by the parent regardless.
-  }
+  // No try/catch: if the IPC channel is gone, process.send throws
+  // (ERR_IPC_CHANNEL_CLOSED) and that failure must NOT be swallowed. Every caller
+  // schedules its process.exit inside a `finally`, so a throw here still tears the
+  // worker down deterministically instead of wedging the event loop (#2264 P3).
+  process.send?.(msg);
 }
 
-// Catch uncaught exceptions and unhandled rejections — report to parent
-process.on('uncaughtException', (err) => {
-  send({ type: 'error', message: err?.message || 'Uncaught exception in worker' });
-  setTimeout(() => process.exit(1), 500);
-});
-
-process.on('unhandledRejection', (reason: any) => {
-  send({ type: 'error', message: reason?.message || 'Unhandled rejection in worker' });
-  setTimeout(() => process.exit(1), 500);
-});
-
-// Handle graceful shutdown — notify parent before exit
-process.on('SIGTERM', async () => {
-  send({ type: 'error', message: 'Analysis cancelled (worker received SIGTERM)' });
+// Catch uncaught exceptions and unhandled rejections — report them to the parent
+// over IPC (the same channel the analysis path uses), then exit. The report runs
+// in `try` and the exit in `finally` so a throw from send() on a closed channel
+// can't skip the exit and leave the worker wedged (#2264 review P3).
+process.on('uncaughtException', (err: unknown) => {
   try {
+    const message = err instanceof Error ? err.message : 'Uncaught exception in worker';
+    send({ type: 'error', message });
+  } finally {
+    setTimeout(() => process.exit(1), 500);
+  }
+});
+
+process.on('unhandledRejection', (reason: unknown) => {
+  try {
+    const message = reason instanceof Error ? reason.message : 'Unhandled rejection in worker';
+    send({ type: 'error', message });
+  } finally {
+    setTimeout(() => process.exit(1), 500);
+  }
+});
+
+// Handle graceful shutdown — notify the parent over IPC, then exit. A cleanup
+// failure is reported to the parent (not swallowed), and the exit lives in
+// `finally` so it always fires even if a send() throws on a closed channel.
+process.on('SIGTERM', async () => {
+  try {
+    send({ type: 'error', message: 'Analysis cancelled (worker received SIGTERM)' });
     await closeLbug();
-  } catch {}
-  process.exit(0);
+  } catch (err: unknown) {
+    const message = err instanceof Error ? err.message : 'Worker cleanup failed during SIGTERM';
+    send({ type: 'error', message });
+  } finally {
+    process.exit(0);
+  }
 });
 
 // Listen for start command from parent — guarded against re-entry
@@ -105,11 +119,14 @@ process.on('message', async (msg: StartMessage) => {
     // a BigInt/circular value would throw and mis-report this success as a
     // failure). See analyze-worker-ipc.ts.
     send({ type: 'complete', result: projectAnalyzeResultForIpc(result) });
-  } catch (err: any) {
-    send({ type: 'error', message: err?.message || 'Analysis failed' });
+  } catch (err: unknown) {
+    // Report the failure to the parent over IPC (the parent surfaces the message).
+    const message = err instanceof Error ? err.message : 'Analysis failed';
+    send({ type: 'error', message });
+  } finally {
+    // LadybugDB's native module prevents clean exit — force it (same reason the
+    // CLI uses process.exit(0)). In `finally` so the exit still fires even if the
+    // error report above throws on a closed IPC channel (#2264 review P3).
+    setTimeout(() => process.exit(0), 500);
   }
-
-  // LadybugDB's native module prevents clean exit — force it
-  // (same reason the CLI uses process.exit(0))
-  setTimeout(() => process.exit(0), 500);
 });