diff --git a/server/src/config/services.worker.ts b/server/src/config/services.worker.ts index e15e1cbf5..802ad4b0c 100644 --- a/server/src/config/services.worker.ts +++ b/server/src/config/services.worker.ts @@ -78,7 +78,7 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig // *********************** // Network providers // *********************** - const pingProvider = new PingProvider(ping); + const pingProvider = new PingProvider(ping, net); const httpProvider = new HttpProvider(got, new AdvancedMatcher(jmespath)); const pageSpeedProvider = new PageSpeedProvider(httpProvider, settingsService, logger); const hardwareProvider = new HardwareProvider(httpProvider); diff --git a/server/src/domain/app-settings/app-settings-repository.interface.ts b/server/src/domain/app-settings/app-settings-repository.interface.ts index 401d867f7..c8e5a37b3 100644 --- a/server/src/domain/app-settings/app-settings-repository.interface.ts +++ b/server/src/domain/app-settings/app-settings-repository.interface.ts @@ -6,6 +6,7 @@ export interface ISettingsRepository { findSingleton(): Promise; // update update(settings: SettingsUpdate): Promise; + removeEgressNotification(notificationId: string): Promise; // $pull, so it cannot race a concurrent settings update // delete deleteLegacy: () => Promise; } diff --git a/server/src/domain/app-settings/app-settings.repository.mongo.ts b/server/src/domain/app-settings/app-settings.repository.mongo.ts index f2ef3351c..795a2db35 100644 --- a/server/src/domain/app-settings/app-settings.repository.mongo.ts +++ b/server/src/domain/app-settings/app-settings.repository.mongo.ts @@ -84,6 +84,10 @@ class MongoSettingsRepository implements ISettingsRepository { return this.toEntity(updatedSettings); }; + removeEgressNotification = async (notificationId: string) => { + await AppSettingsModel.updateMany({ egressNotifications: notificationId }, { $pull: { egressNotifications: notificationId } }); + }; + deleteLegacy = async () => { const res = await AppSettingsModel.deleteMany({ version: { $exists: false } }); return res.deletedCount > 0; diff --git a/server/src/domain/app-settings/app-settings.service.ts b/server/src/domain/app-settings/app-settings.service.ts index 2df2b7512..c0a0a184a 100755 --- a/server/src/domain/app-settings/app-settings.service.ts +++ b/server/src/domain/app-settings/app-settings.service.ts @@ -24,6 +24,7 @@ export interface ISettingsService { areStatusPageThemesEnabled(): boolean; getDBSettings(): Promise; updateDbSettings(newSettings: SettingsUpdate): Promise; + removeEgressNotification(notificationId: string): Promise; } export class SettingsService implements ISettingsService { @@ -78,6 +79,10 @@ export class SettingsService implements ISettingsService { return await this.getRepository().update(newSettings); }; + removeEgressNotification = async (notificationId: string) => { + await this.getRepository().removeEgressNotification(notificationId); + }; + getDBSettings = async () => { const repo = this.getRepository(); // Remove any old settings diff --git a/server/src/domain/egress/egress-state.repository.mongo.ts b/server/src/domain/egress/egress-state.repository.mongo.ts index a5bb110c3..6f2715d1d 100644 --- a/server/src/domain/egress/egress-state.repository.mongo.ts +++ b/server/src/domain/egress/egress-state.repository.mongo.ts @@ -30,12 +30,21 @@ class MongoEgressStateRepository implements IEgressStateRepository { // Read first: with timestamps on, every findOneAndUpdate would $set updatedAt, so a read would write. const existing = await EgressStateModel.findOne({ singleton: true }).lean(); if (existing) return this.toEntity(existing); - const doc = await EgressStateModel.findOneAndUpdate( - { singleton: true }, - { $setOnInsert: { singleton: true, status: "ok" } }, - { upsert: true, new: true, setDefaultsOnInsert: true } - ).lean(); - return this.toEntity(doc); + try { + const doc = await EgressStateModel.findOneAndUpdate( + { singleton: true }, + { $setOnInsert: { singleton: true, status: "ok" } }, + { upsert: true, new: true, setDefaultsOnInsert: true } + ).lean(); + return this.toEntity(doc); + } catch (error: unknown) { + // Two processes can race the very first insert; the unique index rejects the loser, who reads the winner's row. + if ((error as { code?: number }).code === 11000) { + const doc = await EgressStateModel.findOne({ singleton: true }).lean(); + if (doc) return this.toEntity(doc); + } + throw error; + } }; reset = async () => { diff --git a/server/src/domain/egress/egress-state.service.ts b/server/src/domain/egress/egress-state.service.ts index c4701abe9..9d605156f 100644 --- a/server/src/domain/egress/egress-state.service.ts +++ b/server/src/domain/egress/egress-state.service.ts @@ -25,7 +25,7 @@ export class EgressStateService implements IEgressStateService { // Called when the feature is switched on or off: drop any pending recovery job and start from "ok". reset = async (): Promise => { - await this.jobsRepository.deleteByIdAndType(null, "egress"); + await this.jobsRepository.deleteGlobalJob("egress"); return this.egressStateRepository.reset(); }; } diff --git a/server/src/domain/egress/egress.service.ts b/server/src/domain/egress/egress.service.ts index 40a4d4cd2..eeec82e21 100644 --- a/server/src/domain/egress/egress.service.ts +++ b/server/src/domain/egress/egress.service.ts @@ -25,6 +25,9 @@ const ASSESSMENT_REUSE_MS = 5000; const RECOVERY_JOB_TYPE = "egress" as const; const RECOVERY_JOB_ID = jobId(RECOVERY_JOB_TYPE, null); +// HttpProvider reports transport failures with NETWORK_ERROR (outside the HTTP range) and everything else with the real status code. +const isHttpStatusCode = (code: unknown): boolean => typeof code === "number" && code >= 100 && code <= 599; + // Synthetic identity stamped on the probe monitors so provider responses are recognisable in logs. const PROBE_MONITOR_ID = "egress-probe"; const PROBE_TEAM_ID = "system"; @@ -115,7 +118,9 @@ export class EgressService implements IEgressService { return { target: raw, - reachable: response.status === true, + // Any HTTP response, including 4xx/5xx, proves the instance can reach the target; only a transport + // failure counts against egress. Ping and port providers have no such distinction. + reachable: response.status === true || (target.kind === "http" && isHttpStatusCode(response.code)), responseTime: response.responseTime ?? responseTime, message: response.message, }; @@ -188,6 +193,9 @@ export class EgressService implements IEgressService { return "ok"; } + // Schedule the recovery job before recording the transition: a crash between the two then leaves a stray row + // while ok, which the job's first run removes, rather than a degraded state that nothing is polling. + await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds, true); const degraded = await this.egressStateRepository.markDegraded(results, now); this.logger.warn({ message: degraded @@ -197,8 +205,6 @@ export class EgressService implements IEgressService { method: "assessAfterFailure", details: { results }, }); - // The worker that performed the transition owns the job's schedule; anyone else only makes sure the row exists. - await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds, degraded !== null); return "degraded"; } catch (error: unknown) { // The egress check must never break check production; treat an internal failure as "unknown" and carry on. diff --git a/server/src/domain/jobs/job.repository.interface.ts b/server/src/domain/jobs/job.repository.interface.ts index 6fe6b23b9..435e49118 100644 --- a/server/src/domain/jobs/job.repository.interface.ts +++ b/server/src/domain/jobs/job.repository.interface.ts @@ -58,9 +58,11 @@ export interface IJobsRepository { // Delete, drop all rows deleteById(refId: string): Promise; - // Delete a single job of type. refId null addresses a global row - deleteByIdAndType(refId: string | null, type: JobType): Promise; + // Delete a single job of type + deleteByIdAndType(refId: string, type: JobType): Promise; + // Global rows (refId null) are addressed by their canonical _id, see jobId + deleteGlobalJob(type: JobType): Promise; // Delete a global row only if nobody has rescheduled it since it was claimed (nextScheduledAt unchanged) deleteGlobalJobIfUnchanged(type: JobType, nextScheduledAt: number): Promise; diff --git a/server/src/domain/jobs/job.repository.mongo.ts b/server/src/domain/jobs/job.repository.mongo.ts index d545b0241..d1aff472e 100644 --- a/server/src/domain/jobs/job.repository.mongo.ts +++ b/server/src/domain/jobs/job.repository.mongo.ts @@ -273,8 +273,13 @@ class MongoJobsRepository implements IJobsRepository { return res.deletedCount > 0; }; - deleteByIdAndType = async (refId: string | null, type: JobType) => { - const res = await JobModel.deleteOne({ refId, type }); // Delete a single typed row, e.g. just the geo row or the global egress row + deleteByIdAndType = async (refId: string, type: JobType) => { + const res = await JobModel.deleteOne({ refId, type }); // Delete a single typed row, e.g. just the geo row + return res.deletedCount > 0; + }; + + deleteGlobalJob = async (type: JobType) => { + const res = await JobModel.deleteOne({ _id: jobId(type, null) }); return res.deletedCount > 0; }; diff --git a/server/src/domain/notifications/notification.service.ts b/server/src/domain/notifications/notification.service.ts index e270e76d4..eb0e333d4 100644 --- a/server/src/domain/notifications/notification.service.ts +++ b/server/src/domain/notifications/notification.service.ts @@ -178,18 +178,10 @@ export class NotificationsService implements INotificationsService { deleteById = async (id: string, teamId: string): Promise => { await this.monitorsRepository.removeNotificationFromMonitors(id); - await this.removeFromEgressNotifications(id); const deleted = await this.notificationsRepository.deleteById(id, teamId); + // App settings hold notification ids for the egress recovered alert; drop the deleted one so it never dangles. + // After the team-scoped delete, so a request for another team's id (404 above) cannot touch instance settings. + await this.settingsService.removeEgressNotification(id); return deleted; }; - - // App settings hold notification ids for the egress recovered alert; drop the deleted one so it never dangles - private removeFromEgressNotifications = async (id: string) => { - const settings = await this.settingsService.getDBSettings(); - const current = settings.egressNotifications ?? []; - const remaining = current.filter((notificationId) => notificationId !== id); - if (remaining.length !== current.length) { - await this.settingsService.updateDbSettings({ egressNotifications: remaining }); - } - }; } diff --git a/server/src/service/network/PingProvider.ts b/server/src/service/network/PingProvider.ts index 9194ed9d2..7bcde8c7e 100644 --- a/server/src/service/network/PingProvider.ts +++ b/server/src/service/network/PingProvider.ts @@ -4,16 +4,20 @@ import { MonitorType, Monitor } from "@/domain/monitors/monitor.type.js"; import { MonitorStatusResponse } from "@/types/network.js"; import { AppError } from "@/utils/AppError.js"; import ping from "ping"; -import net from "net"; +import * as net from "net"; import { timeRequest } from "@/service/network/utils.js"; const SERVICE_NAME = "PingProvider"; type Ping = typeof ping; +type NetType = typeof net; export class PingProvider implements IStatusProvider { readonly type = "ping"; - constructor(private ping: Ping) {} + constructor( + private ping: Ping, + private net: NetType + ) {} supports(type: MonitorType): boolean { return type === "ping"; @@ -25,7 +29,7 @@ export class PingProvider implements IStatusProvider { const bracketed = /^\[([^\]]+)\](?::\d+)?$/.exec(host); if (bracketed) return bracketed[1]!; // A bare IPv6 address contains colons that are not a port separator - if (net.isIPv6(host)) return host; + if (this.net.isIPv6(host)) return host; return host.replace(/:.*/, ""); } diff --git a/server/test/integration/egressStateRepository.test.ts b/server/test/integration/egressStateRepository.test.ts new file mode 100644 index 000000000..c5090ab84 --- /dev/null +++ b/server/test/integration/egressStateRepository.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it, beforeAll, afterAll, beforeEach } from "@jest/globals"; +import mongoose from "mongoose"; +import { MongoMemoryServer } from "mongodb-memory-server"; +import MongoEgressStateRepository from "../../src/domain/egress/egress-state.repository.mongo.ts"; +import { EgressStateModel } from "../../src/domain/egress/egress-state.model.ts"; + +// ── Real-Mongo harness ───────────────────────────────────────────────────────── +// The transitions here are the guarantees several worker processes rely on: exactly one +// caller flips ok -> degraded (and degraded -> ok), and a plain read never writes. Both are +// MongoDB filter/update semantics, so they need a live engine. + +let mongod: MongoMemoryServer; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create(); + await mongoose.connect(mongod.getUri()); + await EgressStateModel.init(); // unique index on singleton +}, 120_000); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +beforeEach(async () => { + await EgressStateModel.deleteMany({}); +}); + +const probe = (target: string, reachable: boolean) => ({ target, reachable, responseTime: 5, message: reachable ? "Success" : "timeout" }); +const NOW = new Date("2026-01-01T10:00:00.000Z"); +const LATER = new Date("2026-01-01T10:05:00.000Z"); + +describe("MongoEgressStateRepository", () => { + const repo = new MongoEgressStateRepository(); + + describe("findSingleton", () => { + it("creates the ok singleton on first call and returns the same document afterwards", async () => { + const first = await repo.findSingleton(); + const second = await repo.findSingleton(); + + expect(first.status).toBe("ok"); + expect(second.id).toBe(first.id); + expect(await EgressStateModel.countDocuments()).toBe(1); + }); + + it("does not bump updatedAt on a read", async () => { + const created = await repo.findSingleton(); + await new Promise((resolve) => setTimeout(resolve, 20)); + + const read = await repo.findSingleton(); + + expect(read.updatedAt).toBe(created.updatedAt); + }); + + it("survives two processes racing the first insert", async () => { + const [a, b] = await Promise.all([repo.findSingleton(), repo.findSingleton()]); + + expect(a.id).toBe(b.id); + expect(await EgressStateModel.countDocuments()).toBe(1); + }); + }); + + describe("markDegraded / markRecovered", () => { + it("markDegraded does nothing when the singleton has never been created (callers read it first)", async () => { + expect(await repo.markDegraded([probe("1.1.1.1", false)], NOW)).toBeNull(); + expect(await EgressStateModel.countDocuments()).toBe(0); + }); + + it("lets exactly one caller perform each transition", async () => { + await repo.findSingleton(); + + const [d1, d2] = await Promise.all([repo.markDegraded([probe("1.1.1.1", false)], NOW), repo.markDegraded([probe("1.1.1.1", false)], NOW)]); + expect([d1, d2].filter((result) => result !== null)).toHaveLength(1); + expect((await repo.findSingleton()).status).toBe("degraded"); + + const [r1, r2] = await Promise.all([repo.markRecovered([probe("1.1.1.1", true)], LATER), repo.markRecovered([probe("1.1.1.1", true)], LATER)]); + expect([r1, r2].filter((result) => result !== null)).toHaveLength(1); + const state = await repo.findSingleton(); + expect(state.status).toBe("ok"); + expect(state.degradedSince).toBe(NOW.toISOString()); + expect(state.lastRecoveredAt).toBe(LATER.toISOString()); + }); + + it("markRecovered is a no-op while ok and markDegraded is a no-op while degraded", async () => { + await repo.findSingleton(); + expect(await repo.markRecovered([probe("1.1.1.1", true)], NOW)).toBeNull(); + + await repo.markDegraded([probe("1.1.1.1", false)], NOW); + expect(await repo.markDegraded([probe("1.1.1.1", false)], LATER)).toBeNull(); + expect((await repo.findSingleton()).degradedSince).toBe(NOW.toISOString()); + }); + }); + + describe("recordProbe", () => { + it("stores the probe without changing the status", async () => { + await repo.findSingleton(); + await repo.markDegraded([probe("1.1.1.1", false)], NOW); + + const state = await repo.recordProbe([probe("1.1.1.1", false), probe("8.8.8.8", false)], LATER); + + expect(state.status).toBe("degraded"); + expect(state.lastProbeAt).toBe(LATER.toISOString()); + expect(state.lastProbeResults.map((result) => result.target)).toEqual(["1.1.1.1", "8.8.8.8"]); + }); + }); + + describe("reset", () => { + it("returns a degraded singleton to ok with no episode and keeps the recovery history", async () => { + await repo.findSingleton(); + await repo.markDegraded([probe("1.1.1.1", false)], NOW); + await repo.markRecovered([probe("1.1.1.1", true)], LATER); + await repo.markDegraded([probe("1.1.1.1", false)], LATER); + + const state = await repo.reset(); + + expect(state.status).toBe("ok"); + expect(state.degradedSince).toBeNull(); + expect(state.lastProbeAt).toBeNull(); + expect(state.lastProbeResults).toEqual([]); + expect(state.lastRecoveredAt).toBe(LATER.toISOString()); + }); + + it("creates the singleton when none exists", async () => { + const state = await repo.reset(); + + expect(state.status).toBe("ok"); + expect(await EgressStateModel.countDocuments()).toBe(1); + }); + }); +}); diff --git a/server/test/integration/jobsRepository.test.ts b/server/test/integration/jobsRepository.test.ts index 78ac17b64..e2b426ce4 100644 --- a/server/test/integration/jobsRepository.test.ts +++ b/server/test/integration/jobsRepository.test.ts @@ -533,4 +533,47 @@ describe("MongoJobsRepository", () => { expect(await repo.countDueBacklog(NOW)).toBe(0); }); }); + + // ── global rows: egress recovery job ────────────────────────────────────── + + describe("global rows", () => { + const repo = new MongoJobsRepository(WORKER_ID); + const egressRow = (nextScheduledAt: number) => + seedJob({ _id: "egress", type: "egress" as JobType, refId: null, nextScheduledAt, intervalMs: 30_000 }); + + it("deleteGlobalJob removes the row by its canonical _id and leaves other global rows alone", async () => { + await egressRow(NOW); + await seedJob({ _id: "cleanup-orphaned", type: "cleanup-orphaned" as JobType, refId: null }); + + expect(await repo.deleteGlobalJob("egress")).toBe(true); + + expect(await readRow("egress")).toBeNull(); + expect(await readRow("cleanup-orphaned")).not.toBeNull(); + expect(await repo.deleteGlobalJob("egress")).toBe(false); + }); + + it("deleteGlobalJobIfUnchanged removes the row only while nextScheduledAt matches the claimed value", async () => { + await egressRow(NOW); + + // Another worker started a new episode and rescheduled the row after this run claimed it + await JobModel.updateOne({ _id: "egress" }, { $set: { nextScheduledAt: NOW + 30_000 } }); + expect(await repo.deleteGlobalJobIfUnchanged("egress", NOW)).toBe(false); + expect(await readRow("egress")).not.toBeNull(); + + // Nobody touched it: the row goes + expect(await repo.deleteGlobalJobIfUnchanged("egress", NOW + 30_000)).toBe(true); + expect(await readRow("egress")).toBeNull(); + }); + + it("recordSuccess after a refused conditional delete is a no-op once the lock has moved on", async () => { + await egressRow(NOW); + const [claimed] = await repo.claimDueBatch("egress", 1, NOW); + expect(claimed?.id).toBe("egress"); + + // Row deleted by the handler (recovered), then recordSuccess runs from the queue: nothing to update + expect(await repo.deleteGlobalJobIfUnchanged("egress", claimed!.nextScheduledAt)).toBe(true); + expect(await repo.recordSuccess("egress", claimed!.nextScheduledAt, 30_000, NOW + 1_000)).toBe(false); + expect(await readRow("egress")).toBeNull(); + }); + }); }); diff --git a/server/test/unit/providers/network/pingProvider.test.ts b/server/test/unit/providers/network/pingProvider.test.ts index 6b4129850..150472925 100644 --- a/server/test/unit/providers/network/pingProvider.test.ts +++ b/server/test/unit/providers/network/pingProvider.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, jest } from "@jest/globals"; import { PingProvider } from "../../../../src/service/network/PingProvider.ts"; import { testStatusProviderContract } from "../../../helpers/statusProviderContract.ts"; import type { Monitor } from "../../../../src/domain/monitors/monitor.type.ts"; +import * as net from "net"; // ── Helpers ────────────────────────────────────────────────────────────────── @@ -27,7 +28,7 @@ const createMockPing = (result?: Partial<{ alive: boolean; time: number | string // ── Contract ───────────────────────────────────────────────────────────────── testStatusProviderContract("PingProvider", { - create: () => new PingProvider(createMockPing() as any), + create: () => new PingProvider(createMockPing() as any, net), supportedType: "ping", unsupportedType: "http", makeMonitor: () => makeMonitor(), @@ -37,7 +38,7 @@ testStatusProviderContract("PingProvider", { describe("PingProvider", () => { it("returns success when host is alive", async () => { - const provider = new PingProvider(createMockPing({ alive: true, time: 30 }) as any); + const provider = new PingProvider(createMockPing({ alive: true, time: 30 }) as any, net); const result = await provider.handle(makeMonitor()); @@ -55,7 +56,7 @@ describe("PingProvider", () => { }); it("returns failure when host is not alive", async () => { - const provider = new PingProvider(createMockPing({ alive: false, time: 0 }) as any); + const provider = new PingProvider(createMockPing({ alive: false, time: 0 }) as any, net); const result = await provider.handle(makeMonitor()); @@ -69,7 +70,7 @@ describe("PingProvider", () => { }); it("parses string time values", async () => { - const provider = new PingProvider(createMockPing({ alive: true, time: "42.5" }) as any); + const provider = new PingProvider(createMockPing({ alive: true, time: "42.5" }) as any, net); const result = await provider.handle(makeMonitor()); @@ -77,7 +78,7 @@ describe("PingProvider", () => { }); it("defaults responseTime to 0 for non-numeric time", async () => { - const provider = new PingProvider(createMockPing({ alive: true, time: "unknown" }) as any); + const provider = new PingProvider(createMockPing({ alive: true, time: "unknown" }) as any, net); const result = await provider.handle(makeMonitor()); @@ -85,7 +86,7 @@ describe("PingProvider", () => { }); it("defaults status to false when alive is undefined", async () => { - const provider = new PingProvider(createMockPing({ alive: undefined as any }) as any); + const provider = new PingProvider(createMockPing({ alive: undefined as any }) as any, net); const result = await provider.handle(makeMonitor()); @@ -94,7 +95,7 @@ describe("PingProvider", () => { it("sanitizes URL by stripping protocol and path", async () => { const mockPing = createMockPing(); - const provider = new PingProvider(mockPing as any); + const provider = new PingProvider(mockPing as any, net); await provider.handle(makeMonitor({ url: "https://example.com/path?query=1" })); @@ -103,7 +104,7 @@ describe("PingProvider", () => { it("sanitizes URL by stripping port", async () => { const mockPing = createMockPing(); - const provider = new PingProvider(mockPing as any); + const provider = new PingProvider(mockPing as any, net); await provider.handle(makeMonitor({ url: "example.com:8080" })); @@ -112,7 +113,7 @@ describe("PingProvider", () => { it("keeps a bare IPv6 address intact", async () => { const mockPing = createMockPing(); - const provider = new PingProvider(mockPing as any); + const provider = new PingProvider(mockPing as any, net); await provider.handle(makeMonitor({ url: "2606:4700:4700::1111" })); @@ -121,7 +122,7 @@ describe("PingProvider", () => { it("unwraps a bracketed IPv6 address and drops its port", async () => { const mockPing = createMockPing(); - const provider = new PingProvider(mockPing as any); + const provider = new PingProvider(mockPing as any, net); await provider.handle(makeMonitor({ url: "[2606:4700:4700::1111]:53" })); @@ -129,28 +130,28 @@ describe("PingProvider", () => { }); it("throws AppError when url is missing", async () => { - const provider = new PingProvider(createMockPing() as any); + const provider = new PingProvider(createMockPing() as any, net); await expect(provider.handle(makeMonitor({ url: "" }))).rejects.toThrow("URL is required for ping monitor"); }); it("throws AppError when ping.probe rejects", async () => { const mockPing = { promise: { probe: jest.fn().mockRejectedValue(new Error("Host unreachable")) } }; - const provider = new PingProvider(mockPing as any); + const provider = new PingProvider(mockPing as any, net); await expect(provider.handle(makeMonitor())).rejects.toThrow("Host unreachable"); }); it("throws AppError when probe returns no response", async () => { const mockPing = { promise: { probe: jest.fn().mockResolvedValue(null) } }; - const provider = new PingProvider(mockPing as any); + const provider = new PingProvider(mockPing as any, net); await expect(provider.handle(makeMonitor())).rejects.toThrow("No response from ping"); }); it("throws AppError with stringified message for non-Error throws", async () => { const mockPing = { promise: { probe: jest.fn().mockRejectedValue(42) } }; - const provider = new PingProvider(mockPing as any); + const provider = new PingProvider(mockPing as any, net); await expect(provider.handle(makeMonitor())).rejects.toThrow("42"); }); diff --git a/server/test/unit/services/egressService.test.ts b/server/test/unit/services/egressService.test.ts index 8aee753d7..72f76ac02 100644 --- a/server/test/unit/services/egressService.test.ts +++ b/server/test/unit/services/egressService.test.ts @@ -146,6 +146,27 @@ describe("EgressService", () => { ]); }); + it("treats any HTTP response, even 4xx/5xx, as reachable but a transport failure as unreachable", async () => { + const requestStatus = jest.fn().mockImplementation(async (monitor: any) => ({ + monitorId: monitor.id, + teamId: monitor.teamId, + type: monitor.type, + status: false, + code: monitor.url.includes("maintenance") ? 503 : 5000, // 5000 is HttpProvider's NETWORK_ERROR + message: monitor.url.includes("maintenance") ? "Service Unavailable" : "ECONNREFUSED", + responseTime: 3, + })); + const { service } = createService({ networkService: { requestStatus } }); + + const results = await service.probeTargets(["https://example.com/maintenance", "https://example.com/refused", "8.8.8.8:53"]); + + expect(results).toEqual([ + expect.objectContaining({ target: "https://example.com/maintenance", reachable: true }), + expect.objectContaining({ target: "https://example.com/refused", reachable: false }), + expect.objectContaining({ target: "8.8.8.8:53", reachable: false }), // non-http: only status true counts + ]); + }); + it("counts an unparseable stored target as unreachable without probing", async () => { const { service, defaults } = createService(); @@ -218,7 +239,6 @@ describe("EgressService", () => { ); expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); expect(defaults.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("egress degraded") })); - // The worker that performed the transition sets the schedule expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith({ id: "egress", type: "egress", @@ -228,9 +248,13 @@ describe("EgressService", () => { intervalMs: 30_000, }); expect(defaults.jobsRepository.upsertJob).not.toHaveBeenCalled(); + // Job first, transition second: a crash in between leaves a stray row, never an unpolled degraded state + const scheduleOrder = (defaults.jobsRepository.upsertCleanupJob as jest.Mock).mock.invocationCallOrder[0]!; + const transitionOrder = (defaults.egressStateRepository.markDegraded as jest.Mock).mock.invocationCallOrder[0]!; + expect(scheduleOrder).toBeLessThan(transitionOrder); }); - it("only makes sure the job row exists when another worker performed the transition", async () => { + it("still returns degraded and leaves the job scheduled when another worker performed the transition first", async () => { const { service, defaults } = createService({ networkService: { requestStatus: statusFor([]) }, egressStateRepository: degradedRepository({ @@ -242,8 +266,22 @@ describe("EgressService", () => { const result = await service.assessAfterFailure(); expect(result).toBe("degraded"); - expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 })); - expect(defaults.jobsRepository.upsertCleanupJob).not.toHaveBeenCalled(); + expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 })); + expect(defaults.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("already recorded") })); + }); + + it("does not mark degraded when the recovery job cannot be scheduled", async () => { + const { service, defaults } = createService({ + networkService: { requestStatus: statusFor([]) }, + jobsRepository: { + upsertJob: jest.fn(), + upsertCleanupJob: jest.fn().mockRejectedValue(new Error("db down")), + deleteGlobalJobIfUnchanged: jest.fn(), + }, + }); + + expect(await service.assessAfterFailure()).toBeNull(); + expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled(); }); it("returns degraded without probing when the persisted state is already degraded, without touching the job's schedule", async () => { diff --git a/server/test/unit/services/egressStateService.test.ts b/server/test/unit/services/egressStateService.test.ts index a75c0de1f..1701098d2 100644 --- a/server/test/unit/services/egressStateService.test.ts +++ b/server/test/unit/services/egressStateService.test.ts @@ -13,7 +13,7 @@ const createEgressStateRepo = () => ({ }); const createJobsRepo = () => ({ - deleteByIdAndType: jest.fn().mockResolvedValue(true), + deleteGlobalJob: jest.fn().mockResolvedValue(true), }); const createService = () => { @@ -82,7 +82,7 @@ describe("EgressStateService", () => { const result = await service.reset(); - expect(jobsRepository.deleteByIdAndType).toHaveBeenCalledWith(null, "egress"); + expect(jobsRepository.deleteGlobalJob).toHaveBeenCalledWith("egress"); expect(egressStateRepository.reset).toHaveBeenCalledTimes(1); expect(result).toEqual(reset); }); diff --git a/server/test/unit/services/notificationsService.test.ts b/server/test/unit/services/notificationsService.test.ts index f2462866b..d0fdd61b2 100644 --- a/server/test/unit/services/notificationsService.test.ts +++ b/server/test/unit/services/notificationsService.test.ts @@ -26,10 +26,9 @@ const createMonitorsRepo = () => ({ removeNotificationFromMonitors: jest.fn(), }); -const createSettingsService = (clientHost = "https://app.example.com", egressNotifications: string[] = []) => ({ +const createSettingsService = (clientHost = "https://app.example.com") => ({ getSettings: jest.fn().mockReturnValue({ clientHost }), - getDBSettings: jest.fn().mockResolvedValue({ egressNotifications }), - updateDbSettings: jest.fn().mockResolvedValue({ egressNotifications: [] }), + removeEgressNotification: jest.fn().mockResolvedValue(undefined), }); const createMessageBuilder = () => ({ @@ -374,24 +373,25 @@ describe("NotificationsService", () => { expect(monitorsRepository.removeNotificationFromMonitors).toHaveBeenCalledWith("notif-1"); }); - it("removes the notification from the egress recovered recipients in app settings", async () => { - const settingsService = createSettingsService("https://app.example.com", ["notif-0", "notif-1"]); - const { service, notificationsRepository } = createService({ settingsService }); + it("removes the notification from the egress recovered recipients after the team-scoped delete succeeds", async () => { + const { service, notificationsRepository, settingsService } = createService(); (notificationsRepository.deleteById as jest.Mock).mockResolvedValue(makeNotification()); await service.deleteById("notif-1", "team-1"); - expect(settingsService.updateDbSettings).toHaveBeenCalledWith({ egressNotifications: ["notif-0"] }); + expect(settingsService.removeEgressNotification).toHaveBeenCalledWith("notif-1"); + const deleteOrder = (notificationsRepository.deleteById as jest.Mock).mock.invocationCallOrder[0]!; + const cascadeOrder = (settingsService.removeEgressNotification as jest.Mock).mock.invocationCallOrder[0]!; + expect(deleteOrder).toBeLessThan(cascadeOrder); }); - it("leaves app settings alone when the notification is not an egress recipient", async () => { - const settingsService = createSettingsService("https://app.example.com", ["notif-0"]); - const { service, notificationsRepository } = createService({ settingsService }); - (notificationsRepository.deleteById as jest.Mock).mockResolvedValue(makeNotification()); + it("does not touch app settings when the delete is refused", async () => { + const { service, notificationsRepository, settingsService } = createService(); + (notificationsRepository.deleteById as jest.Mock).mockRejectedValue(new Error("Notification not found")); - await service.deleteById("notif-1", "team-1"); + await expect(service.deleteById("notif-1", "other-team")).rejects.toThrow("Notification not found"); - expect(settingsService.updateDbSettings).not.toHaveBeenCalled(); + expect(settingsService.removeEgressNotification).not.toHaveBeenCalled(); }); }); }); diff --git a/server/test/unit/services/settingsService.test.ts b/server/test/unit/services/settingsService.test.ts index 92a31a89b..9e611c247 100644 --- a/server/test/unit/services/settingsService.test.ts +++ b/server/test/unit/services/settingsService.test.ts @@ -43,6 +43,7 @@ const createSettingsRepo = () => create: jest.fn().mockResolvedValue(makeSettings()), findSingleton: jest.fn().mockResolvedValue(makeSettings()), update: jest.fn().mockResolvedValue(makeSettings()), + removeEgressNotification: jest.fn().mockResolvedValue(undefined), deleteLegacy: jest.fn().mockResolvedValue(true), }) as unknown as jest.Mocked; @@ -192,4 +193,20 @@ describe("SettingsService", () => { await expect(service.updateDbSettings({ checkTTL: 60 })).rejects.toThrow("Settings repository not initialized"); }); }); + + describe("removeEgressNotification", () => { + it("delegates the pull to the repository", async () => { + const { service, settingsRepository } = createService(); + + await service.removeEgressNotification("notif-1"); + + expect(settingsRepository.removeEgressNotification).toHaveBeenCalledWith("notif-1"); + }); + + it("throws when repository is not set", async () => { + const service = new SettingsService(makeEnv()); + + await expect(service.removeEgressNotification("notif-1")).rejects.toThrow("Settings repository not initialized"); + }); + }); });