Merge 22306fd410 into 9169a647cb

server/model/monitor.js

@@ -1009,7 +1009,20 @@ class Monitor extends BeanModel {
             if (isImportant) {
                 bean.important = true;
 
-                if (Monitor.isImportantForNotification(isFirstBeat, previousBeat?.status, bean.status)) {
+                // Get lastNonPendingStatus only for PENDING -> UP transitions
+                let lastNonPendingStatus = null;
+                if (previousBeat?.status === PENDING && bean.status === UP) {
+                    lastNonPendingStatus = await Monitor.getLastNonPendingStatus(this.id);
+                }
+
+                if (
+                    Monitor.isImportantForNotification(
+                        isFirstBeat,
+                        previousBeat?.status,
+                        bean.status,
+                        lastNonPendingStatus
+                    )
+                ) {
                     log.debug("monitor", `[${this.name}] sendNotification`);
                     await Monitor.sendNotification(isFirstBeat, this, bean);
                 } else {
@@ -1458,26 +1471,22 @@ class Monitor extends BeanModel {
      * @param {boolean} isFirstBeat Is this the first beat of this monitor?
      * @param {const} previousBeatStatus Status of the previous beat
      * @param {const} currentBeatStatus Status of the current beat
+     * @param {string|null} lastNonPendingStatus Optional last non-PENDING status for PENDING->UP transitions
      * @returns {boolean} True if is an important beat else false
      */
-    static isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus) {
+    static isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus, lastNonPendingStatus = null) {
-        // * ? -> ANY STATUS = important [isFirstBeat]
+        // First beat is always important
-        // UP -> PENDING = not important
+        if (isFirstBeat) {
-        // * UP -> DOWN = important
+            return true;
-        // UP -> UP = not important
+        }
-        // PENDING -> PENDING = not important
+
-        // * PENDING -> DOWN = important
+        // PENDING -> UP is important only if monitor was DOWN before entering PENDING (fix for issue #6025)
-        // PENDING -> UP = not important
+        if (previousBeatStatus === PENDING && currentBeatStatus === UP) {
-        // DOWN -> PENDING = this case not exists
+            return lastNonPendingStatus === DOWN;
-        // DOWN -> DOWN = not important
+        }
-        // * DOWN -> UP = important
+
-        // MAINTENANCE -> MAINTENANCE = not important
+        // Important transitions
-        // MAINTENANCE -> UP = not important
-        // * MAINTENANCE -> DOWN = important
-        // DOWN -> MAINTENANCE = not important
-        // UP -> MAINTENANCE = not important
         return (
-            isFirstBeat ||
             (previousBeatStatus === MAINTENANCE && currentBeatStatus === DOWN) ||
             (previousBeatStatus === UP && currentBeatStatus === DOWN) ||
             (previousBeatStatus === DOWN && currentBeatStatus === UP) ||
@@ -1687,6 +1696,20 @@ class Monitor extends BeanModel {
         return await R.findOne("heartbeat", " id = (select MAX(id) from heartbeat where monitor_id = ?)", [monitorID]);
     }
 
+    /**
+     * Get the last non-PENDING heartbeat status for a monitor
+     * This is useful to determine if a monitor was DOWN before entering PENDING state
+     * @param {number} monitorID ID of monitor to check
+     * @returns {Promise<string|null>} Last non-PENDING status or null if not found
+     */
+    static async getLastNonPendingStatus(monitorID) {
+        const heartbeat = await R.findOne("heartbeat", " monitor_id = ? AND status != ? ORDER BY time DESC LIMIT 1", [
+            monitorID,
+            PENDING,
+        ]);
+        return heartbeat?.status || null;
+    }
+
     /**
      * Check if monitor is under maintenance
      * @param {number} monitorID ID of monitor to check

server/routers/api-router.js

@@ -99,7 +99,20 @@ router.all("/api/push/:pushToken", async (request, response) => {
 
         bean.important = Monitor.isImportantBeat(isFirstBeat, previousHeartbeat?.status, bean.status);
 
-        if (Monitor.isImportantForNotification(isFirstBeat, previousHeartbeat?.status, bean.status)) {
+        // Get lastNonPendingStatus only for PENDING -> UP transitions
+        let lastNonPendingStatus = null;
+        if (previousHeartbeat?.status === PENDING && bean.status === UP) {
+            lastNonPendingStatus = await Monitor.getLastNonPendingStatus(monitor.id);
+        }
+
+        if (
+            Monitor.isImportantForNotification(
+                isFirstBeat,
+                previousHeartbeat?.status,
+                bean.status,
+                lastNonPendingStatus
+            )
+        ) {
             // Reset down count
             bean.downCount = 0;
 

test/backend-test/test-monitor-notification.js

@@ -0,0 +1,95 @@
+const { describe, test } = require("node:test");
+const assert = require("node:assert");
+const Monitor = require("../../server/model/monitor");
+const { UP, DOWN, PENDING, MAINTENANCE } = require("../../src/util");
+
+describe("Monitor.isImportantForNotification", () => {
+    // First beat is always important
+    test("first beat is always important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(true, null, UP), true);
+        assert.strictEqual(Monitor.isImportantForNotification(true, null, DOWN), true);
+        assert.strictEqual(Monitor.isImportantForNotification(true, null, PENDING), true);
+        assert.strictEqual(Monitor.isImportantForNotification(true, null, MAINTENANCE), true);
+    });
+
+    // UP -> PENDING = not important
+    test("UP -> PENDING is not important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, UP, PENDING), false);
+    });
+
+    // UP -> DOWN = important
+    test("UP -> DOWN is important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, UP, DOWN), true);
+    });
+
+    // UP -> UP = not important
+    test("UP -> UP is not important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, UP, UP), false);
+    });
+
+    // PENDING -> PENDING = not important
+    test("PENDING -> PENDING is not important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, PENDING), false);
+    });
+
+    // PENDING -> DOWN = important
+    test("PENDING -> DOWN is important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, DOWN), true);
+    });
+
+    // PENDING -> UP = important if monitor was DOWN before PENDING (fix for issue #6025)
+    test("PENDING -> UP is important when lastNonPendingStatus was DOWN", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, DOWN), true);
+    });
+
+    test("PENDING -> UP is not important when lastNonPendingStatus was not DOWN", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, UP), false);
+        assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, MAINTENANCE), false);
+        assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, null), false);
+    });
+
+    // DOWN -> DOWN = not important
+    test("DOWN -> DOWN is not important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, DOWN, DOWN), false);
+    });
+
+    // DOWN -> UP = important
+    test("DOWN -> UP is important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, DOWN, UP), true);
+    });
+
+    // MAINTENANCE -> MAINTENANCE = not important
+    test("MAINTENANCE -> MAINTENANCE is not important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, MAINTENANCE, MAINTENANCE), false);
+    });
+
+    // MAINTENANCE -> UP = not important
+    test("MAINTENANCE -> UP is not important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, MAINTENANCE, UP), false);
+    });
+
+    // MAINTENANCE -> DOWN = important
+    test("MAINTENANCE -> DOWN is important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, MAINTENANCE, DOWN), true);
+    });
+
+    // DOWN -> MAINTENANCE = not important
+    test("DOWN -> MAINTENANCE is not important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, DOWN, MAINTENANCE), false);
+    });
+
+    // UP -> MAINTENANCE = not important
+    test("UP -> MAINTENANCE is not important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, UP, MAINTENANCE), false);
+    });
+
+    // Additional edge cases
+    test("PENDING -> UP with undefined lastNonPendingStatus defaults to not important", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, undefined), false);
+    });
+
+    test("non-PENDING -> UP transitions ignore lastNonPendingStatus parameter", () => {
+        assert.strictEqual(Monitor.isImportantForNotification(false, DOWN, UP, DOWN), true);
+        assert.strictEqual(Monitor.isImportantForNotification(false, UP, UP, DOWN), false);
+    });
+});