fix(daemon): address monitoring layer review feedback

- Replace process.exit with MONITORING_IDLE_TIMEOUT callback event
- Handle MONITORING_IDLE_TIMEOUT in SyncMachine (-> ERROR state)
- Add machine.onDone shutdown path in index.ts
- Wrap monitoring send-actions in choose guard (no-op if actor absent)
- Add reconnection storm alert cooldown (1 min) to prevent spam
- Halve idle check interval to reduce worst-case detection lag to 1.5x
- Fix stuck-processing doc comment (MAJOR alert, machine keeps running)
- Fix err.message interpolation in alert catch handlers

Author: andreabadesso

packages/daemon/__tests__/actors/MonitoringActor.test.ts

@@ -11,6 +11,8 @@ import { EventTypes } from '../../src/types/event';
 import getConfig from '../../src/config';
 import { addAlert } from '@wallet-service/common';
 
+const MONITORING_IDLE_TIMEOUT_EVENT = { type: EventTypes.MONITORING_IDLE_TIMEOUT };
+
 jest.useFakeTimers();
 jest.spyOn(global, 'setInterval');
 jest.spyOn(global, 'clearInterval');
@@ -86,7 +88,7 @@ describe('MonitoringActor', () => {
     expect(clearInterval).toHaveBeenCalledTimes(1);
   });
 
