From 5628c202f706d337ed0edb2ab1e5a07ae2637f8b Mon Sep 17 00:00:00 2001 From: harsh-aghara Date: Thu, 4 Jun 2026 22:43:43 +0530 Subject: [PATCH] refactor: extract isStatusUp utility to decouple from got's response.ok and modified utils test accordingly --- .../infrastructure/globalPingService.ts | 4 +- .../infrastructure/network/HttpProvider.ts | 6 +-- .../service/infrastructure/network/utils.ts | 7 ++++ .../providers/network/httpProvider.test.ts | 3 +- .../test/unit/providers/network/utils.test.ts | 38 ++++++++++++++++++- 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/server/src/service/infrastructure/globalPingService.ts b/server/src/service/infrastructure/globalPingService.ts index e261c7b19..4d7a67113 100644 --- a/server/src/service/infrastructure/globalPingService.ts +++ b/server/src/service/infrastructure/globalPingService.ts @@ -3,6 +3,7 @@ import { supportsGeoCheck, type HttpStatusCode } from "@/types/monitor.js"; import { MonitorType } from "@/types/index.js"; import type { ILogger } from "@/utils/logger.js"; import got from "got"; +import { isStatusUp } from "@/service/infrastructure/network/utils.js"; const SERVICE_NAME = "GlobalPingService"; const GLOBAL_PING_API_BASE = "https://api.globalping.io/v1"; @@ -209,8 +210,7 @@ export class GlobalPingService implements IGlobalPingService { successfulResults.push({ location, - status: - customUpCodes.includes(probeResult.result.statusCode) || (probeResult.result.statusCode >= 200 && probeResult.result.statusCode < 300), + status: isStatusUp(probeResult.result.statusCode, customUpCodes), statusCode: probeResult.result.statusCode, timings, }); diff --git a/server/src/service/infrastructure/network/HttpProvider.ts b/server/src/service/infrastructure/network/HttpProvider.ts index 971c4e6c2..73cdd6faf 100644 --- a/server/src/service/infrastructure/network/HttpProvider.ts +++ b/server/src/service/infrastructure/network/HttpProvider.ts @@ -5,7 +5,7 @@ import { HttpStatusPayload } from "@/types/network.js"; import { MonitorStatusResponse } from "@/types/network.js"; import { Agent as HttpsAgent } from "https"; import { Monitor, MonitorType } from "@/types/monitor.js"; -import { NETWORK_ERROR } from "@/service/infrastructure/network/utils.js"; +import { NETWORK_ERROR, isStatusUp } from "@/service/infrastructure/network/utils.js"; import CacheableLookup from "cacheable-lookup"; export class HttpProvider implements IStatusProvider { @@ -32,7 +32,7 @@ export class HttpProvider implements IStatusProvider { private handleHttpError(error: unknown, monitor: Monitor): MonitorStatusResponse { if (error instanceof HTTPError || error instanceof RequestError) { const statusCode = error.response?.statusCode; - const statusUp = statusCode !== undefined && monitor.customUpCodes.includes(statusCode); + const statusUp = isStatusUp(statusCode, monitor.customUpCodes); return { monitorId: monitor.id, @@ -105,7 +105,7 @@ export class HttpProvider implements IStatusProvider { } const matchResult = this.advancedMatcher.validate(payload, monitor); - const statusUp = monitor.customUpCodes.includes(response.statusCode) || response.ok; + const statusUp = isStatusUp(response.statusCode, monitor.customUpCodes); return { monitorId: monitor.id, teamId: monitor.teamId, diff --git a/server/src/service/infrastructure/network/utils.ts b/server/src/service/infrastructure/network/utils.ts index ddb962f47..994e19d9d 100644 --- a/server/src/service/infrastructure/network/utils.ts +++ b/server/src/service/infrastructure/network/utils.ts @@ -1,3 +1,5 @@ +import type { HttpStatusCode } from "@/types/monitor.js"; + export const timeRequest = async (operation: () => Promise): Promise<{ response: T | null; responseTime: number; error: unknown }> => { const start = process.hrtime.bigint(); try { @@ -12,3 +14,8 @@ export const timeRequest = async (operation: () => Promise): Promise<{ res export const NETWORK_ERROR = 5000; export const PING_ERROR = 5001; + +export const isStatusUp = (statusCode: number | undefined, customUpCodes: HttpStatusCode[] = []): boolean => { + if (statusCode === undefined) return false; + return (statusCode >= 200 && statusCode < 300) || customUpCodes.includes(statusCode); +}; diff --git a/server/test/unit/providers/network/httpProvider.test.ts b/server/test/unit/providers/network/httpProvider.test.ts index 9252493e0..ea24b9872 100644 --- a/server/test/unit/providers/network/httpProvider.test.ts +++ b/server/test/unit/providers/network/httpProvider.test.ts @@ -254,14 +254,13 @@ describe("HttpProvider", () => { expect(result.extracted).toBe("value"); }); - it("sets status to false when response.ok is false even if matcher passes", async () => { + it("sets status to false when status code is non-2xx even if matcher passes", async () => { mockGot.mockResolvedValue(makeGotResponse({ ok: false, statusCode: 301 })); const matcher = createMockMatcher({ ok: true, message: "Success" }); const { provider } = createProvider(matcher); const result = await provider.handle(makeMonitor()); - // status = response.ok && matchResult.ok expect(result.status).toBe(false); }); }); diff --git a/server/test/unit/providers/network/utils.test.ts b/server/test/unit/providers/network/utils.test.ts index 56d8e7f9c..84427152e 100644 --- a/server/test/unit/providers/network/utils.test.ts +++ b/server/test/unit/providers/network/utils.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "@jest/globals"; -import { timeRequest, NETWORK_ERROR, PING_ERROR } from "../../../../src/service/infrastructure/network/utils.ts"; +import { timeRequest, NETWORK_ERROR, PING_ERROR, isStatusUp } from "../../../../src/service/infrastructure/network/utils.ts"; describe("network utils", () => { describe("timeRequest", () => { @@ -41,4 +41,40 @@ describe("network utils", () => { expect(PING_ERROR).toBe(5001); }); }); + + describe("isStatusUp", () => { + it("returns true for 2xx status codes", () => { + expect(isStatusUp(200)).toBe(true); + expect(isStatusUp(201)).toBe(true); + expect(isStatusUp(204)).toBe(true); + expect(isStatusUp(299)).toBe(true); + }); + + it("returns false for non-2xx status codes without customUpCodes", () => { + expect(isStatusUp(100)).toBe(false); + expect(isStatusUp(301)).toBe(false); + expect(isStatusUp(404)).toBe(false); + expect(isStatusUp(500)).toBe(false); + }); + + it("returns true when status code is in customUpCodes", () => { + expect(isStatusUp(301, [301])).toBe(true); + expect(isStatusUp(404, [200, 404])).toBe(true); + expect(isStatusUp(401, [401, 403])).toBe(true); + }); + + it("returns false when status code is not in customUpCodes", () => { + expect(isStatusUp(500, [401, 403])).toBe(false); + }); + + it("returns false when statusCode is undefined", () => { + expect(isStatusUp(undefined)).toBe(false); + expect(isStatusUp(undefined, [200])).toBe(false); + }); + + it("defaults customUpCodes to empty array", () => { + expect(isStatusUp(200)).toBe(true); + expect(isStatusUp(404)).toBe(false); + }); + }); });