From 39873c2e6e87bd45f416617d4708993f97e8cc0a Mon Sep 17 00:00:00 2001
From: Neil <neil@stably.ai>
Date: Thu, 10 Sep 2026 17:54:03 -0700
Subject: [PATCH] fix(relay): a throwing retry step must not kill the retry
 chain
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`RelayRetrySchedule` ran the caller's recovery step bare inside its timer and
resolved `armed` only on the line after it. Injected a synchronous throw:
the error escaped the timer as an uncaughtException, `armed.resolve()` never
ran, and the schedule was left with no armed timer — so every waiter on
`settled` parked on a promise nothing would ever settle and nothing re-entered
the chain.

The reachable trigger is the origin pool's drain recovery, which calls
`onStatus('draining')` straight through to `webContents.send`. `state.mainWindow`
is nulled only on `'closed'`, so between destroy and that event the send throws
`Object has been destroyed` — every other `webContents.send` under
`src/main/startup/` already guards with `isDestroyed()`. Guarded here too, so
the trigger is removed as well as contained.
---
 .../relay/relay-retry-schedule.test.ts        | 26 +++++++++++++++++++
 .../runtime/relay/relay-retry-schedule.ts     | 14 ++++++++--
 src/main/startup/main-process-relay-status.ts |  7 ++++-
 3 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/src/main/runtime/relay/relay-retry-schedule.test.ts b/src/main/runtime/relay/relay-retry-schedule.test.ts
index aff1ce6319a..db99e31382e 100644
--- a/src/main/runtime/relay/relay-retry-schedule.test.ts
+++ b/src/main/runtime/relay/relay-retry-schedule.test.ts
@@ -27,6 +27,32 @@ describe('RelayRetrySchedule', () => {
     expect(schedule.settled).toBeNull()
   })
 
+  // Why this matters beyond the throw itself: the retry callback is the caller's whole recovery
+  // step (the origin pool's `handleDrain` reaches `onStatus` -> `webContents.send`). A throw used
+  // to escape the timer AND skip the resolve, parking every `settled` waiter on a promise nothing
+  // would settle while the schedule held no armed timer — dead with no re-entry.
+  it('settles and stays reschedulable when the retry throws', async () => {
+    vi.useFakeTimers()
+    const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
+    const schedule = new RelayRetrySchedule(() => 0.5)
+    schedule.schedule(0, () => {
+      throw new Error('recovery step blew up')
+    })
+    const settled = schedule.settled!
+
+    await vi.advanceTimersByTimeAsync(500)
+
+    await expect(settled).resolves.toBeUndefined()
+    expect(schedule.pending).toBe(false)
+    expect(consoleError).toHaveBeenCalled()
+
+    const next = vi.fn()
+    schedule.schedule(0, next)
+    await vi.advanceTimersByTimeAsync(10_000)
+    expect(next).toHaveBeenCalledTimes(1)
+    consoleError.mockRestore()
+  })
+
   it('settles on cancel without running the retry', async () => {
     vi.useFakeTimers()
     const schedule = new RelayRetrySchedule(() => 0.5)
diff --git a/src/main/runtime/relay/relay-retry-schedule.ts b/src/main/runtime/relay/relay-retry-schedule.ts
index dcab1069e37..a3ef621d454 100644
--- a/src/main/runtime/relay/relay-retry-schedule.ts
+++ b/src/main/runtime/relay/relay-retry-schedule.ts
@@ -28,8 +28,18 @@ export class RelayRetrySchedule {
     this.timer = setTimeout(() => {
       this.timer = null
       this.armed = null
-      retry()
-      armed.resolve()
+      // Why contained: `retry` is the caller's whole recovery step, run bare inside a timer. A
+      // synchronous throw there escaped as an uncaughtException AND skipped the resolve, so every
+      // waiter on `settled` parked on a promise nothing would ever settle, while the schedule was
+      // left with no armed timer — the chain dead with no re-entry. Waking the waiters is right
+      // either way: the retry is over, however it ended.
+      try {
+        retry()
+      } catch (error) {
+        console.error('[relay] scheduled retry threw:', error)
+      } finally {
+        armed.resolve()
+      }
     }, delayMs)
   }
 
diff --git a/src/main/startup/main-process-relay-status.ts b/src/main/startup/main-process-relay-status.ts
index 1dd254df8b3..a2a80cb2bd0 100644
--- a/src/main/startup/main-process-relay-status.ts
+++ b/src/main/startup/main-process-relay-status.ts
@@ -14,5 +14,10 @@ export function publishDesktopRelayStatus(
 ): void {
   state.desktopRelayStatus = status
   state.desktopRelayCellUrl = cellUrl
-  state.mainWindow?.webContents.send('mobile:relayStatusChanged', getDesktopRelayStatus())
+  // Why isDestroyed and not just the optional chain: `state.mainWindow` is nulled on 'closed',
+  // so between destroy and that event `webContents.send` throws "Object has been destroyed".
+  // This runs from inside a bare `setTimeout` recovery step, where a throw killed the retry chain.
+  if (state.mainWindow && !state.mainWindow.isDestroyed()) {
+    state.mainWindow.webContents.send('mobile:relayStatusChanged', getDesktopRelayStatus())
+  }
 }