From 6e49601eed26cdf8afa5c1a8d8a5900181d6a196 Mon Sep 17 00:00:00 2001 From: Louis Lam Date: Fri, 28 Nov 2025 20:25:06 +0800 Subject: [PATCH] Enforce UP status for non-custom status monitors (#6433) Co-authored-by: Frank Elsinga Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/PULL_REQUEST_TEMPLATE.md | 9 +++++ server/model/monitor.js | 5 +++ server/monitor-types/dns.js | 8 +++- server/monitor-types/group.js | 1 + server/monitor-types/manual.js | 2 + server/monitor-types/monitor-type.js | 9 +++++ server/monitor-types/rabbitmq.js | 11 +++--- server/monitor-types/tcp.js | 12 ++---- server/monitor-types/websocket-upgrade.js | 11 ++++-- test/backend-test/test-rabbitmq.js | 11 ++++-- test/backend-test/test-tcp.js | 26 ++++++++----- test/backend-test/test-websocket.js | 46 +++++++++-------------- 12 files changed, 90 insertions(+), 61 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index e351aa2e2..71a2f7fc4 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -78,3 +78,12 @@ Avoid using external image services as the image will be uploaded automatically. | `DOWN` | ![Before](image-link) | ![After](image-link) | | Certificate-expiry | ![Before](image-link) | ![After](image-link) | | Testing | ![Before](image-link) | ![After](image-link) | + + diff --git a/server/model/monitor.js b/server/model/monitor.js index aa5d6ba9b..0b6be92d7 100644 --- a/server/model/monitor.js +++ b/server/model/monitor.js @@ -860,6 +860,11 @@ class Monitor extends BeanModel { let startTime = dayjs().valueOf(); const monitorType = UptimeKumaServer.monitorTypeList[this.type]; await monitorType.check(this, bean, UptimeKumaServer.getInstance()); + + if (!monitorType.allowCustomStatus && bean.status !== UP) { + throw new Error("The monitor implementation is incorrect, non-UP error must throw error inside check()"); + } + if (!bean.ping) { bean.ping = dayjs().valueOf() - startTime; } diff --git a/server/monitor-types/dns.js b/server/monitor-types/dns.js index 5a47e4591..77032b302 100644 --- a/server/monitor-types/dns.js +++ b/server/monitor-types/dns.js @@ -1,5 +1,5 @@ const { MonitorType } = require("./monitor-type"); -const { UP, DOWN } = require("../../src/util"); +const { UP } = require("../../src/util"); const dayjs = require("dayjs"); const { dnsResolve } = require("../util-server"); const { R } = require("redbean-node"); @@ -79,8 +79,12 @@ class DnsMonitorType extends MonitorType { await R.exec("UPDATE `monitor` SET dns_last_result = ? WHERE id = ? ", [ dnsMessage, monitor.id ]); } + if (!conditionsResult) { + throw new Error(dnsMessage); + } + heartbeat.msg = dnsMessage; - heartbeat.status = conditionsResult ? UP : DOWN; + heartbeat.status = UP; } } diff --git a/server/monitor-types/group.js b/server/monitor-types/group.js index 8372b4f17..212244a39 100644 --- a/server/monitor-types/group.js +++ b/server/monitor-types/group.js @@ -4,6 +4,7 @@ const Monitor = require("../model/monitor"); class GroupMonitorType extends MonitorType { name = "group"; + allowCustomStatus = true; /** * @inheritdoc diff --git a/server/monitor-types/manual.js b/server/monitor-types/manual.js index e587b7409..7134d6c0f 100644 --- a/server/monitor-types/manual.js +++ b/server/monitor-types/manual.js @@ -8,6 +8,8 @@ class ManualMonitorType extends MonitorType { supportsConditions = false; conditionVariables = []; + allowCustomStatus = true; + /** * @inheritdoc */ diff --git a/server/monitor-types/monitor-type.js b/server/monitor-types/monitor-type.js index 8f3cbcac4..aa7b1d200 100644 --- a/server/monitor-types/monitor-type.js +++ b/server/monitor-types/monitor-type.js @@ -14,8 +14,17 @@ class MonitorType { */ conditionVariables = []; + /** + * Allows setting any custom status to heartbeat, other than UP. + * @type {boolean} + */ + allowCustomStatus = false; + /** * Run the monitoring check on the given monitor + * + * Successful cases: Should update heartbeat.status to "up" and set response time. + * Failure cases: Throw an error with a descriptive message. * @param {Monitor} monitor Monitor to check * @param {Heartbeat} heartbeat Monitor heartbeat to update * @param {UptimeKumaServer} server Uptime Kuma server diff --git a/server/monitor-types/rabbitmq.js b/server/monitor-types/rabbitmq.js index 165a0ed91..251f1b8ac 100644 --- a/server/monitor-types/rabbitmq.js +++ b/server/monitor-types/rabbitmq.js @@ -1,5 +1,5 @@ const { MonitorType } = require("./monitor-type"); -const { log, UP, DOWN } = require("../../src/util"); +const { log, UP } = require("../../src/util"); const { axiosAbortSignal } = require("../util-server"); const axios = require("axios"); @@ -17,7 +17,6 @@ class RabbitMqMonitorType extends MonitorType { throw new Error("Invalid RabbitMQ Nodes"); } - heartbeat.status = DOWN; for (let baseUrl of baseUrls) { try { // Without a trailing slash, path in baseUrl will be removed. https://example.com/api -> https://example.com @@ -45,17 +44,17 @@ class RabbitMqMonitorType extends MonitorType { heartbeat.msg = "OK"; break; } else if (res.status === 503) { - heartbeat.msg = res.data.reason; + throw new Error(res.data.reason); } else { - heartbeat.msg = `${res.status} - ${res.statusText}`; + throw new Error(`${res.status} - ${res.statusText}`); } } catch (error) { if (axios.isCancel(error)) { - heartbeat.msg = "Request timed out"; log.debug("monitor", `[${monitor.name}] Request timed out`); + throw new Error("Request timed out"); } else { log.debug("monitor", `[${monitor.name}] Axios Error: ${JSON.stringify(error.message)}`); - heartbeat.msg = error.message; + throw new Error(error.message); } } } diff --git a/server/monitor-types/tcp.js b/server/monitor-types/tcp.js index f0bc12a01..e71973b1f 100644 --- a/server/monitor-types/tcp.js +++ b/server/monitor-types/tcp.js @@ -1,5 +1,5 @@ const { MonitorType } = require("./monitor-type"); -const { UP, DOWN, PING_GLOBAL_TIMEOUT_DEFAULT: TIMEOUT, log } = require("../../src/util"); +const { UP, PING_GLOBAL_TIMEOUT_DEFAULT: TIMEOUT, log } = require("../../src/util"); const { checkCertificate } = require("../util-server"); const tls = require("tls"); const net = require("net"); @@ -47,9 +47,7 @@ class TCPMonitorType extends MonitorType { heartbeat.msg = `${resp} ms`; heartbeat.status = UP; } catch { - heartbeat.status = DOWN; - heartbeat.msg = "Connection failed"; - return; + throw new Error("Connection failed"); } let socket_; @@ -133,13 +131,11 @@ class TCPMonitorType extends MonitorType { await monitor.handleTlsInfo(tlsInfoObject); if (!tlsInfoObject.valid) { - heartbeat.status = DOWN; - heartbeat.msg = "Certificate is invalid"; + throw new Error("Certificate is invalid"); } } catch (error) { const message = error instanceof Error ? error.message : "Unknown error"; - heartbeat.status = DOWN; - heartbeat.msg = `TLS Connection failed: ${message}`; + throw new Error(`TLS Connection failed: ${message}`); } finally { if (socket && !socket.destroyed) { socket.end(); diff --git a/server/monitor-types/websocket-upgrade.js b/server/monitor-types/websocket-upgrade.js index e85c4baa7..c53225306 100644 --- a/server/monitor-types/websocket-upgrade.js +++ b/server/monitor-types/websocket-upgrade.js @@ -1,6 +1,6 @@ const { MonitorType } = require("./monitor-type"); const WebSocket = require("ws"); -const { UP, DOWN } = require("../../src/util"); +const { UP } = require("../../src/util"); class WebSocketMonitorType extends MonitorType { name = "websocket-upgrade"; @@ -10,8 +10,13 @@ class WebSocketMonitorType extends MonitorType { */ async check(monitor, heartbeat, _server) { const [ message, code ] = await this.attemptUpgrade(monitor); - heartbeat.status = code === 1000 ? UP : DOWN; - heartbeat.msg = message; + + if (code === 1000) { + heartbeat.status = UP; + heartbeat.msg = message; + } else { + throw new Error(message); + } } /** diff --git a/test/backend-test/test-rabbitmq.js b/test/backend-test/test-rabbitmq.js index 5782ef250..31f018aa9 100644 --- a/test/backend-test/test-rabbitmq.js +++ b/test/backend-test/test-rabbitmq.js @@ -2,7 +2,7 @@ const { describe, test } = require("node:test"); const assert = require("node:assert"); const { RabbitMQContainer } = require("@testcontainers/rabbitmq"); const { RabbitMqMonitorType } = require("../../server/monitor-types/rabbitmq"); -const { UP, DOWN, PENDING } = require("../../src/util"); +const { UP, PENDING } = require("../../src/util"); describe("RabbitMQ Single Node", { skip: !!process.env.CI && (process.platform !== "linux" || process.arch !== "x64"), @@ -46,8 +46,13 @@ describe("RabbitMQ Single Node", { status: PENDING, }; - await rabbitMQMonitor.check(monitor, heartbeat, {}); - assert.strictEqual(heartbeat.status, DOWN); + // regex match any string + const regex = /.+/; + + await assert.rejects( + rabbitMQMonitor.check(monitor, heartbeat, {}), + regex + ); }); }); diff --git a/test/backend-test/test-tcp.js b/test/backend-test/test-tcp.js index 3d389c154..46a4d12dd 100644 --- a/test/backend-test/test-tcp.js +++ b/test/backend-test/test-tcp.js @@ -1,7 +1,7 @@ const { describe, test } = require("node:test"); const assert = require("node:assert"); const { TCPMonitorType } = require("../../server/monitor-types/tcp"); -const { UP, DOWN, PENDING } = require("../../src/util"); +const { UP, PENDING } = require("../../src/util"); const net = require("net"); /** @@ -77,9 +77,10 @@ describe("TCP Monitor", () => { status: PENDING, }; - await tcpMonitor.check(monitor, heartbeat, {}); - - assert.strictEqual(heartbeat.status, DOWN); + await assert.rejects( + tcpMonitor.check(monitor, heartbeat, {}), + new Error("Connection failed") + ); }); /** @@ -104,10 +105,13 @@ describe("TCP Monitor", () => { status: PENDING, }; - await tcpMonitor.check(monitor, heartbeat, {}); + // Regex: contains with "TLS Connection failed:" or "Certificate is invalid" + const regex = /TLS Connection failed:|Certificate is invalid/; - assert.strictEqual(heartbeat.status, DOWN); - assert([ "Certificate is invalid", "TLS Connection failed:" ].some(prefix => heartbeat.msg.startsWith(prefix))); + await assert.rejects( + tcpMonitor.check(monitor, heartbeat, {}), + regex + ); }); test("TCP server with valid TLS certificate (SSL)", async t => { @@ -174,9 +178,11 @@ describe("TCP Monitor", () => { status: PENDING, }; - await tcpMonitor.check(monitor, heartbeat, {}); + const regex = /does not match certificate/; - assert.strictEqual(heartbeat.status, DOWN); - assert([ "does not match certificate" ].some(msg => heartbeat.msg.includes(msg))); + await assert.rejects( + tcpMonitor.check(monitor, heartbeat, {}), + regex + ); }); }); diff --git a/test/backend-test/test-websocket.js b/test/backend-test/test-websocket.js index a84930bdf..33660d134 100644 --- a/test/backend-test/test-websocket.js +++ b/test/backend-test/test-websocket.js @@ -2,7 +2,7 @@ const { WebSocketServer } = require("ws"); const { describe, test } = require("node:test"); const assert = require("node:assert"); const { WebSocketMonitorType } = require("../../server/monitor-types/websocket-upgrade"); -const { UP, DOWN, PENDING } = require("../../src/util"); +const { UP, PENDING } = require("../../src/util"); describe("Websocket Test", { }, () => { @@ -19,13 +19,10 @@ describe("Websocket Test", { status: PENDING, }; - const expected = { - msg: "Unexpected server response: 200", - status: DOWN, - }; - - await websocketMonitor.check(monitor, heartbeat, {}); - assert.deepStrictEqual(heartbeat, expected); + await assert.rejects( + websocketMonitor.check(monitor, heartbeat, {}), + new Error("Unexpected server response: 200") + ); }); test("Secure Websocket", async () => { @@ -87,13 +84,10 @@ describe("Websocket Test", { status: PENDING, }; - const expected = { - msg: "Invalid Sec-WebSocket-Accept header", - status: DOWN, - }; - - await websocketMonitor.check(monitor, heartbeat, {}); - assert.deepStrictEqual(heartbeat, expected); + await assert.rejects( + websocketMonitor.check(monitor, heartbeat, {}), + new Error("Invalid Sec-WebSocket-Accept header") + ); }); test("Non compliant WS server with IgnoreSecWebsocket", async () => { @@ -153,13 +147,10 @@ describe("Websocket Test", { status: PENDING, }; - const expected = { - msg: "Unexpected server response: 200", - status: DOWN, - }; - - await websocketMonitor.check(monitor, heartbeat, {}); - assert.deepStrictEqual(heartbeat, expected); + await assert.rejects( + websocketMonitor.check(monitor, heartbeat, {}), + new Error("Unexpected server response: 200") + ); }); test("Secure Websocket with Subprotocol", async () => { @@ -176,12 +167,9 @@ describe("Websocket Test", { status: PENDING, }; - const expected = { - msg: "Server sent no subprotocol", - status: DOWN, - }; - - await websocketMonitor.check(monitor, heartbeat, {}); - assert.deepStrictEqual(heartbeat, expected); + await assert.rejects( + websocketMonitor.check(monitor, heartbeat, {}), + new Error("Server sent no subprotocol") + ); }); });