-  it('should fire an idle alert and exit after IDLE_EVENT_TIMEOUT_MS with no events', async () => {
+  it('should fire an idle alert and send MONITORING_IDLE_TIMEOUT after IDLE_EVENT_TIMEOUT_MS with no events', async () => {
     MonitoringActor(mockCallback, mockReceive, config);
     sendEvent('CONNECTED');
 
@@ -96,7 +98,8 @@ describe('MonitoringActor', () => {
 
     expect(mockAddAlert).toHaveBeenCalledTimes(1);
     expect(mockAddAlert.mock.calls[0][0]).toBe('Daemon Idle — No Events Received');
-    expect(processExitSpy).toHaveBeenCalledWith(1);
+    expect(mockCallback).toHaveBeenCalledWith(MONITORING_IDLE_TIMEOUT_EVENT);
+    expect(processExitSpy).not.toHaveBeenCalled();
   });
 
   it('should NOT fire an idle alert when events keep arriving', async () => {
@@ -112,7 +115,7 @@ describe('MonitoringActor', () => {
     expect(mockAddAlert).not.toHaveBeenCalled();
   });
 
-  it('should fire only one idle alert and exit once per idle period', async () => {
+  it('should fire only one idle alert and send MONITORING_IDLE_TIMEOUT once per idle period', async () => {
     MonitoringActor(mockCallback, mockReceive, config);
     sendEvent('CONNECTED');
 
@@ -121,33 +124,35 @@ describe('MonitoringActor', () => {
     await Promise.resolve();
 
     expect(mockAddAlert).toHaveBeenCalledTimes(1);
-    expect(processExitSpy).toHaveBeenCalledTimes(1);
-    expect(processExitSpy).toHaveBeenCalledWith(1);
+    expect(mockCallback).toHaveBeenCalledTimes(1);
+    expect(mockCallback).toHaveBeenCalledWith(MONITORING_IDLE_TIMEOUT_EVENT);
+    expect(processExitSpy).not.toHaveBeenCalled();
   });
 
-  it('should reset the idle alert flag when an event is received, allowing a second exit', async () => {
+  it('should reset the idle alert flag when an event is received, allowing a second MONITORING_IDLE_TIMEOUT', async () => {
     MonitoringActor(mockCallback, mockReceive, config);
     sendEvent('CONNECTED');
 
-    // Trigger first alert + exit
+    // Trigger first alert (interval = T/2, fires at T/2 then T — alert at T)
     jest.advanceTimersByTime(config['IDLE_EVENT_TIMEOUT_MS'] + 1);
     await Promise.resolve();
     await Promise.resolve();
     expect(mockAddAlert).toHaveBeenCalledTimes(1);
-    expect(processExitSpy).toHaveBeenCalledTimes(1);
+    expect(mockCallback).toHaveBeenCalledTimes(1);
 
-    // Receive an event — resets idleAlertFired and lastEventReceivedAt
+    // Receive an event — resets idleAlertFired and lastEventReceivedAt (~T from start)
     sendEvent('EVENT_RECEIVED');
 
-    // Advance far enough for the interval to fire when idleMs >= threshold again.
-    // The interval fires at 2T, 3T, … from start.  After EVENT_RECEIVED at ~T,
-    // the next fire where idleMs >= T is at 3T (fire at 2T gives idleMs = T-1).
+    // With interval=T/2, interval fires at 3T/2, 2T, 5T/2, … from start.
+    // The first fire where idleMs >= T after EVENT_RECEIVED is at 5T/2 (idleMs = 3T/2 - ε).
+    // Advancing 2T from here (total ~3T from start) covers 5T/2, so the second alert fires.
     jest.advanceTimersByTime(2 * config['IDLE_EVENT_TIMEOUT_MS']);
     await Promise.resolve();
     await Promise.resolve();
 
     expect(mockAddAlert).toHaveBeenCalledTimes(2);
-    expect(processExitSpy).toHaveBeenCalledTimes(2);
+    expect(mockCallback).toHaveBeenCalledTimes(2);
+    expect(processExitSpy).not.toHaveBeenCalled();
   });
 
   it('should restart the idle timer when CONNECTED is sent while already running', () => {
@@ -173,7 +178,7 @@ describe('MonitoringActor', () => {
     expect(clearTimeout).toHaveBeenCalledTimes(1);
   });
 
-  it('should fire a MAJOR alert when stuck', async () => {
+  it('should fire a MAJOR alert when stuck and NOT send MONITORING_IDLE_TIMEOUT', async () => {
     MonitoringActor(mockCallback, mockReceive, config);
     sendEvent('PROCESSING_STARTED');
 
@@ -183,6 +188,7 @@ describe('MonitoringActor', () => {
 
     expect(mockAddAlert).toHaveBeenCalledTimes(1);
     expect(mockAddAlert.mock.calls[0][0]).toBe('Daemon Stuck In Processing State');
+    // Stuck detection intentionally does not notify the machine — machine keeps running
     expect(mockCallback).not.toHaveBeenCalled();
   });
 
@@ -249,6 +255,46 @@ describe('MonitoringActor', () => {
     expect(mockAddAlert).not.toHaveBeenCalled();
   });
 
+  it('should not fire more than one storm alert within the 1-minute cooldown window', async () => {
+    MonitoringActor(mockCallback, mockReceive, config);
+
+    // Trigger threshold
+    sendEvent('RECONNECTING');
+    sendEvent('RECONNECTING');
+    sendEvent('RECONNECTING'); // threshold = 3 → first alert
+
+    await Promise.resolve();
+    expect(mockAddAlert).toHaveBeenCalledTimes(1);
+
+    // Additional reconnections within cooldown (no time advanced)
+    sendEvent('RECONNECTING');
+    sendEvent('RECONNECTING');
+
+    await Promise.resolve();
+    // Cooldown prevents a second alert
+    expect(mockAddAlert).toHaveBeenCalledTimes(1);
+  });
+
+  it('should fire another storm alert after the 1-minute cooldown expires', async () => {
+    MonitoringActor(mockCallback, mockReceive, config);
+
+    sendEvent('RECONNECTING');
+    sendEvent('RECONNECTING');
+    sendEvent('RECONNECTING'); // first alert
+
+    await Promise.resolve();
+    expect(mockAddAlert).toHaveBeenCalledTimes(1);
+
+    // Advance past the 1-minute cooldown
+    jest.advanceTimersByTime(61 * 1000);
+
+    sendEvent('RECONNECTING'); // still >= threshold in window, cooldown expired
+
+    await Promise.resolve();
+    expect(mockAddAlert).toHaveBeenCalledTimes(2);
+    expect(mockAddAlert.mock.calls[1][0]).toBe('Daemon Reconnection Storm');
+  });
+
   it('should evict old reconnections outside the storm window', async () => {
     MonitoringActor(mockCallback, mockReceive, config);
 

packages/daemon/src/actions/index.ts

@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import { assign, AssignAction, sendTo } from 'xstate';
+import { assign, AssignAction, sendTo, choose } from 'xstate';
 import { Context, Event, EventTypes, StandardFullNodeEvent } from '../types';
 import { get } from 'lodash';
 import logger from '../logger';
@@ -209,54 +209,55 @@ export const getMonitoringRefFromContext = (context: Context) => {
   return context.monitoring;
 };
 
+const monitoringIsPresent = (context: Context) => context.monitoring !== null;
+
 /*
  * Notifies the monitoring actor that the WebSocket became connected.
  */
-export const sendMonitoringConnected = sendTo(
-  getMonitoringRefFromContext,
-  { type: EventTypes.MONITORING_EVENT, event: { type: 'CONNECTED' } },
-);
+export const sendMonitoringConnected = choose([{
+  cond: monitoringIsPresent,
+  actions: sendTo(getMonitoringRefFromContext, { type: EventTypes.MONITORING_EVENT, event: { type: 'CONNECTED' } }),
+}]);
 
 /*
  * Notifies the monitoring actor that the WebSocket disconnected.
  */
-export const sendMonitoringDisconnected = sendTo(
-  getMonitoringRefFromContext,
-  { type: EventTypes.MONITORING_EVENT, event: { type: 'DISCONNECTED' } },
-);
+export const sendMonitoringDisconnected = choose([{
+  cond: monitoringIsPresent,
+  actions: sendTo(getMonitoringRefFromContext, { type: EventTypes.MONITORING_EVENT, event: { type: 'DISCONNECTED' } }),
+}]);
 
 /*
  * Notifies the monitoring actor that a fullnode event was received (resets the idle timer).
  */
-export const sendMonitoringEventReceived = sendTo(
-  getMonitoringRefFromContext,
-  { type: EventTypes.MONITORING_EVENT, event: { type: 'EVENT_RECEIVED' } },
-);
+export const sendMonitoringEventReceived = choose([{
+  cond: monitoringIsPresent,
+  actions: sendTo(getMonitoringRefFromContext, { type: EventTypes.MONITORING_EVENT, event: { type: 'EVENT_RECEIVED' } }),
+}]);
 
 /*
  * Notifies the monitoring actor that the machine is entering the RECONNECTING state.
  */
-export const sendMonitoringReconnecting = sendTo(
-  getMonitoringRefFromContext,
-  { type: EventTypes.MONITORING_EVENT, event: { type: 'RECONNECTING' } },
-);
+export const sendMonitoringReconnecting = choose([{
+  cond: monitoringIsPresent,
+  actions: sendTo(getMonitoringRefFromContext, { type: EventTypes.MONITORING_EVENT, event: { type: 'RECONNECTING' } }),
+}]);
 
 /*
  * Notifies the monitoring actor that a processing state was entered.
  * The actor starts a stuck-detection timer; if PROCESSING_COMPLETED doesn't
- * arrive within STUCK_PROCESSING_TIMEOUT_MS it fires a CRITICAL alert and sends
- * MONITORING_STUCK_PROCESSING back to the machine.
+ * arrive within STUCK_PROCESSING_TIMEOUT_MS it fires a MAJOR alert.
  */
-export const sendMonitoringProcessingStarted = sendTo(
-  getMonitoringRefFromContext,
-  { type: EventTypes.MONITORING_EVENT, event: { type: 'PROCESSING_STARTED' } },
-);
+export const sendMonitoringProcessingStarted = choose([{
+  cond: monitoringIsPresent,
+  actions: sendTo(getMonitoringRefFromContext, { type: EventTypes.MONITORING_EVENT, event: { type: 'PROCESSING_STARTED' } }),
+}]);
 
 /*
  * Notifies the monitoring actor that a processing state was exited normally,
  * cancelling the stuck-detection timer.
  */
-export const sendMonitoringProcessingCompleted = sendTo(
-  getMonitoringRefFromContext,
-  { type: EventTypes.MONITORING_EVENT, event: { type: 'PROCESSING_COMPLETED' } },
-);
+export const sendMonitoringProcessingCompleted = choose([{
+  cond: monitoringIsPresent,
+  actions: sendTo(getMonitoringRefFromContext, { type: EventTypes.MONITORING_EVENT, event: { type: 'PROCESSING_COMPLETED' } }),
+}]);

packages/daemon/src/actors/MonitoringActor.ts

@@ -15,8 +15,8 @@ import { Event, EventTypes } from '../types';
  *
  * Centralises all runtime health monitoring for the sync state machine.
  * The machine sends MONITORING_EVENTs to this actor; when anomalies are detected
- * the actor fires alerts and, when necessary, sends MONITORING_STUCK_PROCESSING
- * back to the machine to trigger a soft reconnect.
+ * the actor fires alerts and, when necessary, sends MONITORING_IDLE_TIMEOUT back
+ * to the machine to trigger a graceful shutdown.
  *
  * Responsibilities:
  *
@@ -26,13 +26,13 @@ import { Event, EventTypes } from '../types';
  *
  * 2. Stuck-processing detection — PROCESSING_STARTED begins a one-shot timer;
  *    PROCESSING_COMPLETED cancels it.  If the timer fires the actor fires a
- *    CRITICAL alert and sends MONITORING_STUCK_PROCESSING to the machine, which
- *    transitions to RECONNECTING.  This replaces the per-state XState `after`
- *    blocks that previously scattered this logic across every processing state.
+ *    MAJOR alert; the machine keeps running so that a long-running handler
+ *    (e.g. a large reorg) is allowed to finish.
  *
- * 3. Reconnection storm detection — fires a CRITICAL alert when the daemon
+ * 3. Reconnection storm detection — fires a MAJOR alert when the daemon
  *    reconnects more than RECONNECTION_STORM_THRESHOLD times within
- *    RECONNECTION_STORM_WINDOW_MS.
+ *    RECONNECTION_STORM_WINDOW_MS.  Duplicate alerts are suppressed for
+ *    STORM_ALERT_COOLDOWN_MS (1 min) to avoid spamming the alerting system.
  */
 const DEFAULT_IDLE_EVENT_TIMEOUT_MS = 5 * 60 * 1000;
 const DEFAULT_STUCK_PROCESSING_TIMEOUT_MS = 60 * 60 * 1000; // 1 hour
@@ -59,6 +59,7 @@ export default (callback: any, receive: any, config = getConfig()) => {
     idleAlertFired = false;
 
     idleCheckTimer = setInterval(async () => {
+      // Interval is idleTimeoutMs/2 so the worst-case detection lag is 1.5×timeout
       if (!isConnected || lastEventReceivedAt === null) return;
 
       const idleMs = Date.now() - lastEventReceivedAt;
@@ -75,9 +76,11 @@ export default (callback: any, receive: any, config = getConfig()) => {
           Severity.MAJOR,
           { idleMs: String(idleMs) },
           logger,
-        ).finally(() => process.exit(1));
+        ).finally(() => {
+          callback({ type: EventTypes.MONITORING_IDLE_TIMEOUT });
+        });
       }
-    }, idleTimeoutMs);
+    }, Math.floor(idleTimeoutMs / 2));
   };
 
   const stopIdleCheck = () => {
@@ -102,7 +105,7 @@ export default (callback: any, receive: any, config = getConfig()) => {
         { timeoutMs: String(stuckTimeoutMs) },
         logger,
       ).catch((err: Error) =>
-        logger.error(`[monitoring] Failed to send stuck-processing alert: ${err}`),
+        logger.error(`[monitoring] Failed to send stuck-processing alert: ${err.message}`),
       );
     }, stuckTimeoutMs);
   };
@@ -115,7 +118,9 @@ export default (callback: any, receive: any, config = getConfig()) => {
   };
 
   // ── Reconnection storm detection ─────────────────────────────────────────────
+  const STORM_ALERT_COOLDOWN_MS = 60 * 1000; // suppress duplicate storm alerts for 1 minute
   let reconnectionTimestamps: number[] = [];
+  let stormAlertLastFiredAt: number | null = null;
 
   const trackReconnection = () => {
     const now = Date.now();
@@ -125,6 +130,11 @@ export default (callback: any, receive: any, config = getConfig()) => {
     reconnectionTimestamps = reconnectionTimestamps.filter(t => t >= windowStart);
 
     if (reconnectionTimestamps.length >= stormThreshold) {
+      if (stormAlertLastFiredAt !== null && now - stormAlertLastFiredAt < STORM_ALERT_COOLDOWN_MS) {
+        return; // still within cooldown — do not spam the alerting system
+      }
+      stormAlertLastFiredAt = now;
+
       const windowMinutes = Math.round(stormWindowMs / 60000);
       logger.error(
         `[monitoring] Reconnection storm: ${reconnectionTimestamps.length} reconnections in the last ${windowMinutes} minutes`,
@@ -140,7 +150,7 @@ export default (callback: any, receive: any, config = getConfig()) => {
         },
         logger,
       ).catch((err: Error) =>
-        logger.error(`[monitoring] Failed to send reconnection storm alert: ${err}`),
+        logger.error(`[monitoring] Failed to send reconnection storm alert: ${err.message}`),
       );
     }
   };

packages/daemon/src/delays/index.ts

@@ -24,4 +24,3 @@ export const ACK_TIMEOUT = () => {
   const { ACK_TIMEOUT_MS } = getConfig();
   return ACK_TIMEOUT_MS;
 };
-

packages/daemon/src/index.ts

@@ -20,6 +20,11 @@ const main = async () => {
     logger.info(`Transitioned to ${bigIntUtils.JSONBigInt.stringify(state.value)}`);
   });
 
+  machine.onDone(() => {
+    logger.error('Sync machine reached a final state — terminating process for Kubernetes restart');
+    process.exit(1);
+  });
+
   machine.onEvent((event) => {
     logger.info(`Processing event: ${bigIntUtils.JSONBigInt.stringify(event.type)}`);
   });

packages/daemon/src/machines/SyncMachine.ts

@@ -391,6 +391,9 @@ export const SyncMachine = Machine<Context, any, Event>({
           cond: 'websocketDisconnected',
           target: SYNC_MACHINE_STATES.RECONNECTING,
         }],
+        MONITORING_IDLE_TIMEOUT: {
+          target: `#${SYNC_MACHINE_STATES.ERROR}`,
+        },
       },
     },
     [SYNC_MACHINE_STATES.ERROR]: {

packages/daemon/src/types/event.ts

@@ -42,6 +42,7 @@ export enum EventTypes {
   WEBSOCKET_SEND_EVENT = 'WEBSOCKET_SEND_EVENT',
   HEALTHCHECK_EVENT = 'HEALTHCHECK_EVENT',
   MONITORING_EVENT = 'MONITORING_EVENT',
+  MONITORING_IDLE_TIMEOUT = 'MONITORING_IDLE_TIMEOUT',
 }
 
 export enum FullNodeEventTypes {
@@ -82,7 +83,8 @@ export type Event =
   | { type: EventTypes.FULLNODE_EVENT, event: FullNodeEvent }
   | { type: EventTypes.WEBSOCKET_SEND_EVENT, event: WebSocketSendEvent }
   | { type: EventTypes.HEALTHCHECK_EVENT, event: HealthCheckEvent }
-  | { type: EventTypes.MONITORING_EVENT, event: MonitoringEvent };
+  | { type: EventTypes.MONITORING_EVENT, event: MonitoringEvent }
+  | { type: EventTypes.MONITORING_IDLE_TIMEOUT };
 
 
 export interface VertexRemovedEventData {