diff --git a/server/model/monitor.js b/server/model/monitor.js index 3b8d89dd4..5ca6b5481 100644 --- a/server/model/monitor.js +++ b/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) { - // * ? -> ANY STATUS = important [isFirstBeat] - // UP -> PENDING = not important - // * UP -> DOWN = important - // UP -> UP = not important - // PENDING -> PENDING = not important - // * PENDING -> DOWN = important - // PENDING -> UP = not important - // DOWN -> PENDING = this case not exists - // DOWN -> DOWN = not important - // * DOWN -> UP = important - // MAINTENANCE -> MAINTENANCE = not important - // MAINTENANCE -> UP = not important - // * MAINTENANCE -> DOWN = important - // DOWN -> MAINTENANCE = not important - // UP -> MAINTENANCE = not important + static isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus, lastNonPendingStatus = null) { + // First beat is always important + if (isFirstBeat) { + return true; + } + + // PENDING -> UP is important only if monitor was DOWN before entering PENDING (fix for issue #6025) + if (previousBeatStatus === PENDING && currentBeatStatus === UP) { + return lastNonPendingStatus === DOWN; + } + + // Important transitions 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} 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 diff --git a/server/routers/api-router.js b/server/routers/api-router.js index 05c953756..ce79ab009 100644 --- a/server/routers/api-router.js +++ b/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; diff --git a/test/backend-test/test-monitor-notification.js b/test/backend-test/test-monitor-notification.js new file mode 100644 index 000000000..06b1b7232 --- /dev/null +++ b/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); + }); +});