From d01cf6e3759e6d1195c96859a378cfc6fd477394 Mon Sep 17 00:00:00 2001 From: saber04414 Date: Tue, 13 Jan 2026 11:45:12 +0200 Subject: [PATCH 1/6] fix: Send webhook notification when monitor transitions from PENDING to UP after being DOWN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #6025 When a monitor transitions DOWN → PENDING → UP, the UP webhook notification was not being sent because PENDING → UP transitions were marked as 'not important' for notifications. This fix: - Adds getLastNonPendingStatus() helper to check if monitor was DOWN before PENDING - Modifies isImportantForNotification() to check history when PENDING → UP transition occurs - If monitor was DOWN before PENDING, PENDING → UP is now treated as important for notifications - Maintains backward compatibility: UP → PENDING → UP still doesn't trigger notifications Tested with Docker container monitor and HTTP monitor scenarios. --- server/model/monitor.js | 39 ++++++++++++++++++++++++++++++++---- server/routers/api-router.js | 2 +- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/server/model/monitor.js b/server/model/monitor.js index 01d655f23..0dd5a6497 100644 --- a/server/model/monitor.js +++ b/server/model/monitor.js @@ -1009,7 +1009,7 @@ class Monitor extends BeanModel { if (isImportant) { bean.important = true; - if (Monitor.isImportantForNotification(isFirstBeat, previousBeat?.status, bean.status)) { + if (await Monitor.isImportantForNotification(isFirstBeat, previousBeat?.status, bean.status, this.id)) { log.debug("monitor", `[${this.name}] sendNotification`); await Monitor.sendNotification(isFirstBeat, this, bean); } else { @@ -1450,16 +1450,17 @@ 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 - * @returns {boolean} True if is an important beat else false + * @param {number} [monitorID] Optional monitor ID to check history for PENDING->UP transitions + * @returns {Promise} True if is an important beat else false */ - static isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus) { + static async isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus, monitorID = null) { // * ? -> 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 + // PENDING -> UP = important if monitor was DOWN before PENDING (fix for issue #6025) // DOWN -> PENDING = this case not exists // DOWN -> DOWN = not important // * DOWN -> UP = important @@ -1468,6 +1469,21 @@ class Monitor extends BeanModel { // * MAINTENANCE -> DOWN = important // DOWN -> MAINTENANCE = not important // UP -> MAINTENANCE = not important + + // Check for PENDING -> UP transition + if (previousBeatStatus === PENDING && currentBeatStatus === UP) { + // If monitorID is provided, check if the monitor was DOWN before entering PENDING + if (monitorID) { + const lastNonPendingStatus = await Monitor.getLastNonPendingStatus(monitorID); + // If the last non-PENDING status was DOWN, this transition is important + if (lastNonPendingStatus === DOWN) { + return true; + } + } + // Otherwise, PENDING -> UP is not important (original behavior) + return false; + } + return ( isFirstBeat || (previousBeatStatus === MAINTENANCE && currentBeatStatus === DOWN) || @@ -1657,6 +1673,21 @@ 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..c2a9bc7dc 100644 --- a/server/routers/api-router.js +++ b/server/routers/api-router.js @@ -99,7 +99,7 @@ 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)) { + if (await Monitor.isImportantForNotification(isFirstBeat, previousHeartbeat?.status, bean.status, monitor.id)) { // Reset down count bean.downCount = 0; From c1919b2cba826e595a556c0dbf881bfb9d580aac Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 13 Jan 2026 09:47:57 +0000 Subject: [PATCH 2/6] [autofix.ci] apply automated fixes --- server/model/monitor.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/server/model/monitor.js b/server/model/monitor.js index 0dd5a6497..a6fd59d53 100644 --- a/server/model/monitor.js +++ b/server/model/monitor.js @@ -1450,7 +1450,7 @@ 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 {number} [monitorID] Optional monitor ID to check history for PENDING->UP transitions + * @param {number} monitorID Optional monitor ID to check history for PENDING->UP transitions * @returns {Promise} True if is an important beat else false */ static async isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus, monitorID = null) { @@ -1680,11 +1680,10 @@ class Monitor extends BeanModel { * @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] - ); + const heartbeat = await R.findOne("heartbeat", " monitor_id = ? AND status != ? ORDER BY time DESC LIMIT 1", [ + monitorID, + PENDING, + ]); return heartbeat?.status || null; } From f0c17ab9e8cf0c52019f3b0fe607677653f0db72 Mon Sep 17 00:00:00 2001 From: saber04414 Date: Tue, 13 Jan 2026 19:10:49 +0200 Subject: [PATCH 3/6] Refactor isImportantForNotification: remove DB dependency and add tests - Make function synchronous and remove database dependency - Accept lastNonPendingStatus as parameter instead of monitorID - Simplify function logic for better testability - Update call sites to fetch lastNonPendingStatus only when needed - Add comprehensive test suite with 17 test cases covering all status transitions Addresses code review feedback about function complexity and testability --- server/model/monitor.js | 52 ++++------ server/routers/api-router.js | 8 +- .../backend-test/test-monitor-notification.js | 96 +++++++++++++++++++ 3 files changed, 122 insertions(+), 34 deletions(-) create mode 100644 test/backend-test/test-monitor-notification.js diff --git a/server/model/monitor.js b/server/model/monitor.js index a6fd59d53..27da0f8b4 100644 --- a/server/model/monitor.js +++ b/server/model/monitor.js @@ -1009,7 +1009,13 @@ class Monitor extends BeanModel { if (isImportant) { bean.important = true; - if (await Monitor.isImportantForNotification(isFirstBeat, previousBeat?.status, bean.status, this.id)) { + // 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 { @@ -1450,42 +1456,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 {number} monitorID Optional monitor ID to check history for PENDING->UP transitions - * @returns {Promise} True if is an important beat else false + * @returns {boolean} True if is an important beat else false +>>>>>>> d4c2e5fc (Refactor isImportantForNotification: remove DB dependency and add tests) */ - static async isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus, monitorID = null) { - // * ? -> ANY STATUS = important [isFirstBeat] - // UP -> PENDING = not important - // * UP -> DOWN = important - // UP -> UP = not important - // PENDING -> PENDING = not important - // * PENDING -> DOWN = important - // PENDING -> UP = important if monitor was DOWN before PENDING (fix for issue #6025) - // 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 - - // Check for PENDING -> UP transition - if (previousBeatStatus === PENDING && currentBeatStatus === UP) { - // If monitorID is provided, check if the monitor was DOWN before entering PENDING - if (monitorID) { - const lastNonPendingStatus = await Monitor.getLastNonPendingStatus(monitorID); - // If the last non-PENDING status was DOWN, this transition is important - if (lastNonPendingStatus === DOWN) { - return true; - } - } - // Otherwise, PENDING -> UP is not important (original behavior) - return false; + 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) || diff --git a/server/routers/api-router.js b/server/routers/api-router.js index c2a9bc7dc..12876cb4a 100644 --- a/server/routers/api-router.js +++ b/server/routers/api-router.js @@ -99,7 +99,13 @@ router.all("/api/push/:pushToken", async (request, response) => { bean.important = Monitor.isImportantBeat(isFirstBeat, previousHeartbeat?.status, bean.status); - if (await Monitor.isImportantForNotification(isFirstBeat, previousHeartbeat?.status, bean.status, monitor.id)) { + // 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..89ed8f589 --- /dev/null +++ b/test/backend-test/test-monitor-notification.js @@ -0,0 +1,96 @@ +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); + }); +}); + From d35f3e8a7f4c9bc25648f211d3a2353949ab43d5 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 13 Jan 2026 17:13:48 +0000 Subject: [PATCH 4/6] [autofix.ci] apply automated fixes --- server/model/monitor.js | 10 +++++++++- server/routers/api-router.js | 9 ++++++++- test/backend-test/test-monitor-notification.js | 1 - 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/server/model/monitor.js b/server/model/monitor.js index 27da0f8b4..9727fc1cd 100644 --- a/server/model/monitor.js +++ b/server/model/monitor.js @@ -1015,7 +1015,14 @@ class Monitor extends BeanModel { lastNonPendingStatus = await Monitor.getLastNonPendingStatus(this.id); } - if (Monitor.isImportantForNotification(isFirstBeat, previousBeat?.status, bean.status, lastNonPendingStatus)) { + if ( + Monitor.isImportantForNotification( + isFirstBeat, + previousBeat?.status, + bean.status, + lastNonPendingStatus + ) + ) { log.debug("monitor", `[${this.name}] sendNotification`); await Monitor.sendNotification(isFirstBeat, this, bean); } else { @@ -1456,6 +1463,7 @@ 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 lastNonPendingStatus * @returns {boolean} True if is an important beat else false >>>>>>> d4c2e5fc (Refactor isImportantForNotification: remove DB dependency and add tests) */ diff --git a/server/routers/api-router.js b/server/routers/api-router.js index 12876cb4a..ce79ab009 100644 --- a/server/routers/api-router.js +++ b/server/routers/api-router.js @@ -105,7 +105,14 @@ router.all("/api/push/:pushToken", async (request, response) => { lastNonPendingStatus = await Monitor.getLastNonPendingStatus(monitor.id); } - if (Monitor.isImportantForNotification(isFirstBeat, previousHeartbeat?.status, bean.status, lastNonPendingStatus)) { + 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 index 89ed8f589..06b1b7232 100644 --- a/test/backend-test/test-monitor-notification.js +++ b/test/backend-test/test-monitor-notification.js @@ -93,4 +93,3 @@ describe("Monitor.isImportantForNotification", () => { assert.strictEqual(Monitor.isImportantForNotification(false, UP, UP, DOWN), false); }); }); - From e64949c447d04f64b1de5adbefa6781523679f81 Mon Sep 17 00:00:00 2001 From: saber04414 Date: Tue, 13 Jan 2026 19:14:53 +0200 Subject: [PATCH 5/6] Fix JSDoc: remove brackets from optional parameter declaration --- server/model/monitor.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/model/monitor.js b/server/model/monitor.js index 9727fc1cd..7a4516e85 100644 --- a/server/model/monitor.js +++ b/server/model/monitor.js @@ -1463,9 +1463,8 @@ 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 lastNonPendingStatus + * @param {string|null} lastNonPendingStatus Optional last non-PENDING status for PENDING->UP transitions * @returns {boolean} True if is an important beat else false ->>>>>>> d4c2e5fc (Refactor isImportantForNotification: remove DB dependency and add tests) */ static isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus, lastNonPendingStatus = null) { // First beat is always important @@ -1674,10 +1673,11 @@ class Monitor extends BeanModel { * @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, - ]); + const heartbeat = await R.findOne( + "heartbeat", + " monitor_id = ? AND status != ? ORDER BY time DESC LIMIT 1", + [monitorID, PENDING] + ); return heartbeat?.status || null; } From c09573a01951324c5ce55c7671c95c68a7fc85c8 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 13 Jan 2026 17:17:09 +0000 Subject: [PATCH 6/6] [autofix.ci] apply automated fixes --- server/model/monitor.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server/model/monitor.js b/server/model/monitor.js index 7a4516e85..be5cb150e 100644 --- a/server/model/monitor.js +++ b/server/model/monitor.js @@ -1673,11 +1673,10 @@ class Monitor extends BeanModel { * @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] - ); + const heartbeat = await R.findOne("heartbeat", " monitor_id = ? AND status != ? ORDER BY time DESC LIMIT 1", [ + monitorID, + PENDING, + ]); return heartbeat?.status || null; }