refactor: extract isStatusUp utility to decouple from got's response.ok and modified utils test accordingly

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,
 				});

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<HttpStatusPayload> {
@@ -32,7 +32,7 @@ export class HttpProvider implements IStatusProvider<HttpStatusPayload> {
 	private handleHttpError<T>(error: unknown, monitor: Monitor): MonitorStatusResponse<T> {
 		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<HttpStatusPayload> {
 			}
 
 			const matchResult = this.advancedMatcher.validate<T>(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,

server/src/service/infrastructure/network/utils.ts

@@ -1,3 +1,5 @@
+import type { HttpStatusCode } from "@/types/monitor.js";
+
 export const timeRequest = async <T>(operation: () => Promise<T>): Promise<{ response: T | null; responseTime: number; error: unknown }> => {
 	const start = process.hrtime.bigint();
 	try {
@@ -12,3 +14,8 @@ export const timeRequest = async <T>(operation: () => Promise<T>): 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);
+};

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);
 		});
 	});

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);
+		});
+	});
 });