From ec01a5dfe4791992f7178238b0515a3fd62a77cf Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Mon, 14 Sep 2026 10:26:09 +0100 Subject: [PATCH 1/4] Add egress state and probe service (#3937) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the EgressService that probes a configurable set of reliability targets (ping, TCP or HTTP) when asked, persists a single instance-wide egress state document, and runs a recovery poll loop while egress is degraded. Adds the four egress settings fields with validation. The worker constructs and initialises the service but nothing calls assessAfterFailure yet, so behaviour is unchanged. 🔧 --- server/openapi.json | 25 ++ .../src/api/validation/settingsValidation.ts | 14 + server/src/config/services.api.ts | 5 + server/src/config/services.shared.ts | 5 + server/src/config/services.worker.ts | 9 +- .../domain/app-settings/app-settings.model.ts | 5 + .../app-settings.repository.mongo.ts | 5 + .../domain/app-settings/app-settings.type.ts | 5 + .../src/domain/egress/egress-state.model.ts | 40 ++ .../egress-state.repository.interface.ts | 12 + .../egress/egress-state.repository.mongo.ts | 67 +++ .../src/domain/egress/egress-state.service.ts | 20 + server/src/domain/egress/egress.service.ts | 290 +++++++++++++ server/src/domain/egress/egress.type.ts | 33 ++ .../test/unit/services/egressService.test.ts | 404 ++++++++++++++++++ .../unit/services/egressStateService.test.ts | 70 +++ .../validation/settingsValidation.test.ts | 72 ++++ 17 files changed, 1080 insertions(+), 1 deletion(-) create mode 100644 server/src/domain/egress/egress-state.model.ts create mode 100644 server/src/domain/egress/egress-state.repository.interface.ts create mode 100644 server/src/domain/egress/egress-state.repository.mongo.ts create mode 100644 server/src/domain/egress/egress-state.service.ts create mode 100644 server/src/domain/egress/egress.service.ts create mode 100644 server/src/domain/egress/egress.type.ts create mode 100644 server/test/unit/services/egressService.test.ts create mode 100644 server/test/unit/services/egressStateService.test.ts create mode 100644 server/test/unit/validation/settingsValidation.test.ts diff --git a/server/openapi.json b/server/openapi.json index 4b4faf896..d0ae81f26 100644 --- a/server/openapi.json +++ b/server/openapi.json @@ -13294,6 +13294,31 @@ "globalProxyId": { "type": "string", "nullable": true + }, + "egressCheckEnabled": { + "type": "boolean" + }, + "egressCheckTargets": { + "type": "array", + "items": { + "type": "string", + "minLength": 1, + "maxLength": 253, + "pattern": "^(https?:\\/\\/\\S+|[A-Za-z0-9.\\-:[\\]]+)$" + }, + "maxItems": 10 + }, + "egressPollIntervalSeconds": { + "type": "integer", + "minimum": 5, + "maximum": 600 + }, + "egressNotifications": { + "type": "array", + "items": { + "type": "string", + "pattern": "^[a-f\\d]{24}$/i" + } } } } diff --git a/server/src/api/validation/settingsValidation.ts b/server/src/api/validation/settingsValidation.ts index 50c75b043..33fe433fc 100644 --- a/server/src/api/validation/settingsValidation.ts +++ b/server/src/api/validation/settingsValidation.ts @@ -1,6 +1,15 @@ import { CHECK_TTL_SENTINEL } from "@/domain/checks/check.type.js"; +import { MAX_EGRESS_POLL_INTERVAL_SECONDS, MAX_EGRESS_TARGETS, MIN_EGRESS_POLL_INTERVAL_SECONDS } from "@/domain/egress/egress.type.js"; import { z } from "zod"; +// A reliability target is a bare host/IP (ICMP ping), host:port (TCP connect) or an http(s) URL. +const egressTargetValidation = z + .string() + .trim() + .min(1) + .max(253) + .regex(/^(https?:\/\/\S+|[A-Za-z0-9.\-:[\]]+)$/, "Invalid egress target"); + //**************************************** // Settings Validations //**************************************** @@ -42,6 +51,11 @@ export const updateAppSettingsBodyValidation = z .optional(), globalProxyEnabled: z.boolean().optional(), globalProxyId: z.string().nullable().optional(), + + egressCheckEnabled: z.boolean().optional(), + egressCheckTargets: z.array(egressTargetValidation).max(MAX_EGRESS_TARGETS).optional(), // empty list falls back to DEFAULT_EGRESS_TARGETS + egressPollIntervalSeconds: z.number().int().min(MIN_EGRESS_POLL_INTERVAL_SECONDS).max(MAX_EGRESS_POLL_INTERVAL_SECONDS).optional(), + egressNotifications: z.array(z.string().regex(/^[a-f\d]{24}$/i, "Invalid notification id")).optional(), }) .strip() .superRefine((body, ctx) => { diff --git a/server/src/config/services.api.ts b/server/src/config/services.api.ts index c5f58ac19..e929fbfe8 100644 --- a/server/src/config/services.api.ts +++ b/server/src/config/services.api.ts @@ -7,6 +7,7 @@ import { ITagsService, TagsService } from "@/domain/tags/tag.service.js"; import { IUserService, UserService } from "@/domain/users/user.service.js"; import { IJobScheduler } from "@/worker/worker.interface.js"; import { ProxiesService, IProxiesService } from "@/domain/proxies/proxy.service.js"; +import { EgressStateService, IEgressStateService } from "@/domain/egress/egress-state.service.js"; import { SharedServices } from "@/config/services.shared.js"; // Third-party @@ -24,6 +25,7 @@ export interface ApiServices extends SharedServices { tagsService: ITagsService; diagnosticService: IDiagnosticService; proxiesService: IProxiesService; + egressStateService: IEgressStateService; } export const buildApi = (shared: SharedServices, jobScheduler: IJobScheduler): ApiServices => { @@ -48,6 +50,7 @@ export const buildApi = (shared: SharedServices, jobScheduler: IJobScheduler): A maintenanceWindowsRepository, jobsRepository, proxiesRepository, + egressStateRepository, } = shared; const userService = new UserService({ @@ -100,6 +103,7 @@ export const buildApi = (shared: SharedServices, jobScheduler: IJobScheduler): A const tagsService = new TagsService(tagsRepository, monitorsRepository); const diagnosticService = new DiagnosticService(db); const proxiesService = new ProxiesService(proxiesRepository, monitorsRepository, settingsService); + const egressStateService = new EgressStateService(egressStateRepository); return { ...shared, worker: jobScheduler, @@ -111,5 +115,6 @@ export const buildApi = (shared: SharedServices, jobScheduler: IJobScheduler): A tagsService, diagnosticService, proxiesService, + egressStateService, }; }; diff --git a/server/src/config/services.shared.ts b/server/src/config/services.shared.ts index 3b4fdb147..a1a69566b 100644 --- a/server/src/config/services.shared.ts +++ b/server/src/config/services.shared.ts @@ -56,6 +56,7 @@ import { ITagsRepository } from "@/domain/tags/tag.repository.interface.js"; import { ITeamsRepository } from "@/domain/teams/team.repository.interface.js"; import { IUsersRepository } from "@/domain/users/user.repository.interface.js"; import { IProxiesRepository } from "@/domain/proxies/proxy.repository.interface.js"; +import { IEgressStateRepository } from "@/domain/egress/egress-state.repository.interface.js"; // Mongo repository implementations import MongoSettingsRepository from "@/domain/app-settings/app-settings.repository.mongo.js"; @@ -76,6 +77,7 @@ import MongoTagsRepository from "@/domain/tags/tag.repository.mongo.js"; import MongoTeamsRepository from "@/domain/teams/team.repository.model.js"; import MongoUsersRepository from "@/domain/users/user.repository.mongo.js"; import MongoProxiesRepository from "@/domain/proxies/proxy.repository.mongo.js"; +import MongoEgressStateRepository from "@/domain/egress/egress-state.repository.mongo.js"; import { IDockerLogsRepository } from "@/domain/docker/docker-log.repository.interface.js"; // Shared infrastructure + business services that both the API and the worker process construct. @@ -114,6 +116,7 @@ export interface SharedServices { teamsRepository: ITeamsRepository; maintenanceWindowsRepository: IMaintenanceWindowsRepository; proxiesRepository: IProxiesRepository; + egressStateRepository: IEgressStateRepository; } export const buildShared = async ({ @@ -157,6 +160,7 @@ export const buildShared = async ({ const teamsRepository = new MongoTeamsRepository(); const maintenanceWindowsRepository = new MongoMaintenanceWindowsRepository(); const proxiesRepository = new MongoProxiesRepository(); + const egressStateRepository = new MongoEgressStateRepository(); // Inject settings repository into settings service (now that DB is connected) (settingsService as SettingsService).setRepository(settingsRepository); @@ -246,6 +250,7 @@ export const buildShared = async ({ teamsRepository, maintenanceWindowsRepository, proxiesRepository, + egressStateRepository, }; return sharedServices; }; diff --git a/server/src/config/services.worker.ts b/server/src/config/services.worker.ts index acd4a944a..83a875f05 100644 --- a/server/src/config/services.worker.ts +++ b/server/src/config/services.worker.ts @@ -28,6 +28,7 @@ import { IncidentReactor } from "@/worker/reactors/reactor.incident.js"; import { ReactorDispatcher } from "@/worker/reactors/reactor.dispatcher.js"; import { DBQueueWorker } from "@/worker/worker.db-queue.js"; import { ProxyResolver } from "@/service/network/ProxyResolver.js"; +import { IEgressService, EgressService } from "@/domain/egress/egress.service.js"; // Network providers import { PingProvider } from "@/service/network/PingProvider.js"; import { HttpProvider } from "@/service/network/HttpProvider.js"; @@ -46,6 +47,7 @@ export interface WorkerServices { networkService: INetworkService; bufferService: IBufferService; statusService: IStatusService; + egressService: IEgressService; } export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig): Promise => { @@ -70,6 +72,7 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig teamsRepository, maintenanceWindowsRepository, proxiesRepository, + egressStateRepository, } = shared; // *********************** @@ -103,6 +106,7 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig const bufferService = new BufferService(logger, checkService, geoChecksService, dockerLogsService, settingsService, jobsRepository); const statusService = new StatusService(logger, monitorsRepository, monitorStatsRepository); const monitorStatusPolicy = new MonitorStatusPolicy(); + const egressService = new EgressService(settingsService, egressStateRepository, networkService, logger); // *********************** // Reactors and dispatcher @@ -185,5 +189,8 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig }); } - return { worker, networkService, bufferService, statusService }; + // Resume recovery polling if the instance was mid-episode when it last stopped + await egressService.init(); + + return { worker, networkService, bufferService, statusService, egressService }; }; diff --git a/server/src/domain/app-settings/app-settings.model.ts b/server/src/domain/app-settings/app-settings.model.ts index 3dd3c3abe..dfe138b8f 100644 --- a/server/src/domain/app-settings/app-settings.model.ts +++ b/server/src/domain/app-settings/app-settings.model.ts @@ -1,5 +1,6 @@ import { Schema, model, type Types } from "mongoose"; import type { Settings, SettingsThresholds } from "@/domain/app-settings/app-settings.type.js"; +import { DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; interface AppSettingsDocument extends Omit { _id: Types.ObjectId; @@ -42,6 +43,10 @@ const AppSettingsSchema = new Schema( globalThresholds: { type: thresholdsSchema }, globalProxyEnabled: { type: Boolean, default: false }, globalProxyId: { type: String, default: null }, + egressCheckEnabled: { type: Boolean, default: false }, + egressCheckTargets: { type: [String], default: () => [...DEFAULT_EGRESS_TARGETS] }, + egressPollIntervalSeconds: { type: Number, default: DEFAULT_EGRESS_POLL_INTERVAL_SECONDS }, + egressNotifications: { type: [String], default: () => [] }, // Notification ids, stored as strings like globalProxyId }, { timestamps: true } ); 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 d80289a7b..f2ef3351c 100644 --- a/server/src/domain/app-settings/app-settings.repository.mongo.ts +++ b/server/src/domain/app-settings/app-settings.repository.mongo.ts @@ -3,6 +3,7 @@ import { ISettingsRepository } from "@/domain/app-settings/app-settings-reposito import type { Settings, SettingsUpdate } from "@/domain/app-settings/app-settings.type.js"; import { AppSettingsModel, type AppSettingsDocument } from "@/domain/app-settings/app-settings.model.js"; import { toStringId, toDateString } from "@/utils/mongoMappers.js"; +import { DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; class MongoSettingsRepository implements ISettingsRepository { private toEntity = (doc: AppSettingsDocument): Settings => { @@ -30,6 +31,10 @@ class MongoSettingsRepository implements ISettingsRepository { globalThresholds: doc.globalThresholds ?? undefined, globalProxyEnabled: doc.globalProxyEnabled ?? false, globalProxyId: doc.globalProxyId ?? undefined, + egressCheckEnabled: doc.egressCheckEnabled ?? false, + egressCheckTargets: doc.egressCheckTargets ?? [...DEFAULT_EGRESS_TARGETS], + egressPollIntervalSeconds: doc.egressPollIntervalSeconds ?? DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, + egressNotifications: doc.egressNotifications ?? [], createdAt: toDateString(doc.createdAt), updatedAt: toDateString(doc.updatedAt), }; diff --git a/server/src/domain/app-settings/app-settings.type.ts b/server/src/domain/app-settings/app-settings.type.ts index e4d2f9559..9dc4eae45 100644 --- a/server/src/domain/app-settings/app-settings.type.ts +++ b/server/src/domain/app-settings/app-settings.type.ts @@ -51,6 +51,11 @@ export interface Settings { globalThresholds?: SettingsThresholds; globalProxyEnabled: boolean; globalProxyId?: string | null; + // Egress self-check (see domain/egress) + egressCheckEnabled: boolean; + egressCheckTargets: string[]; + egressPollIntervalSeconds: number; // recovery poll cadence while degraded, not a general schedule + egressNotifications: string[]; // Notification ids to receive the "egress recovered" alert createdAt: string; updatedAt: string; } diff --git a/server/src/domain/egress/egress-state.model.ts b/server/src/domain/egress/egress-state.model.ts new file mode 100644 index 000000000..14f7cafe8 --- /dev/null +++ b/server/src/domain/egress/egress-state.model.ts @@ -0,0 +1,40 @@ +import { Schema, model, type Types } from "mongoose"; +import { EgressStatuses, type EgressProbeResult, type EgressState } from "@/domain/egress/egress.type.js"; + +interface EgressStateDocument extends Omit { + _id: Types.ObjectId; + singleton: boolean; + degradedSince: Date | null; + lastRecoveredAt: Date | null; + lastProbeAt: Date | null; + createdAt: Date; + updatedAt: Date; +} + +const probeResultSchema = new Schema( + { + target: { type: String, required: true }, + reachable: { type: Boolean, required: true }, + responseTime: { type: Number, default: 0 }, + message: { type: String }, + }, + { _id: false } +); + +const EgressStateSchema = new Schema( + { + singleton: { type: Boolean, required: true, unique: true, default: true }, + status: { type: String, enum: EgressStatuses, required: true, default: "ok" }, + degradedSince: { type: Date, default: null }, + lastRecoveredAt: { type: Date, default: null }, + lastProbeAt: { type: Date, default: null }, + lastProbeResults: { type: [probeResultSchema], default: () => [] }, + }, + { timestamps: true } +); + +const EgressStateModel = model("EgressState", EgressStateSchema); + +export type { EgressStateDocument }; +export { EgressStateModel }; +export default EgressStateModel; diff --git a/server/src/domain/egress/egress-state.repository.interface.ts b/server/src/domain/egress/egress-state.repository.interface.ts new file mode 100644 index 000000000..533aad656 --- /dev/null +++ b/server/src/domain/egress/egress-state.repository.interface.ts @@ -0,0 +1,12 @@ +import type { EgressProbeResult, EgressState } from "@/domain/egress/egress.type.js"; + +export interface IEgressStateRepository { + // read + findSingleton(): Promise; // creates the "ok" singleton on first call + // update + recordProbe(results: EgressProbeResult[], now: Date): Promise; + // Atomic transitions. Each returns the updated state when this caller performed the transition, + // or null when the state was already on the target side (another worker got there first). + markDegraded(results: EgressProbeResult[], now: Date): Promise; + markRecovered(results: EgressProbeResult[], now: Date): Promise; +} diff --git a/server/src/domain/egress/egress-state.repository.mongo.ts b/server/src/domain/egress/egress-state.repository.mongo.ts new file mode 100644 index 000000000..606551e5a --- /dev/null +++ b/server/src/domain/egress/egress-state.repository.mongo.ts @@ -0,0 +1,67 @@ +import { IEgressStateRepository } from "@/domain/egress/egress-state.repository.interface.js"; +import type { EgressProbeResult, EgressState } from "@/domain/egress/egress.type.js"; +import { EgressStateModel, type EgressStateDocument } from "@/domain/egress/egress-state.model.js"; +import { toStringId, toDateString } from "@/utils/mongoMappers.js"; + +const SERVICE_NAME = "EgressStateRepository"; + +class MongoEgressStateRepository implements IEgressStateRepository { + static SERVICE_NAME = SERVICE_NAME; + + private toEntity = (doc: EgressStateDocument): EgressState => { + return { + id: toStringId(doc._id), + status: doc.status, + degradedSince: doc.degradedSince ? toDateString(doc.degradedSince) : null, + lastRecoveredAt: doc.lastRecoveredAt ? toDateString(doc.lastRecoveredAt) : null, + lastProbeAt: doc.lastProbeAt ? toDateString(doc.lastProbeAt) : null, + lastProbeResults: (doc.lastProbeResults ?? []).map((result) => ({ + target: result.target, + reachable: result.reachable, + responseTime: result.responseTime ?? 0, + ...(result.message !== undefined && { message: result.message }), + })), + createdAt: toDateString(doc.createdAt), + updatedAt: toDateString(doc.updatedAt), + }; + }; + + findSingleton = async () => { + const doc = await EgressStateModel.findOneAndUpdate( + { singleton: true }, + { $setOnInsert: { singleton: true, status: "ok" } }, + { upsert: true, new: true, setDefaultsOnInsert: true } + ).lean(); + return this.toEntity(doc); + }; + + recordProbe = async (results: EgressProbeResult[], now: Date) => { + const doc = await EgressStateModel.findOneAndUpdate( + { singleton: true }, + { $set: { lastProbeAt: now, lastProbeResults: results }, $setOnInsert: { singleton: true, status: "ok" } }, + { upsert: true, new: true, setDefaultsOnInsert: true } + ).lean(); + return this.toEntity(doc); + }; + + markDegraded = async (results: EgressProbeResult[], now: Date) => { + // Only the caller that flips ok -> degraded sees a document back; everyone else sees null. + const doc = await EgressStateModel.findOneAndUpdate( + { singleton: true, status: "ok" }, + { $set: { status: "degraded", degradedSince: now, lastProbeAt: now, lastProbeResults: results } }, + { new: true } + ).lean(); + return doc ? this.toEntity(doc) : null; + }; + + markRecovered = async (results: EgressProbeResult[], now: Date) => { + const doc = await EgressStateModel.findOneAndUpdate( + { singleton: true, status: "degraded" }, + { $set: { status: "ok", lastRecoveredAt: now, lastProbeAt: now, lastProbeResults: results } }, + { new: true } + ).lean(); + return doc ? this.toEntity(doc) : null; + }; +} + +export default MongoEgressStateRepository; diff --git a/server/src/domain/egress/egress-state.service.ts b/server/src/domain/egress/egress-state.service.ts new file mode 100644 index 000000000..35800c7ac --- /dev/null +++ b/server/src/domain/egress/egress-state.service.ts @@ -0,0 +1,20 @@ +import { IEgressStateRepository } from "@/domain/egress/egress-state.repository.interface.js"; +import type { EgressState } from "@/domain/egress/egress.type.js"; + +export interface IEgressStateService { + getState(): Promise; +} + +const SERVICE_NAME = "EgressStateService"; + +// Read-only view of the instance-global egress state for the API process. Transitions are +// owned by the worker (egress.service.ts); this exists so the controller never touches the repository. +export class EgressStateService implements IEgressStateService { + static SERVICE_NAME = SERVICE_NAME; + + constructor(private egressStateRepository: IEgressStateRepository) {} + + getState = async (): Promise => { + return this.egressStateRepository.findSingleton(); + }; +} diff --git a/server/src/domain/egress/egress.service.ts b/server/src/domain/egress/egress.service.ts new file mode 100644 index 000000000..cb3b79ebb --- /dev/null +++ b/server/src/domain/egress/egress.service.ts @@ -0,0 +1,290 @@ +import type { Monitor } from "@/domain/monitors/monitor.type.js"; +import type { ISettingsService } from "@/domain/app-settings/app-settings.service.js"; +import type { IEgressStateRepository } from "@/domain/egress/egress-state.repository.interface.js"; +import type { INetworkService } from "@/service/networkService.js"; +import type { ILogger } from "@/utils/logger.js"; +import { + DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, + DEFAULT_EGRESS_TARGETS, + type EgressProbeResult, + type EgressStatus, +} from "@/domain/egress/egress.type.js"; +import { timeRequest } from "@/service/network/utils.js"; + +const SERVICE_NAME = "EgressService"; +const PROBE_TIMEOUT_MS = 5000; + +// 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"; + +// "[host]:port" or "host:port" where host has no colons (so a bare IPv6 address is not mistaken for host:port). +const HOST_PORT_PATTERN = /^(?:\[([^\]]+)\]|([^:/\s]+)):(\d{1,5})$/; + +export interface IEgressService { + probeTargets(targets: string[]): Promise; + assessAfterFailure(): Promise; + startRecoveryPolling(): void; + stop(): void; + init(): Promise; +} + +export class EgressService implements IEgressService { + static SERVICE_NAME = SERVICE_NAME; + + private polling = false; + private pollTimer: NodeJS.Timeout | null = null; + + constructor( + private settingsService: ISettingsService, + private egressStateRepository: IEgressStateRepository, + private networkService: INetworkService, + private logger: ILogger + ) {} + + // **************************** + // Probing + // **************************** + + // Reuses the existing ping/port/http providers by dressing each target up as a minimal monitor. + private toProbeMonitor = (target: string): Monitor => { + const base = { + id: PROBE_MONITOR_ID, + teamId: PROBE_TEAM_ID, + name: `Egress probe: ${target}`, + method: "GET", + useAdvancedMatching: false, + ignoreTlsErrors: false, + customUpCodes: [], + }; + + if (/^https?:\/\//i.test(target)) { + return { ...base, type: "http", url: target } as unknown as Monitor; + } + + const hostPort = HOST_PORT_PATTERN.exec(target); + if (hostPort) { + const host = hostPort[1] ?? hostPort[2]; + return { ...base, type: "port", url: host, port: Number(hostPort[3]) } as unknown as Monitor; + } + + return { ...base, type: "ping", url: target } as unknown as Monitor; + }; + + private probeTarget = async (target: string): Promise => { + const monitor = this.toProbeMonitor(target); + let timer: NodeJS.Timeout | undefined; + + const { response, responseTime, error } = await timeRequest(() => + Promise.race([ + this.networkService.requestStatus(monitor), + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`Egress probe timed out after ${PROBE_TIMEOUT_MS}ms`)), PROBE_TIMEOUT_MS); + }), + ]).finally(() => clearTimeout(timer)) + ); + + if (error || !response) { + return { + target, + reachable: false, + responseTime, + message: error instanceof Error ? error.message : error ? String(error) : "No response", + }; + } + + return { + target, + reachable: response.status === true, + responseTime: response.responseTime ?? responseTime, + message: response.message, + }; + }; + + probeTargets = async (targets: string[]): Promise => { + return await Promise.all(targets.map((target) => this.probeTarget(target))); + }; + + private resolveTargets = (configured: string[] | undefined): string[] => { + const targets = (configured ?? []).map((target) => target.trim()).filter((target) => target.length > 0); + // An empty list would make "all unreachable" vacuously true, so fall back to the defaults. + return targets.length > 0 ? targets : [...DEFAULT_EGRESS_TARGETS]; + }; + + // **************************** + // Event-triggered assessment + // **************************** + + assessAfterFailure = async (): Promise => { + try { + const settings = await this.settingsService.getDBSettings(); + if (!settings.egressCheckEnabled) { + return null; + } + + const state = await this.egressStateRepository.findSingleton(); + if (state.status === "degraded") { + // Recovery detection belongs to the poll loop. Make sure one is running in this process in case + // the worker that entered the degraded state is no longer around. + this.startRecoveryPolling(); + return "degraded"; + } + + const results = await this.probeTargets(this.resolveTargets(settings.egressCheckTargets)); + const now = new Date(); + + if (results.some((result) => result.reachable)) { + await this.egressStateRepository.recordProbe(results, now); + return "ok"; + } + + const degraded = await this.egressStateRepository.markDegraded(results, now); + this.logger.warn({ + message: degraded + ? "Instance egress degraded: every reliability target is unreachable. Monitor failures will not be counted until it recovers" + : "Instance egress degraded (transition already recorded by another worker)", + service: SERVICE_NAME, + method: "assessAfterFailure", + details: { results }, + }); + this.startRecoveryPolling(); + return "degraded"; + } catch (error: unknown) { + // The egress check must never break check production; treat an internal failure as "unknown" and carry on. + this.logger.error({ + message: error instanceof Error ? error.message : String(error), + service: SERVICE_NAME, + method: "assessAfterFailure", + stack: error instanceof Error ? error.stack : undefined, + }); + return null; + } + }; + + // **************************** + // Recovery poll loop + // **************************** + + startRecoveryPolling = () => { + if (this.polling) { + return; + } + this.polling = true; + this.logger.info({ message: "Starting egress recovery polling", service: SERVICE_NAME, method: "startRecoveryPolling" }); + void this.scheduleNextTick(); + }; + + stop = () => { + this.polling = false; + if (this.pollTimer) { + clearTimeout(this.pollTimer); + this.pollTimer = null; + } + }; + + private readPollIntervalMs = async () => { + try { + const settings = await this.settingsService.getDBSettings(); + const seconds = settings.egressPollIntervalSeconds; + return (Number.isFinite(seconds) && seconds > 0 ? seconds : DEFAULT_EGRESS_POLL_INTERVAL_SECONDS) * 1000; + } catch (error: unknown) { + this.logger.warn({ + message: `Could not read egress poll interval, using default: ${error instanceof Error ? error.message : String(error)}`, + service: SERVICE_NAME, + method: "readPollIntervalMs", + }); + return DEFAULT_EGRESS_POLL_INTERVAL_SECONDS * 1000; + } + }; + + private scheduleNextTick = async () => { + if (!this.polling) { + return; + } + const intervalMs = await this.readPollIntervalMs(); + if (!this.polling) { + return; + } + this.pollTimer = setTimeout(() => void this.tick(), intervalMs); + // Never hold the process open for this loop; the state is persisted and init() resumes it on restart. + this.pollTimer.unref(); + }; + + private tick = async () => { + this.pollTimer = null; + if (!this.polling) { + return; + } + + try { + const state = await this.egressStateRepository.findSingleton(); + if (state.status !== "degraded") { + this.logger.info({ message: "Egress no longer degraded, stopping recovery polling", service: SERVICE_NAME, method: "tick" }); + this.stop(); + return; + } + + const settings = await this.settingsService.getDBSettings(); + const results = await this.probeTargets(this.resolveTargets(settings.egressCheckTargets)); + const now = new Date(); + + if (!results.some((result) => result.reachable)) { + await this.egressStateRepository.recordProbe(results, now); + return; + } + + const recovered = await this.egressStateRepository.markRecovered(results, now); + this.stop(); + if (!recovered) { + // Another process performed the transition. + return; + } + + this.logger.info({ + message: `Instance egress recovered (degraded since ${recovered.degradedSince ?? "unknown"})`, + service: SERVICE_NAME, + method: "tick", + details: { results }, + }); + } catch (error: unknown) { + this.logger.error({ + message: error instanceof Error ? error.message : String(error), + service: SERVICE_NAME, + method: "tick", + stack: error instanceof Error ? error.stack : undefined, + }); + } finally { + // No-op once stop() has been called; otherwise keep the loop alive through errors. + await this.scheduleNextTick(); + } + }; + + // **************************** + // Lifecycle + // **************************** + + init = async () => { + try { + const settings = await this.settingsService.getDBSettings(); + if (!settings.egressCheckEnabled) { + return; + } + const state = await this.egressStateRepository.findSingleton(); + if (state.status === "degraded") { + this.logger.warn({ + message: `Instance egress was degraded at startup (since ${state.degradedSince ?? "unknown"}), resuming recovery polling`, + service: SERVICE_NAME, + method: "init", + }); + this.startRecoveryPolling(); + } + } catch (error: unknown) { + this.logger.error({ + message: error instanceof Error ? error.message : String(error), + service: SERVICE_NAME, + method: "init", + stack: error instanceof Error ? error.stack : undefined, + }); + } + }; +} diff --git a/server/src/domain/egress/egress.type.ts b/server/src/domain/egress/egress.type.ts new file mode 100644 index 000000000..386116ef7 --- /dev/null +++ b/server/src/domain/egress/egress.type.ts @@ -0,0 +1,33 @@ +// Egress self-check: detects loss of the instance's own outbound connectivity so that +// a burst of simultaneous monitor failures is not reported as a burst of real outages. + +export const EgressStatuses = ["ok", "degraded"] as const; +export type EgressStatus = (typeof EgressStatuses)[number]; + +// Sensible defaults for a fresh install. Bare hosts are ICMP-pinged, host:port is a TCP connect, +// http(s):// URLs are fetched. Any one target being reachable means egress is fine. +export const DEFAULT_EGRESS_TARGETS = ["1.1.1.1", "8.8.8.8"] as const; +export const DEFAULT_EGRESS_POLL_INTERVAL_SECONDS = 30; +export const MIN_EGRESS_POLL_INTERVAL_SECONDS = 5; +export const MAX_EGRESS_POLL_INTERVAL_SECONDS = 600; +export const MAX_EGRESS_TARGETS = 10; + +// Instance-global singleton persisted in MongoDB so that every worker process and the API +// process share one view of the state (the queue worker may run separately from the API). +export interface EgressState { + id: string; + status: EgressStatus; + degradedSince: string | null; // ISO timestamp of the current or most recent degraded entry + lastRecoveredAt: string | null; // ISO timestamp of the most recent recovery + lastProbeAt: string | null; // ISO timestamp of the most recent reliability probe + lastProbeResults: EgressProbeResult[]; + createdAt: string; + updatedAt: string; +} + +export interface EgressProbeResult { + target: string; + reachable: boolean; + responseTime: number; // ms + message?: string; +} diff --git a/server/test/unit/services/egressService.test.ts b/server/test/unit/services/egressService.test.ts new file mode 100644 index 000000000..8c4fbf183 --- /dev/null +++ b/server/test/unit/services/egressService.test.ts @@ -0,0 +1,404 @@ +import { afterEach, describe, expect, it, jest } from "@jest/globals"; +import { EgressService } from "../../../src/domain/egress/egress.service.ts"; +import type { EgressState } from "../../../src/domain/egress/egress.type.ts"; +import { createMockLogger } from "../../helpers/createMockLogger.ts"; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +const makeState = (overrides?: Partial): EgressState => ({ + id: "egress-1", + status: "ok", + degradedSince: null, + lastRecoveredAt: null, + lastProbeAt: null, + lastProbeResults: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + ...overrides, +}); + +const makeSettings = (overrides?: Record) => ({ + egressCheckEnabled: true, + egressCheckTargets: ["1.1.1.1", "8.8.8.8"], + egressPollIntervalSeconds: 30, + egressNotifications: ["notif-1"], + ...overrides, +}); + +// requestStatus mock that answers per target url +const statusFor = (reachableTargets: string[]) => + jest.fn().mockImplementation(async (monitor: any) => ({ + monitorId: monitor.id, + teamId: monitor.teamId, + type: monitor.type, + status: reachableTargets.includes(monitor.url), + code: reachableTargets.includes(monitor.url) ? 200 : 5000, + message: reachableTargets.includes(monitor.url) ? "Success" : "Ping failed", + responseTime: 12, + })); + +const createService = (overrides?: Record) => { + const defaults = { + settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings()) }, + egressStateRepository: { + findSingleton: jest.fn().mockResolvedValue(makeState()), + recordProbe: jest.fn().mockResolvedValue(makeState()), + markDegraded: jest.fn().mockResolvedValue(makeState({ status: "degraded", degradedSince: "2026-01-01T10:00:00.000Z" })), + markRecovered: jest.fn().mockResolvedValue(makeState({ lastRecoveredAt: "2026-01-01T10:05:00.000Z" })), + }, + networkService: { requestStatus: statusFor(["1.1.1.1", "8.8.8.8"]) }, + logger: createMockLogger(), + ...overrides, + }; + const service = new EgressService( + defaults.settingsService as any, + defaults.egressStateRepository as any, + defaults.networkService as any, + defaults.logger as any + ); + return { service, defaults }; +}; + +// Lets the setTimeout chain in startRecoveryPolling settle (settings read, then timer armed) before advancing time. +const runOneTick = async (intervalMs: number) => { + await jest.advanceTimersByTimeAsync(0); + await jest.advanceTimersByTimeAsync(intervalMs); +}; + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe("EgressService", () => { + afterEach(() => { + jest.useRealTimers(); + }); + + // ── probeTargets ────────────────────────────────────────────────────────── + + describe("probeTargets", () => { + it("maps a bare host to a ping monitor, host:port to a port monitor and a URL to an http monitor", async () => { + const { service, defaults } = createService(); + + await service.probeTargets(["1.1.1.1", "8.8.8.8:53", "https://example.com/health", "[2606:4700::1111]:443"]); + + const monitors = (defaults.networkService.requestStatus as jest.Mock).mock.calls.map((call) => call[0] as any); + expect(monitors).toEqual([ + expect.objectContaining({ type: "ping", url: "1.1.1.1" }), + expect.objectContaining({ type: "port", url: "8.8.8.8", port: 53 }), + expect.objectContaining({ type: "http", url: "https://example.com/health", method: "GET", useAdvancedMatching: false }), + expect.objectContaining({ type: "port", url: "2606:4700::1111", port: 443 }), + ]); + }); + + it("reports reachable only when the provider says status is true", async () => { + const { service } = createService({ networkService: { requestStatus: statusFor(["1.1.1.1"]) } }); + + const results = await service.probeTargets(["1.1.1.1", "8.8.8.8"]); + + expect(results).toEqual([ + expect.objectContaining({ target: "1.1.1.1", reachable: true, responseTime: 12 }), + expect.objectContaining({ target: "8.8.8.8", reachable: false }), + ]); + }); + + it("counts a thrown provider error as unreachable", async () => { + const { service } = createService({ networkService: { requestStatus: jest.fn().mockRejectedValue(new Error("ENETUNREACH")) } }); + + const results = await service.probeTargets(["1.1.1.1"]); + + expect(results).toEqual([expect.objectContaining({ target: "1.1.1.1", reachable: false, message: "ENETUNREACH" })]); + }); + + it("counts a hung provider as unreachable once the probe timeout elapses", async () => { + jest.useFakeTimers(); + const { service } = createService({ networkService: { requestStatus: jest.fn().mockReturnValue(new Promise(() => {})) } }); + + const pending = service.probeTargets(["1.1.1.1"]); + await jest.advanceTimersByTimeAsync(5000); + const results = await pending; + + expect(results).toEqual([expect.objectContaining({ target: "1.1.1.1", reachable: false, message: expect.stringContaining("timed out") })]); + }); + }); + + // ── assessAfterFailure ──────────────────────────────────────────────────── + + describe("assessAfterFailure", () => { + it("returns null and does not probe or record when the feature is disabled", async () => { + const { service, defaults } = createService({ + settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false })) }, + }); + + const result = await service.assessAfterFailure(); + + expect(result).toBeNull(); + expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); + expect(defaults.egressStateRepository.findSingleton).not.toHaveBeenCalled(); + expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); + }); + + it("returns ok and records the probe when any target is reachable", async () => { + const { service, defaults } = createService({ networkService: { requestStatus: statusFor(["8.8.8.8"]) } }); + + const result = await service.assessAfterFailure(); + + expect(result).toBe("ok"); + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledWith( + [expect.objectContaining({ target: "1.1.1.1", reachable: false }), expect.objectContaining({ target: "8.8.8.8", reachable: true })], + expect.any(Date) + ); + expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled(); + }); + + it("marks degraded, warns and returns degraded when every target is unreachable", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ networkService: { requestStatus: statusFor([]) } }); + + const result = await service.assessAfterFailure(); + + expect(result).toBe("degraded"); + expect(defaults.egressStateRepository.markDegraded).toHaveBeenCalledWith( + [expect.objectContaining({ target: "1.1.1.1", reachable: false }), expect.objectContaining({ target: "8.8.8.8", reachable: false })], + expect.any(Date) + ); + expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); + expect(defaults.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("egress degraded") })); + expect(defaults.logger.info).toHaveBeenCalledWith(expect.objectContaining({ message: "Starting egress recovery polling" })); + service.stop(); + }); + + it("returns degraded without probing when the persisted state is already degraded", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ + egressStateRepository: { + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + recordProbe: jest.fn(), + markDegraded: jest.fn(), + markRecovered: jest.fn(), + }, + }); + + const result = await service.assessAfterFailure(); + + expect(result).toBe("degraded"); + expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); + expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled(); + expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); + service.stop(); + }); + + it("falls back to the default targets when the configured list is empty", async () => { + const { service, defaults } = createService({ + settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckTargets: [] })) }, + }); + + await service.assessAfterFailure(); + + const urls = (defaults.networkService.requestStatus as jest.Mock).mock.calls.map((call) => (call[0] as any).url); + expect(urls).toEqual(["1.1.1.1", "8.8.8.8"]); + }); + + it("returns null and logs when the assessment itself fails", async () => { + const { service, defaults } = createService({ + settingsService: { getDBSettings: jest.fn().mockRejectedValue(new Error("db down")) }, + }); + + const result = await service.assessAfterFailure(); + + expect(result).toBeNull(); + expect(defaults.logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: "db down", method: "assessAfterFailure" })); + }); + }); + + // ── recovery polling ────────────────────────────────────────────────────── + + describe("recovery polling", () => { + it("marks recovered once and stops polling when a target becomes reachable", async () => { + jest.useFakeTimers(); + const recovered = makeState({ lastRecoveredAt: "2026-01-01T10:05:00.000Z", degradedSince: "2026-01-01T10:00:00.000Z" }); + const { service, defaults } = createService({ + egressStateRepository: { + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + recordProbe: jest.fn(), + markDegraded: jest.fn(), + markRecovered: jest.fn().mockResolvedValue(recovered), + }, + networkService: { requestStatus: statusFor(["1.1.1.1"]) }, + }); + + service.startRecoveryPolling(); + await runOneTick(30_000); + + expect(defaults.egressStateRepository.markRecovered).toHaveBeenCalledTimes(1); + expect(defaults.logger.info).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("Instance egress recovered") })); + + // Loop has stopped: a further interval produces no more work + await runOneTick(30_000); + expect(defaults.egressStateRepository.markRecovered).toHaveBeenCalledTimes(1); + }); + + it("does not log a recovery when another process already recorded it", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ + egressStateRepository: { + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + recordProbe: jest.fn(), + markDegraded: jest.fn(), + markRecovered: jest.fn().mockResolvedValue(null), + }, + networkService: { requestStatus: statusFor(["1.1.1.1"]) }, + }); + + service.startRecoveryPolling(); + await runOneTick(30_000); + + expect(defaults.egressStateRepository.markRecovered).toHaveBeenCalledTimes(1); + expect(defaults.logger.info).not.toHaveBeenCalledWith( + expect.objectContaining({ message: expect.stringContaining("Instance egress recovered") }) + ); + }); + + it("records the probe and keeps polling while every target stays unreachable", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ + egressStateRepository: { + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + recordProbe: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + markDegraded: jest.fn(), + markRecovered: jest.fn(), + }, + networkService: { requestStatus: statusFor([]) }, + }); + + service.startRecoveryPolling(); + await runOneTick(30_000); + await runOneTick(30_000); + + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(2); + expect(defaults.egressStateRepository.markRecovered).not.toHaveBeenCalled(); + service.stop(); + }); + + it("stops polling when the persisted state is no longer degraded", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ + egressStateRepository: { + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "ok" })), + recordProbe: jest.fn(), + markDegraded: jest.fn(), + markRecovered: jest.fn(), + }, + }); + + service.startRecoveryPolling(); + await runOneTick(30_000); + + expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); + await runOneTick(30_000); + expect(defaults.egressStateRepository.findSingleton).toHaveBeenCalledTimes(1); + }); + + it("survives a failing tick and reschedules", async () => { + jest.useFakeTimers(); + const findSingleton = jest + .fn() + .mockRejectedValueOnce(new Error("db blip")) + .mockResolvedValue(makeState({ status: "degraded" })); + const { service, defaults } = createService({ + egressStateRepository: { + findSingleton, + recordProbe: jest.fn().mockResolvedValue(makeState()), + markDegraded: jest.fn(), + markRecovered: jest.fn(), + }, + networkService: { requestStatus: statusFor([]) }, + }); + + service.startRecoveryPolling(); + await runOneTick(30_000); + expect(defaults.logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: "db blip", method: "tick" })); + + await runOneTick(30_000); + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); + service.stop(); + }); + + it("re-reads the poll interval from settings on each tick", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ + settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressPollIntervalSeconds: 5 })) }, + egressStateRepository: { + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + recordProbe: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + markDegraded: jest.fn(), + markRecovered: jest.fn(), + }, + networkService: { requestStatus: statusFor([]) }, + }); + + service.startRecoveryPolling(); + await runOneTick(5_000); + + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); + service.stop(); + }); + + it("is idempotent: a second start does not arm a second loop", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ + egressStateRepository: { + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + recordProbe: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + markDegraded: jest.fn(), + markRecovered: jest.fn(), + }, + networkService: { requestStatus: statusFor([]) }, + }); + + service.startRecoveryPolling(); + service.startRecoveryPolling(); + await runOneTick(30_000); + + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); + service.stop(); + }); + }); + + // ── init ────────────────────────────────────────────────────────────────── + + describe("init", () => { + it("resumes polling when the feature is enabled and the persisted state is degraded", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ + egressStateRepository: { + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + recordProbe: jest.fn(), + markDegraded: jest.fn(), + markRecovered: jest.fn().mockResolvedValue(makeState()), + }, + }); + + await service.init(); + + expect(defaults.logger.info).toHaveBeenCalledWith(expect.objectContaining({ message: "Starting egress recovery polling" })); + service.stop(); + }); + + it("does nothing when the persisted state is ok", async () => { + const { service, defaults } = createService(); + + await service.init(); + + expect(defaults.logger.info).not.toHaveBeenCalled(); + }); + + it("does nothing when the feature is disabled", async () => { + const { service, defaults } = createService({ + settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false })) }, + }); + + await service.init(); + + expect(defaults.egressStateRepository.findSingleton).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/server/test/unit/services/egressStateService.test.ts b/server/test/unit/services/egressStateService.test.ts new file mode 100644 index 000000000..85c32f32d --- /dev/null +++ b/server/test/unit/services/egressStateService.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it, jest } from "@jest/globals"; +import { EgressStateService } from "../../../src/domain/egress/egress-state.service.ts"; +import type { EgressState } from "../../../src/domain/egress/egress.type.ts"; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +const createEgressStateRepo = () => ({ + findSingleton: jest.fn(), + recordProbe: jest.fn(), + markDegraded: jest.fn(), + markRecovered: jest.fn(), +}); + +const createService = () => { + const egressStateRepository = createEgressStateRepo(); + const service = new EgressStateService(egressStateRepository as any); + return { service, egressStateRepository }; +}; + +const makeState = (overrides?: Partial): EgressState => ({ + id: "egress-1", + status: "ok", + degradedSince: null, + lastRecoveredAt: null, + lastProbeAt: null, + lastProbeResults: [], + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + ...overrides, +}); + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe("EgressStateService", () => { + describe("getState", () => { + it("returns the singleton state from the repository", async () => { + const { service, egressStateRepository } = createService(); + const state = makeState({ + status: "degraded", + degradedSince: "2026-01-01T00:05:00Z", + lastProbeAt: "2026-01-01T00:05:00Z", + lastProbeResults: [{ target: "1.1.1.1", reachable: false, responseTime: 0, message: "timeout" }], + }); + (egressStateRepository.findSingleton as jest.Mock).mockResolvedValue(state); + + const result = await service.getState(); + + expect(egressStateRepository.findSingleton).toHaveBeenCalledTimes(1); + expect(result).toEqual(state); + }); + + it("does not call any transition method", async () => { + const { service, egressStateRepository } = createService(); + (egressStateRepository.findSingleton as jest.Mock).mockResolvedValue(makeState()); + + await service.getState(); + + expect(egressStateRepository.recordProbe).not.toHaveBeenCalled(); + expect(egressStateRepository.markDegraded).not.toHaveBeenCalled(); + expect(egressStateRepository.markRecovered).not.toHaveBeenCalled(); + }); + + it("propagates repository errors", async () => { + const { service, egressStateRepository } = createService(); + (egressStateRepository.findSingleton as jest.Mock).mockRejectedValue(new Error("db down")); + + await expect(service.getState()).rejects.toThrow("db down"); + }); + }); +}); diff --git a/server/test/unit/validation/settingsValidation.test.ts b/server/test/unit/validation/settingsValidation.test.ts new file mode 100644 index 000000000..36e25c3b2 --- /dev/null +++ b/server/test/unit/validation/settingsValidation.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "@jest/globals"; +import { updateAppSettingsBodyValidation } from "../../../src/api/validation/settingsValidation.ts"; +import { MAX_EGRESS_POLL_INTERVAL_SECONDS, MAX_EGRESS_TARGETS, MIN_EGRESS_POLL_INTERVAL_SECONDS } from "../../../src/domain/egress/egress.type.ts"; + +const validNotificationId = "64b7f0c2a1d2e3f4a5b6c7d8"; + +describe("settingsValidation", () => { + describe("updateAppSettingsBodyValidation egress fields", () => { + it("accepts a full set of valid egress settings", () => { + const parsed = updateAppSettingsBodyValidation.parse({ + egressCheckEnabled: true, + egressCheckTargets: ["1.1.1.1", "dns.google:53", "https://example.com/health"], + egressPollIntervalSeconds: 30, + egressNotifications: [validNotificationId], + }); + + expect(parsed.egressCheckEnabled).toBe(true); + expect(parsed.egressCheckTargets).toEqual(["1.1.1.1", "dns.google:53", "https://example.com/health"]); + expect(parsed.egressPollIntervalSeconds).toBe(30); + expect(parsed.egressNotifications).toEqual([validNotificationId]); + }); + + it("accepts a body with no egress fields", () => { + expect(() => updateAppSettingsBodyValidation.parse({ checkTTL: 30 })).not.toThrow(); + }); + + it("accepts an empty target list (the worker falls back to the default targets)", () => { + expect(updateAppSettingsBodyValidation.parse({ egressCheckTargets: [] }).egressCheckTargets).toEqual([]); + }); + + it("rejects more targets than the maximum", () => { + const targets = Array.from({ length: MAX_EGRESS_TARGETS + 1 }, (_, i) => `10.0.0.${i}`); + + expect(() => updateAppSettingsBodyValidation.parse({ egressCheckTargets: targets })).toThrow(); + }); + + it("rejects targets that are not a host, host:port or http(s) URL", () => { + for (const target of ["", " ", "ftp://example.com", "bad host", "1.1.1.1;rm"]) { + expect(() => updateAppSettingsBodyValidation.parse({ egressCheckTargets: [target] })).toThrow(); + } + }); + + it("accepts the poll interval boundaries", () => { + expect(updateAppSettingsBodyValidation.parse({ egressPollIntervalSeconds: MIN_EGRESS_POLL_INTERVAL_SECONDS }).egressPollIntervalSeconds).toBe( + MIN_EGRESS_POLL_INTERVAL_SECONDS + ); + expect(updateAppSettingsBodyValidation.parse({ egressPollIntervalSeconds: MAX_EGRESS_POLL_INTERVAL_SECONDS }).egressPollIntervalSeconds).toBe( + MAX_EGRESS_POLL_INTERVAL_SECONDS + ); + }); + + it("rejects out-of-range and non-integer poll intervals", () => { + for (const interval of [MIN_EGRESS_POLL_INTERVAL_SECONDS - 1, MAX_EGRESS_POLL_INTERVAL_SECONDS + 1, 0, -5, 30.5, "30"]) { + expect(() => updateAppSettingsBodyValidation.parse({ egressPollIntervalSeconds: interval })).toThrow(); + } + }); + + it("accepts an empty notification list", () => { + expect(updateAppSettingsBodyValidation.parse({ egressNotifications: [] }).egressNotifications).toEqual([]); + }); + + it("rejects notification ids that are not 24-character hex", () => { + for (const id of ["not-an-id", "64b7f0c2a1d2e3f4a5b6c7d", "zzzzzzzzzzzzzzzzzzzzzzzz", 123]) { + expect(() => updateAppSettingsBodyValidation.parse({ egressNotifications: [id] })).toThrow(); + } + }); + + it("rejects a non-boolean enabled flag", () => { + expect(() => updateAppSettingsBodyValidation.parse({ egressCheckEnabled: "yes" })).toThrow(); + }); + }); +}); From 46eb0b4063b26d20176cd2250b33457dff68eccf Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Mon, 14 Sep 2026 20:41:58 +0100 Subject: [PATCH 2/4] Replace the egress recovery timer with a scheduled job (#3937) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed check still calls assessAfterFailure, which now shares one in-flight assessment between concurrent failures (same cache shape as ProxyResolver). When egress degrades it upserts a global "egress" job; DBQueueWorker runs checkRecovery for that type at the configured interval, and the job deletes its own row once any target answers. The unref'd timer, startRecoveryPolling, stop and init are gone: a degraded episode survives a restart through the jobs collection. deleteByIdAndType accepts a null refId for global rows. 🔧 --- server/src/config/services.worker.ts | 6 +- server/src/domain/egress/egress.service.ts | 197 +++++-------- .../domain/jobs/job.repository.interface.ts | 4 +- .../src/domain/jobs/job.repository.mongo.ts | 4 +- server/src/domain/jobs/job.type.ts | 4 +- server/src/worker/worker.db-queue.ts | 9 + server/src/worker/worker.job-scheduler.ts | 1 + .../test/unit/services/dbQueueWorker.test.ts | 14 +- .../test/unit/services/egressService.test.ts | 273 +++++++----------- 9 files changed, 203 insertions(+), 309 deletions(-) diff --git a/server/src/config/services.worker.ts b/server/src/config/services.worker.ts index 83a875f05..b97bb56f3 100644 --- a/server/src/config/services.worker.ts +++ b/server/src/config/services.worker.ts @@ -106,7 +106,7 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig const bufferService = new BufferService(logger, checkService, geoChecksService, dockerLogsService, settingsService, jobsRepository); const statusService = new StatusService(logger, monitorsRepository, monitorStatsRepository); const monitorStatusPolicy = new MonitorStatusPolicy(); - const egressService = new EgressService(settingsService, egressStateRepository, networkService, logger); + const egressService = new EgressService(settingsService, egressStateRepository, jobsRepository, networkService, logger); // *********************** // Reactors and dispatcher @@ -165,6 +165,7 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig geoCheckPipeline, dispatcher: reactorDispatcher, helper: workerHelper, + egressService, queueWorkersRepository, queueMode: envSettings.queueMode, queuePrimaryProcesses: envSettings.queuePrimaryProcesses, @@ -189,8 +190,5 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig }); } - // Resume recovery polling if the instance was mid-episode when it last stopped - await egressService.init(); - return { worker, networkService, bufferService, statusService, egressService }; }; diff --git a/server/src/domain/egress/egress.service.ts b/server/src/domain/egress/egress.service.ts index cb3b79ebb..9ca98f4b2 100644 --- a/server/src/domain/egress/egress.service.ts +++ b/server/src/domain/egress/egress.service.ts @@ -1,6 +1,8 @@ import type { Monitor } from "@/domain/monitors/monitor.type.js"; import type { ISettingsService } from "@/domain/app-settings/app-settings.service.js"; import type { IEgressStateRepository } from "@/domain/egress/egress-state.repository.interface.js"; +import type { IJobsRepository } from "@/domain/jobs/job.repository.interface.js"; +import { jobId, type JobSeed } from "@/domain/jobs/job.type.js"; import type { INetworkService } from "@/service/networkService.js"; import type { ILogger } from "@/utils/logger.js"; import { @@ -13,6 +15,11 @@ import { timeRequest } from "@/service/network/utils.js"; const SERVICE_NAME = "EgressService"; const PROBE_TIMEOUT_MS = 5000; +// When egress is down every monitor fails at once, so concurrent failures share one assessment +// rather than each probing. Same in-flight cache shape as ProxyResolver. +const ASSESSMENT_TTL_MS = PROBE_TIMEOUT_MS; +// Global job row (refId null) that re-probes while degraded. Inserted when egress degrades, removed on recovery. +const RECOVERY_JOB_ID = jobId("egress", null); // Synthetic identity stamped on the probe monitors so provider responses are recognisable in logs. const PROBE_MONITOR_ID = "egress-probe"; @@ -23,21 +30,21 @@ const HOST_PORT_PATTERN = /^(?:\[([^\]]+)\]|([^:/\s]+)):(\d{1,5})$/; export interface IEgressService { probeTargets(targets: string[]): Promise; + // Called by the check producer when a check fails. Null when the feature is off or the assessment itself failed. assessAfterFailure(): Promise; - startRecoveryPolling(): void; - stop(): void; - init(): Promise; + // Handler for the "egress" job: re-probes while degraded and removes the job once egress is back. + checkRecovery(): Promise; } export class EgressService implements IEgressService { static SERVICE_NAME = SERVICE_NAME; - private polling = false; - private pollTimer: NodeJS.Timeout | null = null; + private assessment: { value: Promise; expiresAt: number } | null = null; constructor( private settingsService: ISettingsService, private egressStateRepository: IEgressStateRepository, + private jobsRepository: IJobsRepository, private networkService: INetworkService, private logger: ILogger ) {} @@ -111,11 +118,31 @@ export class EgressService implements IEgressService { return targets.length > 0 ? targets : [...DEFAULT_EGRESS_TARGETS]; }; + private toIntervalMs = (seconds: number | undefined): number => { + const valid = typeof seconds === "number" && Number.isFinite(seconds) && seconds > 0; + return (valid ? seconds : DEFAULT_EGRESS_POLL_INTERVAL_SECONDS) * 1000; + }; + // **************************** // Event-triggered assessment // **************************** - assessAfterFailure = async (): Promise => { + assessAfterFailure = (): Promise => { + const now = Date.now(); + if (this.assessment && this.assessment.expiresAt > now) { + return this.assessment.value; + } + // Cache the in-flight promise so concurrent failures share one probe + const value = this.assess(); + this.assessment = { value, expiresAt: now + ASSESSMENT_TTL_MS }; + // Don't cache an internal failure, so the next failing check retries + void value.then((status) => { + if (status === null && this.assessment?.value === value) this.assessment = null; + }); + return value; + }; + + private assess = async (): Promise => { try { const settings = await this.settingsService.getDBSettings(); if (!settings.egressCheckEnabled) { @@ -124,9 +151,8 @@ export class EgressService implements IEgressService { const state = await this.egressStateRepository.findSingleton(); if (state.status === "degraded") { - // Recovery detection belongs to the poll loop. Make sure one is running in this process in case - // the worker that entered the degraded state is no longer around. - this.startRecoveryPolling(); + // Recovery is detected by the scheduled job. Make sure it exists in case its row was lost. + await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds); return "degraded"; } @@ -147,7 +173,7 @@ export class EgressService implements IEgressService { method: "assessAfterFailure", details: { results }, }); - this.startRecoveryPolling(); + await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds); return "degraded"; } catch (error: unknown) { // The egress check must never break check production; treat an internal failure as "unknown" and carry on. @@ -162,129 +188,58 @@ export class EgressService implements IEgressService { }; // **************************** - // Recovery poll loop + // Recovery job // **************************** - startRecoveryPolling = () => { - if (this.polling) { - return; - } - this.polling = true; - this.logger.info({ message: "Starting egress recovery polling", service: SERVICE_NAME, method: "startRecoveryPolling" }); - void this.scheduleNextTick(); + // upsertJob only sets nextScheduledAt on insert, so re-arming while degraded never pushes a pending run back. + private scheduleRecoveryCheck = async (pollIntervalSeconds: number | undefined) => { + const intervalMs = this.toIntervalMs(pollIntervalSeconds); + const seed: JobSeed = { + id: RECOVERY_JOB_ID, + type: "egress", + refId: null, + isActive: true, + nextScheduledAt: Date.now() + intervalMs, + intervalMs, + }; + await this.jobsRepository.upsertJob(seed); }; - stop = () => { - this.polling = false; - if (this.pollTimer) { - clearTimeout(this.pollTimer); - this.pollTimer = null; - } + private removeRecoveryJob = async () => { + await this.jobsRepository.deleteByIdAndType(null, "egress"); }; - private readPollIntervalMs = async () => { - try { - const settings = await this.settingsService.getDBSettings(); - const seconds = settings.egressPollIntervalSeconds; - return (Number.isFinite(seconds) && seconds > 0 ? seconds : DEFAULT_EGRESS_POLL_INTERVAL_SECONDS) * 1000; - } catch (error: unknown) { - this.logger.warn({ - message: `Could not read egress poll interval, using default: ${error instanceof Error ? error.message : String(error)}`, - service: SERVICE_NAME, - method: "readPollIntervalMs", - }); - return DEFAULT_EGRESS_POLL_INTERVAL_SECONDS * 1000; - } - }; - - private scheduleNextTick = async () => { - if (!this.polling) { - return; - } - const intervalMs = await this.readPollIntervalMs(); - if (!this.polling) { - return; - } - this.pollTimer = setTimeout(() => void this.tick(), intervalMs); - // Never hold the process open for this loop; the state is persisted and init() resumes it on restart. - this.pollTimer.unref(); - }; - - private tick = async () => { - this.pollTimer = null; - if (!this.polling) { + // Runs on the queue at the configured interval while degraded. Errors propagate so the queue records + // the failure and retries with its usual backoff. + checkRecovery = async (): Promise => { + const state = await this.egressStateRepository.findSingleton(); + if (state.status !== "degraded") { + // Another worker recorded the recovery, or the state was reset; nothing left to poll for. + await this.removeRecoveryJob(); return; } - try { - const state = await this.egressStateRepository.findSingleton(); - if (state.status !== "degraded") { - this.logger.info({ message: "Egress no longer degraded, stopping recovery polling", service: SERVICE_NAME, method: "tick" }); - this.stop(); - return; - } + const settings = await this.settingsService.getDBSettings(); + const results = await this.probeTargets(this.resolveTargets(settings.egressCheckTargets)); + const now = new Date(); - const settings = await this.settingsService.getDBSettings(); - const results = await this.probeTargets(this.resolveTargets(settings.egressCheckTargets)); - const now = new Date(); - - if (!results.some((result) => result.reachable)) { - await this.egressStateRepository.recordProbe(results, now); - return; - } - - const recovered = await this.egressStateRepository.markRecovered(results, now); - this.stop(); - if (!recovered) { - // Another process performed the transition. - return; - } - - this.logger.info({ - message: `Instance egress recovered (degraded since ${recovered.degradedSince ?? "unknown"})`, - service: SERVICE_NAME, - method: "tick", - details: { results }, - }); - } catch (error: unknown) { - this.logger.error({ - message: error instanceof Error ? error.message : String(error), - service: SERVICE_NAME, - method: "tick", - stack: error instanceof Error ? error.stack : undefined, - }); - } finally { - // No-op once stop() has been called; otherwise keep the loop alive through errors. - await this.scheduleNextTick(); + if (!results.some((result) => result.reachable)) { + await this.egressStateRepository.recordProbe(results, now); + return; } - }; - // **************************** - // Lifecycle - // **************************** - - init = async () => { - try { - const settings = await this.settingsService.getDBSettings(); - if (!settings.egressCheckEnabled) { - return; - } - const state = await this.egressStateRepository.findSingleton(); - if (state.status === "degraded") { - this.logger.warn({ - message: `Instance egress was degraded at startup (since ${state.degradedSince ?? "unknown"}), resuming recovery polling`, - service: SERVICE_NAME, - method: "init", - }); - this.startRecoveryPolling(); - } - } catch (error: unknown) { - this.logger.error({ - message: error instanceof Error ? error.message : String(error), - service: SERVICE_NAME, - method: "init", - stack: error instanceof Error ? error.stack : undefined, - }); + const recovered = await this.egressStateRepository.markRecovered(results, now); + await this.removeRecoveryJob(); + if (!recovered) { + // Another process performed the transition. + return; } + + this.logger.info({ + message: `Instance egress recovered (degraded since ${recovered.degradedSince ?? "unknown"})`, + service: SERVICE_NAME, + method: "checkRecovery", + details: { results }, + }); }; } diff --git a/server/src/domain/jobs/job.repository.interface.ts b/server/src/domain/jobs/job.repository.interface.ts index d253523fa..a352c0888 100644 --- a/server/src/domain/jobs/job.repository.interface.ts +++ b/server/src/domain/jobs/job.repository.interface.ts @@ -58,8 +58,8 @@ export interface IJobsRepository { // Delete, drop all rows deleteById(refId: string): Promise; - // Delete a single job of type - deleteByIdAndType(refId: string, type: JobType): Promise; + // Delete a single job of type. refId null addresses a global row + deleteByIdAndType(refId: string | null, type: JobType): Promise; // Delete jobs that reference monitors that no longer exist deleteByMonitorIdsNotIn(monitorIds: string[]): Promise; diff --git a/server/src/domain/jobs/job.repository.mongo.ts b/server/src/domain/jobs/job.repository.mongo.ts index 60bd4cd77..ce4143b2d 100644 --- a/server/src/domain/jobs/job.repository.mongo.ts +++ b/server/src/domain/jobs/job.repository.mongo.ts @@ -273,8 +273,8 @@ class MongoJobsRepository implements IJobsRepository { return res.deletedCount > 0; }; - deleteByIdAndType = async (refId: string, type: JobType) => { - const res = await JobModel.deleteOne({ refId, type }); // Delete a single typed row, e.g. just the geo row + 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 return res.deletedCount > 0; }; diff --git a/server/src/domain/jobs/job.type.ts b/server/src/domain/jobs/job.type.ts index 70d635f81..ae8fbddc3 100644 --- a/server/src/domain/jobs/job.type.ts +++ b/server/src/domain/jobs/job.type.ts @@ -1,10 +1,10 @@ -export const JOB_TYPES = ["check", "geo-check", "evaluate", "cleanup-orphaned", "cleanup-retention"] as const; +export const JOB_TYPES = ["check", "geo-check", "evaluate", "cleanup-orphaned", "cleanup-retention", "egress"] as const; export type JobType = (typeof JOB_TYPES)[number]; export const LOCK_MS = 60_000; export const BACKOFF_MS = 5_000; export const PARKED = Number.MAX_SAFE_INTEGER; -// Canonical _id scheme: monitor-bound rows are `${type}:${refId}`; global rows (refId null) are just the type. +// Canonical _id scheme: monitor-bound rows are `${type}:${refId}`; global rows (refId null: cleanup, egress) are just the type. export const jobId = (type: JobType, refId: string | null): string => (refId === null ? type : `${type}:${refId}`); export type Job = { diff --git a/server/src/worker/worker.db-queue.ts b/server/src/worker/worker.db-queue.ts index 6f4f31353..49f630b01 100644 --- a/server/src/worker/worker.db-queue.ts +++ b/server/src/worker/worker.db-queue.ts @@ -10,6 +10,7 @@ import { ICheckEvaluator } from "@/worker/worker.check-evaluator.js"; import { ICheckPipeline } from "@/worker/worker.check-pipeline.js"; import { IReactorDispatcher } from "@/worker/reactors/reactor.dispatcher.js"; import { IWorkerHelper } from "@/worker/worker.helper.js"; +import { IEgressService } from "@/domain/egress/egress.service.js"; import { IQueueWorkersRepository } from "@/domain/queue-workers/queue-worker.repository.interface.js"; import { WORKER_STALE_MS } from "@/domain/queue-workers/queue-worker.model.js"; import { QueueMode } from "@/domain/app-settings/app-settings.type.js"; @@ -28,6 +29,7 @@ const CONCURRENCY: Record = { evaluate: 20, "cleanup-orphaned": 1, "cleanup-retention": 1, + egress: 1, }; export interface DBQueueWorkerDependencies { @@ -43,6 +45,7 @@ export interface DBQueueWorkerDependencies { geoCheckPipeline: ICheckPipeline; dispatcher: IReactorDispatcher; helper: IWorkerHelper; + egressService: IEgressService; queueWorkersRepository: IQueueWorkersRepository; // forwarded queueMode: QueueMode; // forwarded queuePrimaryProcesses: boolean; // consumed immediately @@ -61,6 +64,7 @@ export class DBQueueWorker extends JobScheduler implements IQueueWorker { evaluate: 0, "cleanup-orphaned": 0, "cleanup-retention": 0, + egress: 0, }; private checksRepository: IChecksRepository; @@ -70,6 +74,7 @@ export class DBQueueWorker extends JobScheduler implements IQueueWorker { private dispatcher: IReactorDispatcher; private geoCheckPipeline: ICheckPipeline; private helper: IWorkerHelper; + private egressService: IEgressService; private bufferService: IBufferService; private isDbConnected: () => boolean; @@ -90,6 +95,7 @@ export class DBQueueWorker extends JobScheduler implements IQueueWorker { this.dispatcher = dependencies.dispatcher; this.geoCheckPipeline = dependencies.geoCheckPipeline; this.helper = dependencies.helper; + this.egressService = dependencies.egressService; this.bufferService = dependencies.bufferService; this.isDbConnected = dependencies.isDbConnected; } @@ -175,6 +181,9 @@ export class DBQueueWorker extends JobScheduler implements IQueueWorker { case "cleanup-retention": await this.helper.getCleanupRetentionJob()(); // Get job and execute break; + case "egress": + await this.egressService.checkRecovery(); // Re-probe while degraded; removes its own row on recovery + break; } if (job.intervalMs === null) { diff --git a/server/src/worker/worker.job-scheduler.ts b/server/src/worker/worker.job-scheduler.ts index aae8a489b..557a2f8e2 100644 --- a/server/src/worker/worker.job-scheduler.ts +++ b/server/src/worker/worker.job-scheduler.ts @@ -41,6 +41,7 @@ export class JobScheduler implements IJobScheduler { evaluate: POLL_MS, "cleanup-orphaned": POLL_MS, "cleanup-retention": POLL_MS, + egress: POLL_MS, }; // Helpers, builds job seeds. immediate=true runs on the next tick; otherwise jitter spreads the herd. protected toCheckJob = (monitor: MonitorScheduleFields, now: number, immediate = false): JobSeed => ({ diff --git a/server/test/unit/services/dbQueueWorker.test.ts b/server/test/unit/services/dbQueueWorker.test.ts index 8b13287d4..75acaa063 100644 --- a/server/test/unit/services/dbQueueWorker.test.ts +++ b/server/test/unit/services/dbQueueWorker.test.ts @@ -9,7 +9,7 @@ import { createMockLogger } from "../../helpers/createMockLogger.ts"; // ── Notes ────────────────────────────────────────────────────────────────────── // // DBQueueWorker is the orchestrator: it drives the polling loops, claims batches -// sized to free capacity, runs jobs (check / evaluate / geo-check / cleanup) and +// sized to free capacity, runs jobs (check / evaluate / geo-check / cleanup / egress) and // renews their locks, and registers itself in the worker heartbeat. The repository // claim/lock atomicity is covered in integration/jobsRepository.test.ts; here we // test only the orchestration around it, with the repository fully mocked. @@ -114,6 +114,7 @@ const createWorker = (overrides?: { queueMode?: QueueMode; queuePrimaryProcesses getCleanupOrphanedJob: jest.fn().mockReturnValue(jest.fn().mockResolvedValue(undefined)), getCleanupRetentionJob: jest.fn().mockReturnValue(jest.fn().mockResolvedValue(undefined)), }; + const egressService = { checkRecovery: jest.fn().mockResolvedValue(undefined) }; const queueWorkersRepository = { upsert: jest.fn().mockResolvedValue(undefined), deleteById: jest.fn().mockResolvedValue(undefined), @@ -134,6 +135,7 @@ const createWorker = (overrides?: { queueMode?: QueueMode; queuePrimaryProcesses geoCheckPipeline, dispatcher, helper, + egressService, queueWorkersRepository, logger, ...overrides?.mocks, @@ -226,7 +228,7 @@ describe("DBQueueWorker", () => { const { mocks } = await start({ queueMode: "worker" }); const claimedTypes = mocks.jobsRepository.claimDueBatch.mock.calls.map((c: any[]) => c[0]); - expect(claimedTypes).toEqual(expect.arrayContaining(["check", "geo-check", "evaluate", "cleanup-orphaned", "cleanup-retention"])); + expect(claimedTypes).toEqual(expect.arrayContaining(["check", "geo-check", "evaluate", "cleanup-orphaned", "cleanup-retention", "egress"])); expect(mocks.jobsRepository.claimDueBatch).toHaveBeenCalledWith("check", 500, expect.any(Number)); expect(mocks.jobsRepository.claimDueBatch).toHaveBeenCalledWith("evaluate", 20, expect.any(Number)); expect(mocks.jobsRepository.claimDueBatch).toHaveBeenCalledWith("cleanup-orphaned", 1, expect.any(Number)); @@ -440,6 +442,14 @@ describe("DBQueueWorker", () => { expect(retentionFn).toHaveBeenCalled(); }); + it("egress job runs the egress recovery check", async () => { + const job = makeJob({ id: "egress", type: "egress", refId: null, intervalMs: 30000 }); + const { mocks } = await start({ mocks: { jobsRepository: { ...createWorker().mocks.jobsRepository, claimDueBatch: claimOnce(job) } } }); + + expect(mocks.egressService.checkRecovery).toHaveBeenCalledTimes(1); + expect(mocks.jobsRepository.recordSuccess).toHaveBeenCalledWith(job.id, job.nextScheduledAt, job.intervalMs, expect.any(Number)); + }); + it("geo-check job runs the geo pipeline (no evaluate handoff)", async () => { const job = makeJob({ id: "geo-check:m1", type: "geo-check", refId: "m1", intervalMs: 300000 }); const { mocks } = await start({ mocks: { jobsRepository: { ...createWorker().mocks.jobsRepository, claimDueBatch: claimOnce(job) } } }); diff --git a/server/test/unit/services/egressService.test.ts b/server/test/unit/services/egressService.test.ts index 8c4fbf183..ffe124495 100644 --- a/server/test/unit/services/egressService.test.ts +++ b/server/test/unit/services/egressService.test.ts @@ -46,6 +46,10 @@ const createService = (overrides?: Record) => { markDegraded: jest.fn().mockResolvedValue(makeState({ status: "degraded", degradedSince: "2026-01-01T10:00:00.000Z" })), markRecovered: jest.fn().mockResolvedValue(makeState({ lastRecoveredAt: "2026-01-01T10:05:00.000Z" })), }, + jobsRepository: { + upsertJob: jest.fn().mockResolvedValue(true), + deleteByIdAndType: jest.fn().mockResolvedValue(true), + }, networkService: { requestStatus: statusFor(["1.1.1.1", "8.8.8.8"]) }, logger: createMockLogger(), ...overrides, @@ -53,17 +57,20 @@ const createService = (overrides?: Record) => { const service = new EgressService( defaults.settingsService as any, defaults.egressStateRepository as any, + defaults.jobsRepository as any, defaults.networkService as any, defaults.logger as any ); return { service, defaults }; }; -// Lets the setTimeout chain in startRecoveryPolling settle (settings read, then timer armed) before advancing time. -const runOneTick = async (intervalMs: number) => { - await jest.advanceTimersByTimeAsync(0); - await jest.advanceTimersByTimeAsync(intervalMs); -}; +const degradedRepository = (overrides?: Record) => ({ + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded", degradedSince: "2026-01-01T10:00:00.000Z" })), + recordProbe: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + markDegraded: jest.fn().mockResolvedValue(null), + markRecovered: jest.fn().mockResolvedValue(makeState({ lastRecoveredAt: "2026-01-01T10:05:00.000Z", degradedSince: "2026-01-01T10:00:00.000Z" })), + ...overrides, +}); // ── Tests ──────────────────────────────────────────────────────────────────── @@ -149,8 +156,7 @@ describe("EgressService", () => { expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled(); }); - it("marks degraded, warns and returns degraded when every target is unreachable", async () => { - jest.useFakeTimers(); + it("marks degraded, warns, inserts the recovery job and returns degraded when every target is unreachable", async () => { const { service, defaults } = createService({ networkService: { requestStatus: statusFor([]) } }); const result = await service.assessAfterFailure(); @@ -162,20 +168,18 @@ describe("EgressService", () => { ); expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); expect(defaults.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("egress degraded") })); - expect(defaults.logger.info).toHaveBeenCalledWith(expect.objectContaining({ message: "Starting egress recovery polling" })); - service.stop(); + expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith({ + id: "egress", + type: "egress", + refId: null, + isActive: true, + nextScheduledAt: expect.any(Number), + intervalMs: 30_000, + }); }); - it("returns degraded without probing when the persisted state is already degraded", async () => { - jest.useFakeTimers(); - const { service, defaults } = createService({ - egressStateRepository: { - findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - recordProbe: jest.fn(), - markDegraded: jest.fn(), - markRecovered: jest.fn(), - }, - }); + it("returns degraded without probing when the persisted state is already degraded, re-arming the recovery job", async () => { + const { service, defaults } = createService({ egressStateRepository: degradedRepository() }); const result = await service.assessAfterFailure(); @@ -183,7 +187,52 @@ describe("EgressService", () => { expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled(); expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); - service.stop(); + expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 })); + }); + + it("uses the default poll interval for the recovery job when the setting is missing or invalid", async () => { + const { service, defaults } = createService({ + settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressPollIntervalSeconds: undefined })) }, + networkService: { requestStatus: statusFor([]) }, + }); + + await service.assessAfterFailure(); + + expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ intervalMs: 30_000 })); + }); + + it("shares one probe between concurrent failures", async () => { + const { service, defaults } = createService({ networkService: { requestStatus: statusFor(["8.8.8.8"]) } }); + + const results = await Promise.all([service.assessAfterFailure(), service.assessAfterFailure(), service.assessAfterFailure()]); + + expect(results).toEqual(["ok", "ok", "ok"]); + expect(defaults.settingsService.getDBSettings).toHaveBeenCalledTimes(1); + expect(defaults.networkService.requestStatus).toHaveBeenCalledTimes(2); // one call per target, once + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); + }); + + it("reuses a settled assessment within the cache window and probes again after it", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ networkService: { requestStatus: statusFor(["8.8.8.8"]) } }); + + await service.assessAfterFailure(); + await jest.advanceTimersByTimeAsync(1000); + await service.assessAfterFailure(); + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); + + await jest.advanceTimersByTimeAsync(5000); + await service.assessAfterFailure(); + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(2); + }); + + it("does not cache an internal failure", async () => { + const getDBSettings = jest.fn().mockRejectedValueOnce(new Error("db down")).mockResolvedValue(makeSettings()); + const { service, defaults } = createService({ settingsService: { getDBSettings } }); + + expect(await service.assessAfterFailure()).toBeNull(); + expect(await service.assessAfterFailure()).toBe("ok"); + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); }); it("falls back to the default targets when the configured list is empty", async () => { @@ -209,196 +258,68 @@ describe("EgressService", () => { }); }); - // ── recovery polling ────────────────────────────────────────────────────── + // ── checkRecovery (the "egress" job) ────────────────────────────────────── - describe("recovery polling", () => { - it("marks recovered once and stops polling when a target becomes reachable", async () => { - jest.useFakeTimers(); - const recovered = makeState({ lastRecoveredAt: "2026-01-01T10:05:00.000Z", degradedSince: "2026-01-01T10:00:00.000Z" }); + describe("checkRecovery", () => { + it("marks recovered, removes the job and logs when a target becomes reachable", async () => { const { service, defaults } = createService({ - egressStateRepository: { - findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - recordProbe: jest.fn(), - markDegraded: jest.fn(), - markRecovered: jest.fn().mockResolvedValue(recovered), - }, + egressStateRepository: degradedRepository(), networkService: { requestStatus: statusFor(["1.1.1.1"]) }, }); - service.startRecoveryPolling(); - await runOneTick(30_000); + await service.checkRecovery(); - expect(defaults.egressStateRepository.markRecovered).toHaveBeenCalledTimes(1); + expect(defaults.egressStateRepository.markRecovered).toHaveBeenCalledWith( + [expect.objectContaining({ target: "1.1.1.1", reachable: true }), expect.objectContaining({ target: "8.8.8.8", reachable: false })], + expect.any(Date) + ); + expect(defaults.jobsRepository.deleteByIdAndType).toHaveBeenCalledWith(null, "egress"); expect(defaults.logger.info).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("Instance egress recovered") })); - - // Loop has stopped: a further interval produces no more work - await runOneTick(30_000); - expect(defaults.egressStateRepository.markRecovered).toHaveBeenCalledTimes(1); }); - it("does not log a recovery when another process already recorded it", async () => { - jest.useFakeTimers(); + it("removes the job but does not log a recovery when another process already recorded it", async () => { const { service, defaults } = createService({ - egressStateRepository: { - findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - recordProbe: jest.fn(), - markDegraded: jest.fn(), - markRecovered: jest.fn().mockResolvedValue(null), - }, + egressStateRepository: degradedRepository({ markRecovered: jest.fn().mockResolvedValue(null) }), networkService: { requestStatus: statusFor(["1.1.1.1"]) }, }); - service.startRecoveryPolling(); - await runOneTick(30_000); + await service.checkRecovery(); - expect(defaults.egressStateRepository.markRecovered).toHaveBeenCalledTimes(1); + expect(defaults.jobsRepository.deleteByIdAndType).toHaveBeenCalledWith(null, "egress"); expect(defaults.logger.info).not.toHaveBeenCalledWith( expect.objectContaining({ message: expect.stringContaining("Instance egress recovered") }) ); }); - it("records the probe and keeps polling while every target stays unreachable", async () => { - jest.useFakeTimers(); + it("records the probe and keeps the job while every target stays unreachable", async () => { const { service, defaults } = createService({ - egressStateRepository: { - findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - recordProbe: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - markDegraded: jest.fn(), - markRecovered: jest.fn(), - }, + egressStateRepository: degradedRepository(), networkService: { requestStatus: statusFor([]) }, }); - service.startRecoveryPolling(); - await runOneTick(30_000); - await runOneTick(30_000); + await service.checkRecovery(); - expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(2); + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); expect(defaults.egressStateRepository.markRecovered).not.toHaveBeenCalled(); - service.stop(); + expect(defaults.jobsRepository.deleteByIdAndType).not.toHaveBeenCalled(); }); - it("stops polling when the persisted state is no longer degraded", async () => { - jest.useFakeTimers(); - const { service, defaults } = createService({ - egressStateRepository: { - findSingleton: jest.fn().mockResolvedValue(makeState({ status: "ok" })), - recordProbe: jest.fn(), - markDegraded: jest.fn(), - markRecovered: jest.fn(), - }, - }); - - service.startRecoveryPolling(); - await runOneTick(30_000); - - expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); - await runOneTick(30_000); - expect(defaults.egressStateRepository.findSingleton).toHaveBeenCalledTimes(1); - }); - - it("survives a failing tick and reschedules", async () => { - jest.useFakeTimers(); - const findSingleton = jest - .fn() - .mockRejectedValueOnce(new Error("db blip")) - .mockResolvedValue(makeState({ status: "degraded" })); - const { service, defaults } = createService({ - egressStateRepository: { - findSingleton, - recordProbe: jest.fn().mockResolvedValue(makeState()), - markDegraded: jest.fn(), - markRecovered: jest.fn(), - }, - networkService: { requestStatus: statusFor([]) }, - }); - - service.startRecoveryPolling(); - await runOneTick(30_000); - expect(defaults.logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: "db blip", method: "tick" })); - - await runOneTick(30_000); - expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); - service.stop(); - }); - - it("re-reads the poll interval from settings on each tick", async () => { - jest.useFakeTimers(); - const { service, defaults } = createService({ - settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressPollIntervalSeconds: 5 })) }, - egressStateRepository: { - findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - recordProbe: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - markDegraded: jest.fn(), - markRecovered: jest.fn(), - }, - networkService: { requestStatus: statusFor([]) }, - }); - - service.startRecoveryPolling(); - await runOneTick(5_000); - - expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); - service.stop(); - }); - - it("is idempotent: a second start does not arm a second loop", async () => { - jest.useFakeTimers(); - const { service, defaults } = createService({ - egressStateRepository: { - findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - recordProbe: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - markDegraded: jest.fn(), - markRecovered: jest.fn(), - }, - networkService: { requestStatus: statusFor([]) }, - }); - - service.startRecoveryPolling(); - service.startRecoveryPolling(); - await runOneTick(30_000); - - expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); - service.stop(); - }); - }); - - // ── init ────────────────────────────────────────────────────────────────── - - describe("init", () => { - it("resumes polling when the feature is enabled and the persisted state is degraded", async () => { - jest.useFakeTimers(); - const { service, defaults } = createService({ - egressStateRepository: { - findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - recordProbe: jest.fn(), - markDegraded: jest.fn(), - markRecovered: jest.fn().mockResolvedValue(makeState()), - }, - }); - - await service.init(); - - expect(defaults.logger.info).toHaveBeenCalledWith(expect.objectContaining({ message: "Starting egress recovery polling" })); - service.stop(); - }); - - it("does nothing when the persisted state is ok", async () => { + it("removes the job without probing when the persisted state is no longer degraded", async () => { const { service, defaults } = createService(); - await service.init(); + await service.checkRecovery(); - expect(defaults.logger.info).not.toHaveBeenCalled(); + expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); + expect(defaults.egressStateRepository.markRecovered).not.toHaveBeenCalled(); + expect(defaults.jobsRepository.deleteByIdAndType).toHaveBeenCalledWith(null, "egress"); }); - it("does nothing when the feature is disabled", async () => { - const { service, defaults } = createService({ - settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false })) }, + it("lets errors propagate so the queue records the failure and retries", async () => { + const { service } = createService({ + egressStateRepository: degradedRepository({ findSingleton: jest.fn().mockRejectedValue(new Error("db down")) }), }); - await service.init(); - - expect(defaults.egressStateRepository.findSingleton).not.toHaveBeenCalled(); + await expect(service.checkRecovery()).rejects.toThrow("db down"); }); }); }); From 08f104e283d45ab393ae67c1fe97aa095bc206a7 Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Tue, 15 Sep 2026 08:42:09 +0100 Subject: [PATCH 3/4] Address review on the egress recovery job (#3937) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The egress job runs through WorkerHelper.getEgressRecoveryJob(), like the cleanup jobs, instead of a direct service call from the queue - Toggling egressCheckEnabled resets the egress state and removes any pending recovery job - Probes resolve and pass the proxy, so http targets honour the global proxy as monitors do - PingProvider keeps IPv6 addresses intact instead of truncating at the first colon - Egress targets are parsed by one function shared by the validator and the probe; malformed addresses such as unbalanced brackets are rejected - Notification id pattern drops the /i flag, which produced an invalid OpenAPI pattern - EgressState findSingleton reads before it writes, so a read no longer bumps updatedAt - "disabled" is a distinct assessment result and is cached like any other - Concurrent failures share an in-flight assessment for as long as it runs, then reuse the settled result for 5 s - The recovery job row is only removed if nobody rescheduled it since this run claimed it, so a concurrent new episode keeps its job - Deleting a notification removes it from egressNotifications 🔧 --- server/openapi.json | 5 +- .../src/api/controllers/settingsController.ts | 17 +- .../src/api/validation/settingsValidation.ts | 6 +- server/src/config/controllers.ts | 7 +- server/src/config/services.api.ts | 2 +- server/src/config/services.worker.ts | 6 +- .../egress-state.repository.interface.ts | 3 +- .../egress/egress-state.repository.mongo.ts | 12 + .../src/domain/egress/egress-state.service.ts | 17 +- server/src/domain/egress/egress.service.ts | 154 +++++++----- server/src/domain/egress/egress.type.ts | 2 + .../domain/jobs/job.repository.interface.ts | 3 + .../src/domain/jobs/job.repository.mongo.ts | 5 + .../notifications/notification.service.ts | 11 + server/src/service/network/PingProvider.ts | 12 +- server/src/utils/egressTarget.ts | 48 ++++ server/src/worker/worker.db-queue.ts | 6 +- server/src/worker/worker.helper.ts | 16 +- .../providers/network/pingProvider.test.ts | 18 ++ .../test/unit/services/dbQueueWorker.test.ts | 15 +- .../test/unit/services/egressService.test.ts | 231 +++++++++++++----- .../unit/services/egressStateService.test.ts | 34 ++- .../services/notificationsService.test.ts | 24 +- .../test/unit/services/workerHelper.test.ts | 25 +- server/test/unit/utils/egressTarget.test.ts | 49 ++++ .../validation/settingsValidation.test.ts | 29 ++- 26 files changed, 600 insertions(+), 157 deletions(-) create mode 100644 server/src/utils/egressTarget.ts create mode 100644 server/test/unit/utils/egressTarget.test.ts diff --git a/server/openapi.json b/server/openapi.json index d0ae81f26..32f5013a1 100644 --- a/server/openapi.json +++ b/server/openapi.json @@ -13303,8 +13303,7 @@ "items": { "type": "string", "minLength": 1, - "maxLength": 253, - "pattern": "^(https?:\\/\\/\\S+|[A-Za-z0-9.\\-:[\\]]+)$" + "maxLength": 253 }, "maxItems": 10 }, @@ -13317,7 +13316,7 @@ "type": "array", "items": { "type": "string", - "pattern": "^[a-f\\d]{24}$/i" + "pattern": "^[0-9a-fA-F]{24}$" } } } diff --git a/server/src/api/controllers/settingsController.ts b/server/src/api/controllers/settingsController.ts index d02c3886a..cd36467e2 100644 --- a/server/src/api/controllers/settingsController.ts +++ b/server/src/api/controllers/settingsController.ts @@ -6,6 +6,7 @@ import { AppError } from "@/utils/AppError.js"; import { ISettingsService } from "@/domain/app-settings/app-settings.service.js"; import { IEmailService } from "@/service/emailService.js"; import { IProxiesService } from "@/domain/proxies/proxy.service.js"; +import { IEgressStateService } from "@/domain/egress/egress-state.service.js"; import { Settings } from "@/domain/app-settings/app-settings.type.js"; export interface ISettingsController { @@ -18,10 +19,17 @@ class SettingsController implements ISettingsController { private settingsService: ISettingsService; private emailService: IEmailService; private proxiesService: IProxiesService; - constructor(settingsService: ISettingsService, emailService: IEmailService, proxiesService: IProxiesService) { + private egressStateService: IEgressStateService; + constructor( + settingsService: ISettingsService, + emailService: IEmailService, + proxiesService: IProxiesService, + egressStateService: IEgressStateService + ) { this.settingsService = settingsService; this.emailService = emailService; this.proxiesService = proxiesService; + this.egressStateService = egressStateService; } buildAppSettings = async (dbSettings: Settings) => { @@ -70,7 +78,14 @@ class SettingsController implements ISettingsController { } } + const previousSettings = await this.settingsService.getDBSettings(); const updatedSettings = await this.settingsService.updateDbSettings(validatedBody); + + // Switching the egress check on or off starts from a clean state: no degraded episode, no pending recovery job + if (validatedBody.egressCheckEnabled !== undefined && validatedBody.egressCheckEnabled !== previousSettings.egressCheckEnabled) { + await this.egressStateService.reset(); + } + const returnSettings = await this.buildAppSettings(updatedSettings); return res.status(200).json({ success: true, diff --git a/server/src/api/validation/settingsValidation.ts b/server/src/api/validation/settingsValidation.ts index 33fe433fc..5bf635054 100644 --- a/server/src/api/validation/settingsValidation.ts +++ b/server/src/api/validation/settingsValidation.ts @@ -1,14 +1,16 @@ import { CHECK_TTL_SENTINEL } from "@/domain/checks/check.type.js"; import { MAX_EGRESS_POLL_INTERVAL_SECONDS, MAX_EGRESS_TARGETS, MIN_EGRESS_POLL_INTERVAL_SECONDS } from "@/domain/egress/egress.type.js"; +import { parseEgressTarget } from "@/utils/egressTarget.js"; import { z } from "zod"; // A reliability target is a bare host/IP (ICMP ping), host:port (TCP connect) or an http(s) URL. +// Parsed with the same function the probe uses, so nothing the validator accepts can fail to probe. const egressTargetValidation = z .string() .trim() .min(1) .max(253) - .regex(/^(https?:\/\/\S+|[A-Za-z0-9.\-:[\]]+)$/, "Invalid egress target"); + .refine((target) => parseEgressTarget(target) !== null, "Enter a hostname or IP address, host:port, or http(s) URL"); //**************************************** // Settings Validations @@ -55,7 +57,7 @@ export const updateAppSettingsBodyValidation = z egressCheckEnabled: z.boolean().optional(), egressCheckTargets: z.array(egressTargetValidation).max(MAX_EGRESS_TARGETS).optional(), // empty list falls back to DEFAULT_EGRESS_TARGETS egressPollIntervalSeconds: z.number().int().min(MIN_EGRESS_POLL_INTERVAL_SECONDS).max(MAX_EGRESS_POLL_INTERVAL_SECONDS).optional(), - egressNotifications: z.array(z.string().regex(/^[a-f\d]{24}$/i, "Invalid notification id")).optional(), + egressNotifications: z.array(z.string().regex(/^[0-9a-fA-F]{24}$/, "Invalid notification id")).optional(), }) .strip() .superRefine((body, ctx) => { diff --git a/server/src/config/controllers.ts b/server/src/config/controllers.ts index 7787a389e..ba98030bb 100644 --- a/server/src/config/controllers.ts +++ b/server/src/config/controllers.ts @@ -36,7 +36,12 @@ export const initializeControllers = (apiServices: ApiServices): InitializedCont return { authController: new AuthController(apiServices.userService), monitorController: new MonitorController(apiServices.monitorService, apiServices.notificationsService), - settingsController: new SettingsController(apiServices.settingsService, apiServices.emailService, apiServices.proxiesService), + settingsController: new SettingsController( + apiServices.settingsService, + apiServices.emailService, + apiServices.proxiesService, + apiServices.egressStateService + ), checkController: new CheckController(apiServices.checkService), geoCheckController: new GeoCheckController(apiServices.geoChecksService), inviteController: new InviteController(apiServices.inviteService), diff --git a/server/src/config/services.api.ts b/server/src/config/services.api.ts index e929fbfe8..68b722f0a 100644 --- a/server/src/config/services.api.ts +++ b/server/src/config/services.api.ts @@ -103,7 +103,7 @@ export const buildApi = (shared: SharedServices, jobScheduler: IJobScheduler): A const tagsService = new TagsService(tagsRepository, monitorsRepository); const diagnosticService = new DiagnosticService(db); const proxiesService = new ProxiesService(proxiesRepository, monitorsRepository, settingsService); - const egressStateService = new EgressStateService(egressStateRepository); + const egressStateService = new EgressStateService(egressStateRepository, jobsRepository); return { ...shared, worker: jobScheduler, diff --git a/server/src/config/services.worker.ts b/server/src/config/services.worker.ts index b97bb56f3..e15e1cbf5 100644 --- a/server/src/config/services.worker.ts +++ b/server/src/config/services.worker.ts @@ -106,7 +106,7 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig const bufferService = new BufferService(logger, checkService, geoChecksService, dockerLogsService, settingsService, jobsRepository); const statusService = new StatusService(logger, monitorsRepository, monitorStatsRepository); const monitorStatusPolicy = new MonitorStatusPolicy(); - const egressService = new EgressService(settingsService, egressStateRepository, jobsRepository, networkService, logger); + const egressService = new EgressService(settingsService, egressStateRepository, jobsRepository, networkService, proxyResolver, logger); // *********************** // Reactors and dispatcher @@ -149,7 +149,8 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig checksRepository, incidentsRepository, geoChecksRepository, - dockerLogsRepository + dockerLogsRepository, + egressService ); const worker = await DBQueueWorker.create({ @@ -165,7 +166,6 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig geoCheckPipeline, dispatcher: reactorDispatcher, helper: workerHelper, - egressService, queueWorkersRepository, queueMode: envSettings.queueMode, queuePrimaryProcesses: envSettings.queuePrimaryProcesses, diff --git a/server/src/domain/egress/egress-state.repository.interface.ts b/server/src/domain/egress/egress-state.repository.interface.ts index 533aad656..c718d4078 100644 --- a/server/src/domain/egress/egress-state.repository.interface.ts +++ b/server/src/domain/egress/egress-state.repository.interface.ts @@ -2,9 +2,10 @@ import type { EgressProbeResult, EgressState } from "@/domain/egress/egress.type export interface IEgressStateRepository { // read - findSingleton(): Promise; // creates the "ok" singleton on first call + findSingleton(): Promise; // creates the "ok" singleton on first call, a plain read after that // update recordProbe(results: EgressProbeResult[], now: Date): Promise; + reset(): Promise; // back to "ok" with no episode; used when the feature is toggled // Atomic transitions. Each returns the updated state when this caller performed the transition, // or null when the state was already on the target side (another worker got there first). markDegraded(results: EgressProbeResult[], now: Date): Promise; diff --git a/server/src/domain/egress/egress-state.repository.mongo.ts b/server/src/domain/egress/egress-state.repository.mongo.ts index 606551e5a..a5bb110c3 100644 --- a/server/src/domain/egress/egress-state.repository.mongo.ts +++ b/server/src/domain/egress/egress-state.repository.mongo.ts @@ -27,6 +27,9 @@ class MongoEgressStateRepository implements IEgressStateRepository { }; findSingleton = async () => { + // 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" } }, @@ -35,6 +38,15 @@ class MongoEgressStateRepository implements IEgressStateRepository { return this.toEntity(doc); }; + reset = async () => { + const doc = await EgressStateModel.findOneAndUpdate( + { singleton: true }, + { $set: { status: "ok", degradedSince: null, lastProbeAt: null, lastProbeResults: [] }, $setOnInsert: { singleton: true } }, + { upsert: true, new: true, setDefaultsOnInsert: true } + ).lean(); + return this.toEntity(doc); + }; + recordProbe = async (results: EgressProbeResult[], now: Date) => { const doc = await EgressStateModel.findOneAndUpdate( { singleton: true }, diff --git a/server/src/domain/egress/egress-state.service.ts b/server/src/domain/egress/egress-state.service.ts index 35800c7ac..c4701abe9 100644 --- a/server/src/domain/egress/egress-state.service.ts +++ b/server/src/domain/egress/egress-state.service.ts @@ -1,20 +1,31 @@ import { IEgressStateRepository } from "@/domain/egress/egress-state.repository.interface.js"; +import { IJobsRepository } from "@/domain/jobs/job.repository.interface.js"; import type { EgressState } from "@/domain/egress/egress.type.js"; export interface IEgressStateService { getState(): Promise; + reset(): Promise; } const SERVICE_NAME = "EgressStateService"; -// Read-only view of the instance-global egress state for the API process. Transitions are -// owned by the worker (egress.service.ts); this exists so the controller never touches the repository. +// API-process view of the instance-global egress state. Probe-driven transitions are owned by the +// worker (egress.service.ts); this exists so the controller never touches the repositories. export class EgressStateService implements IEgressStateService { static SERVICE_NAME = SERVICE_NAME; - constructor(private egressStateRepository: IEgressStateRepository) {} + constructor( + private egressStateRepository: IEgressStateRepository, + private jobsRepository: IJobsRepository + ) {} getState = async (): Promise => { return this.egressStateRepository.findSingleton(); }; + + // 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"); + return this.egressStateRepository.reset(); + }; } diff --git a/server/src/domain/egress/egress.service.ts b/server/src/domain/egress/egress.service.ts index 9ca98f4b2..40a4d4cd2 100644 --- a/server/src/domain/egress/egress.service.ts +++ b/server/src/domain/egress/egress.service.ts @@ -2,50 +2,57 @@ import type { Monitor } from "@/domain/monitors/monitor.type.js"; import type { ISettingsService } from "@/domain/app-settings/app-settings.service.js"; import type { IEgressStateRepository } from "@/domain/egress/egress-state.repository.interface.js"; import type { IJobsRepository } from "@/domain/jobs/job.repository.interface.js"; -import { jobId, type JobSeed } from "@/domain/jobs/job.type.js"; +import { jobId, type Job, type JobSeed } from "@/domain/jobs/job.type.js"; import type { INetworkService } from "@/service/networkService.js"; +import type { IProxyResolver } from "@/service/network/ProxyResolver.js"; import type { ILogger } from "@/utils/logger.js"; import { DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, DEFAULT_EGRESS_TARGETS, + type EgressAssessment, type EgressProbeResult, type EgressStatus, } from "@/domain/egress/egress.type.js"; import { timeRequest } from "@/service/network/utils.js"; +import { parseEgressTarget, type EgressTarget } from "@/utils/egressTarget.js"; const SERVICE_NAME = "EgressService"; const PROBE_TIMEOUT_MS = 5000; -// When egress is down every monitor fails at once, so concurrent failures share one assessment -// rather than each probing. Same in-flight cache shape as ProxyResolver. -const ASSESSMENT_TTL_MS = PROBE_TIMEOUT_MS; +// How long a settled assessment answers for later failures. An assessment still in flight is shared +// for as long as it takes, so this only bounds how stale a reused answer can be. +const ASSESSMENT_REUSE_MS = 5000; // Global job row (refId null) that re-probes while degraded. Inserted when egress degrades, removed on recovery. -const RECOVERY_JOB_ID = jobId("egress", null); +const RECOVERY_JOB_TYPE = "egress" as const; +const RECOVERY_JOB_ID = jobId(RECOVERY_JOB_TYPE, null); // 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"; -// "[host]:port" or "host:port" where host has no colons (so a bare IPv6 address is not mistaken for host:port). -const HOST_PORT_PATTERN = /^(?:\[([^\]]+)\]|([^:/\s]+)):(\d{1,5})$/; - export interface IEgressService { probeTargets(targets: string[]): Promise; // Called by the check producer when a check fails. Null when the feature is off or the assessment itself failed. assessAfterFailure(): Promise; // Handler for the "egress" job: re-probes while degraded and removes the job once egress is back. - checkRecovery(): Promise; + checkRecovery(job: Job): Promise; } +type CachedAssessment = { + value: Promise; + expiresAt: number | null; // null while the assessment is still in flight +}; + export class EgressService implements IEgressService { static SERVICE_NAME = SERVICE_NAME; - private assessment: { value: Promise; expiresAt: number } | null = null; + private assessment: CachedAssessment | null = null; constructor( private settingsService: ISettingsService, private egressStateRepository: IEgressStateRepository, private jobsRepository: IJobsRepository, private networkService: INetworkService, + private proxyResolver: IProxyResolver, private logger: ILogger ) {} @@ -54,46 +61,52 @@ export class EgressService implements IEgressService { // **************************** // Reuses the existing ping/port/http providers by dressing each target up as a minimal monitor. - private toProbeMonitor = (target: string): Monitor => { + // proxyMode "inherit" lets the resolver apply the global proxy to http probes, as it does for monitors. + private toProbeMonitor = (target: EgressTarget, raw: string): Monitor => { const base = { id: PROBE_MONITOR_ID, teamId: PROBE_TEAM_ID, - name: `Egress probe: ${target}`, + name: `Egress probe: ${raw}`, method: "GET", + proxyMode: "inherit", useAdvancedMatching: false, ignoreTlsErrors: false, customUpCodes: [], }; - if (/^https?:\/\//i.test(target)) { - return { ...base, type: "http", url: target } as unknown as Monitor; + switch (target.kind) { + case "http": + return { ...base, type: "http", url: target.url } as unknown as Monitor; + case "port": + return { ...base, type: "port", url: target.host, port: target.port } as unknown as Monitor; + case "ping": + return { ...base, type: "ping", url: target.host } as unknown as Monitor; } - - const hostPort = HOST_PORT_PATTERN.exec(target); - if (hostPort) { - const host = hostPort[1] ?? hostPort[2]; - return { ...base, type: "port", url: host, port: Number(hostPort[3]) } as unknown as Monitor; - } - - return { ...base, type: "ping", url: target } as unknown as Monitor; }; - private probeTarget = async (target: string): Promise => { - const monitor = this.toProbeMonitor(target); + private probeTarget = async (raw: string): Promise => { + const target = parseEgressTarget(raw); + if (!target) { + // Validation rejects these; a stored value can still be stale, and it must not count as reachable. + return { target: raw, reachable: false, responseTime: 0, message: "Invalid egress target" }; + } + + const monitor = this.toProbeMonitor(target, raw); let timer: NodeJS.Timeout | undefined; - const { response, responseTime, error } = await timeRequest(() => - Promise.race([ - this.networkService.requestStatus(monitor), + const { response, responseTime, error } = await timeRequest(async () => { + const proxyUrl = await this.proxyResolver.resolve(monitor); + return Promise.race([ + this.networkService.requestStatus(monitor, { proxyUrl }), new Promise((_, reject) => { timer = setTimeout(() => reject(new Error(`Egress probe timed out after ${PROBE_TIMEOUT_MS}ms`)), PROBE_TIMEOUT_MS); }), - ]).finally(() => clearTimeout(timer)) - ); + ]).finally(() => clearTimeout(timer)); + }); if (error || !response) { return { - target, + target: raw, reachable: false, responseTime, message: error instanceof Error ? error.message : error ? String(error) : "No response", @@ -101,7 +114,7 @@ export class EgressService implements IEgressService { } return { - target, + target: raw, reachable: response.status === true, responseTime: response.responseTime ?? responseTime, message: response.message, @@ -127,32 +140,43 @@ export class EgressService implements IEgressService { // Event-triggered assessment // **************************** - assessAfterFailure = (): Promise => { - const now = Date.now(); - if (this.assessment && this.assessment.expiresAt > now) { - return this.assessment.value; - } - // Cache the in-flight promise so concurrent failures share one probe - const value = this.assess(); - this.assessment = { value, expiresAt: now + ASSESSMENT_TTL_MS }; - // Don't cache an internal failure, so the next failing check retries - void value.then((status) => { - if (status === null && this.assessment?.value === value) this.assessment = null; - }); - return value; + assessAfterFailure = async (): Promise => { + const assessment = await this.sharedAssessment(); + return assessment === "disabled" ? null : assessment; }; - private assess = async (): Promise => { + // When egress is down every monitor fails at once. Concurrent failures share the in-flight assessment + // however long it takes, and a settled one answers for ASSESSMENT_REUSE_MS. Same shape as ProxyResolver's cache. + private sharedAssessment = (): Promise => { + const cached = this.assessment; + if (cached && (cached.expiresAt === null || cached.expiresAt > Date.now())) { + return cached.value; + } + + const entry: CachedAssessment = { value: this.assess(), expiresAt: null }; + this.assessment = entry; + void entry.value.then((result) => { + if (this.assessment !== entry) return; + if (result === null) { + this.assessment = null; // an internal failure is not cached, so the next failing check retries + } else { + entry.expiresAt = Date.now() + ASSESSMENT_REUSE_MS; + } + }); + return entry.value; + }; + + private assess = async (): Promise => { try { const settings = await this.settingsService.getDBSettings(); if (!settings.egressCheckEnabled) { - return null; + return "disabled"; } const state = await this.egressStateRepository.findSingleton(); if (state.status === "degraded") { - // Recovery is detected by the scheduled job. Make sure it exists in case its row was lost. - await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds); + // Recovery is detected by the scheduled job. Make sure the row exists without touching its schedule. + await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds, false); return "degraded"; } @@ -173,7 +197,8 @@ export class EgressService implements IEgressService { method: "assessAfterFailure", details: { results }, }); - await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds); + // 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. @@ -191,35 +216,42 @@ export class EgressService implements IEgressService { // Recovery job // **************************** - // upsertJob only sets nextScheduledAt on insert, so re-arming while degraded never pushes a pending run back. - private scheduleRecoveryCheck = async (pollIntervalSeconds: number | undefined) => { + // reschedule=true sets nextScheduledAt (start of an episode); false only inserts the row if it is missing, + // so re-arming while degraded never pushes a pending run back. + private scheduleRecoveryCheck = async (pollIntervalSeconds: number | undefined, reschedule: boolean) => { const intervalMs = this.toIntervalMs(pollIntervalSeconds); const seed: JobSeed = { id: RECOVERY_JOB_ID, - type: "egress", + type: RECOVERY_JOB_TYPE, refId: null, isActive: true, nextScheduledAt: Date.now() + intervalMs, intervalMs, }; - await this.jobsRepository.upsertJob(seed); + if (reschedule) { + await this.jobsRepository.upsertCleanupJob(seed); // global-row upsert that also sets the schedule + } else { + await this.jobsRepository.upsertJob(seed); + } }; - private removeRecoveryJob = async () => { - await this.jobsRepository.deleteByIdAndType(null, "egress"); + // Removes the row only if nobody has rescheduled it since this run claimed it. Another worker may have seen + // egress fail again in the meantime and started a new episode; its schedule must survive. + private releaseRecoveryJob = async (job: Job) => { + await this.jobsRepository.deleteGlobalJobIfUnchanged(RECOVERY_JOB_TYPE, job.nextScheduledAt); }; // Runs on the queue at the configured interval while degraded. Errors propagate so the queue records // the failure and retries with its usual backoff. - checkRecovery = async (): Promise => { + checkRecovery = async (job: Job): Promise => { + const settings = await this.settingsService.getDBSettings(); const state = await this.egressStateRepository.findSingleton(); - if (state.status !== "degraded") { - // Another worker recorded the recovery, or the state was reset; nothing left to poll for. - await this.removeRecoveryJob(); + if (!settings.egressCheckEnabled || state.status !== "degraded") { + // Feature switched off, recovery recorded by another worker, or state reset: nothing left to poll for. + await this.releaseRecoveryJob(job); return; } - const settings = await this.settingsService.getDBSettings(); const results = await this.probeTargets(this.resolveTargets(settings.egressCheckTargets)); const now = new Date(); @@ -229,7 +261,7 @@ export class EgressService implements IEgressService { } const recovered = await this.egressStateRepository.markRecovered(results, now); - await this.removeRecoveryJob(); + await this.releaseRecoveryJob(job); if (!recovered) { // Another process performed the transition. return; diff --git a/server/src/domain/egress/egress.type.ts b/server/src/domain/egress/egress.type.ts index 386116ef7..49c8d462f 100644 --- a/server/src/domain/egress/egress.type.ts +++ b/server/src/domain/egress/egress.type.ts @@ -3,6 +3,8 @@ export const EgressStatuses = ["ok", "degraded"] as const; export type EgressStatus = (typeof EgressStatuses)[number]; +// Outcome of one assessment: a status, or "disabled" when the feature is switched off (distinct from an internal failure). +export type EgressAssessment = EgressStatus | "disabled"; // Sensible defaults for a fresh install. Bare hosts are ICMP-pinged, host:port is a TCP connect, // http(s):// URLs are fetched. Any one target being reachable means egress is fine. diff --git a/server/src/domain/jobs/job.repository.interface.ts b/server/src/domain/jobs/job.repository.interface.ts index a352c0888..6fe6b23b9 100644 --- a/server/src/domain/jobs/job.repository.interface.ts +++ b/server/src/domain/jobs/job.repository.interface.ts @@ -61,6 +61,9 @@ export interface IJobsRepository { // Delete a single job of type. refId null addresses a global row deleteByIdAndType(refId: string | null, 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; + // Delete jobs that reference monitors that no longer exist deleteByMonitorIdsNotIn(monitorIds: string[]): Promise; diff --git a/server/src/domain/jobs/job.repository.mongo.ts b/server/src/domain/jobs/job.repository.mongo.ts index ce4143b2d..d545b0241 100644 --- a/server/src/domain/jobs/job.repository.mongo.ts +++ b/server/src/domain/jobs/job.repository.mongo.ts @@ -278,6 +278,11 @@ class MongoJobsRepository implements IJobsRepository { return res.deletedCount > 0; }; + deleteGlobalJobIfUnchanged = async (type: JobType, nextScheduledAt: number) => { + const res = await JobModel.deleteOne({ _id: jobId(type, null), nextScheduledAt }); + return res.deletedCount > 0; + }; + deleteByMonitorIdsNotIn = async (monitorIds: string[]): Promise => { // remove jobs who'se refId is not null (cleanup jobs) and not in the list of monitor IDs (orphaned monitor jobs) const res = await JobModel.deleteMany({ refId: { $ne: null, $nin: monitorIds } }); diff --git a/server/src/domain/notifications/notification.service.ts b/server/src/domain/notifications/notification.service.ts index 61d8d09e3..e270e76d4 100644 --- a/server/src/domain/notifications/notification.service.ts +++ b/server/src/domain/notifications/notification.service.ts @@ -178,7 +178,18 @@ 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); 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 53f6aad0d..9194ed9d2 100644 --- a/server/src/service/network/PingProvider.ts +++ b/server/src/service/network/PingProvider.ts @@ -4,6 +4,7 @@ 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 { timeRequest } from "@/service/network/utils.js"; const SERVICE_NAME = "PingProvider"; @@ -19,10 +20,13 @@ export class PingProvider implements IStatusProvider { } private sanitizeHost(url: string): string { - return url - .replace(/^https?:\/\//, "") - .replace(/\/.*$/, "") - .replace(/:.*/, ""); + const host = url.replace(/^https?:\/\//, "").replace(/\/.*$/, ""); + // "[2001:db8::1]" or "[2001:db8::1]:53": the address is inside the brackets + 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; + return host.replace(/:.*/, ""); } async handle(monitor: Monitor): Promise> { diff --git a/server/src/utils/egressTarget.ts b/server/src/utils/egressTarget.ts new file mode 100644 index 000000000..240ad6c04 --- /dev/null +++ b/server/src/utils/egressTarget.ts @@ -0,0 +1,48 @@ +import { z } from "zod"; +import { dnsHostnameRegex } from "@/api/validation/shared.js"; + +// A reliability target for the egress self-check, in one of three shapes. Parsed here so the +// settings validator and the probe agree on exactly what is accepted. +export type EgressTarget = + | { kind: "http"; url: string } // http(s):// URL, fetched + | { kind: "port"; host: string; port: number } // host:port or [ipv6]:port, TCP connect + | { kind: "ping"; host: string }; // bare hostname or IP address, ICMP + +// "[ipv6]:port" or "host:port" where host has no colons, so a bare IPv6 address is never read as host:port. +const HOST_PORT_PATTERN = /^(?:\[([^\]]+)\]|([^:/\s[\]]+)):(\d{1,5})$/; +const BRACKETED_PATTERN = /^\[([^\]]+)\]$/; + +const isIpv6 = (value: string): boolean => z.ipv6().safeParse(value).success; +const isHostOrIpv4 = (value: string): boolean => value === "localhost" || z.ipv4().safeParse(value).success || dnsHostnameRegex.test(value); + +const parseHttp = (target: string): EgressTarget | null => { + try { + const { protocol, hostname } = new URL(target); + return (protocol === "http:" || protocol === "https:") && hostname.length > 0 ? { kind: "http", url: target } : null; + } catch { + return null; + } +}; + +export const parseEgressTarget = (raw: string): EgressTarget | null => { + const target = raw.trim(); + if (target.length === 0) return null; + + if (/^https?:\/\//i.test(target)) return parseHttp(target); + + const hostPort = HOST_PORT_PATTERN.exec(target); + if (hostPort) { + const port = Number(hostPort[3]); + if (port < 1 || port > 65535) return null; + const bracketedHost = hostPort[1]; + if (bracketedHost !== undefined) return isIpv6(bracketedHost) ? { kind: "port", host: bracketedHost, port } : null; + const host = hostPort[2]!; + return isHostOrIpv4(host) ? { kind: "port", host, port } : null; + } + + const bracketed = BRACKETED_PATTERN.exec(target); + if (bracketed) return isIpv6(bracketed[1]!) ? { kind: "ping", host: bracketed[1]! } : null; + + if (isIpv6(target) || isHostOrIpv4(target)) return { kind: "ping", host: target }; + return null; +}; diff --git a/server/src/worker/worker.db-queue.ts b/server/src/worker/worker.db-queue.ts index 49f630b01..c076151ef 100644 --- a/server/src/worker/worker.db-queue.ts +++ b/server/src/worker/worker.db-queue.ts @@ -10,7 +10,6 @@ import { ICheckEvaluator } from "@/worker/worker.check-evaluator.js"; import { ICheckPipeline } from "@/worker/worker.check-pipeline.js"; import { IReactorDispatcher } from "@/worker/reactors/reactor.dispatcher.js"; import { IWorkerHelper } from "@/worker/worker.helper.js"; -import { IEgressService } from "@/domain/egress/egress.service.js"; import { IQueueWorkersRepository } from "@/domain/queue-workers/queue-worker.repository.interface.js"; import { WORKER_STALE_MS } from "@/domain/queue-workers/queue-worker.model.js"; import { QueueMode } from "@/domain/app-settings/app-settings.type.js"; @@ -45,7 +44,6 @@ export interface DBQueueWorkerDependencies { geoCheckPipeline: ICheckPipeline; dispatcher: IReactorDispatcher; helper: IWorkerHelper; - egressService: IEgressService; queueWorkersRepository: IQueueWorkersRepository; // forwarded queueMode: QueueMode; // forwarded queuePrimaryProcesses: boolean; // consumed immediately @@ -74,7 +72,6 @@ export class DBQueueWorker extends JobScheduler implements IQueueWorker { private dispatcher: IReactorDispatcher; private geoCheckPipeline: ICheckPipeline; private helper: IWorkerHelper; - private egressService: IEgressService; private bufferService: IBufferService; private isDbConnected: () => boolean; @@ -95,7 +92,6 @@ export class DBQueueWorker extends JobScheduler implements IQueueWorker { this.dispatcher = dependencies.dispatcher; this.geoCheckPipeline = dependencies.geoCheckPipeline; this.helper = dependencies.helper; - this.egressService = dependencies.egressService; this.bufferService = dependencies.bufferService; this.isDbConnected = dependencies.isDbConnected; } @@ -182,7 +178,7 @@ export class DBQueueWorker extends JobScheduler implements IQueueWorker { await this.helper.getCleanupRetentionJob()(); // Get job and execute break; case "egress": - await this.egressService.checkRecovery(); // Re-probe while degraded; removes its own row on recovery + await this.helper.getEgressRecoveryJob()(job); // Re-probe while degraded; removes its own row on recovery break; } diff --git a/server/src/worker/worker.helper.ts b/server/src/worker/worker.helper.ts index d8a35e0b2..9e3a6aa8e 100644 --- a/server/src/worker/worker.helper.ts +++ b/server/src/worker/worker.helper.ts @@ -11,10 +11,13 @@ import { IJobsRepository } from "@/domain/jobs/job.repository.interface.js"; import { ITeamsRepository } from "@/domain/teams/team.repository.interface.js"; import { ILogger } from "@/utils/logger.js"; import { IDockerLogsRepository } from "@/domain/docker/docker-log.repository.interface.js"; +import { IEgressService } from "@/domain/egress/egress.service.js"; +import type { Job } from "@/domain/jobs/job.type.js"; export interface IWorkerHelper { getCleanupOrphanedJob(): () => Promise; getCleanupRetentionJob(): () => Promise; + getEgressRecoveryJob(): (job: Job) => Promise; } export interface MonitorActionDecision { @@ -45,6 +48,7 @@ export class WorkerHelper implements IWorkerHelper { private incidentsRepository: IIncidentsRepository; private geoChecksRepository: IGeoChecksRepository; private dockerLogsRepository: IDockerLogsRepository; + private egressService: IEgressService; constructor( logger: ILogger, @@ -57,7 +61,8 @@ export class WorkerHelper implements IWorkerHelper { checksRepository: IChecksRepository, incidentsRepository: IIncidentsRepository, geoChecksRepository: IGeoChecksRepository, - dockerLogsRepository: IDockerLogsRepository + dockerLogsRepository: IDockerLogsRepository, + egressService: IEgressService ) { this.logger = logger; this.checkService = checkService; @@ -70,8 +75,17 @@ export class WorkerHelper implements IWorkerHelper { this.incidentsRepository = incidentsRepository; this.geoChecksRepository = geoChecksRepository; this.dockerLogsRepository = dockerLogsRepository; + this.egressService = egressService; } + // Runs while instance egress is degraded; the service removes the row once egress is back. + // Errors propagate so the queue records the failure and retries with its usual backoff. + getEgressRecoveryJob = () => { + return async (job: Job) => { + await this.egressService.checkRecovery(job); + }; + }; + getCleanupOrphanedJob = () => { return async () => { try { diff --git a/server/test/unit/providers/network/pingProvider.test.ts b/server/test/unit/providers/network/pingProvider.test.ts index 04e4ad74e..6b4129850 100644 --- a/server/test/unit/providers/network/pingProvider.test.ts +++ b/server/test/unit/providers/network/pingProvider.test.ts @@ -110,6 +110,24 @@ describe("PingProvider", () => { expect(mockPing.promise.probe).toHaveBeenCalledWith("example.com"); }); + it("keeps a bare IPv6 address intact", async () => { + const mockPing = createMockPing(); + const provider = new PingProvider(mockPing as any); + + await provider.handle(makeMonitor({ url: "2606:4700:4700::1111" })); + + expect(mockPing.promise.probe).toHaveBeenCalledWith("2606:4700:4700::1111"); + }); + + it("unwraps a bracketed IPv6 address and drops its port", async () => { + const mockPing = createMockPing(); + const provider = new PingProvider(mockPing as any); + + await provider.handle(makeMonitor({ url: "[2606:4700:4700::1111]:53" })); + + expect(mockPing.promise.probe).toHaveBeenCalledWith("2606:4700:4700::1111"); + }); + it("throws AppError when url is missing", async () => { const provider = new PingProvider(createMockPing() as any); diff --git a/server/test/unit/services/dbQueueWorker.test.ts b/server/test/unit/services/dbQueueWorker.test.ts index 75acaa063..a2d532c16 100644 --- a/server/test/unit/services/dbQueueWorker.test.ts +++ b/server/test/unit/services/dbQueueWorker.test.ts @@ -113,8 +113,8 @@ const createWorker = (overrides?: { queueMode?: QueueMode; queuePrimaryProcesses const helper = { getCleanupOrphanedJob: jest.fn().mockReturnValue(jest.fn().mockResolvedValue(undefined)), getCleanupRetentionJob: jest.fn().mockReturnValue(jest.fn().mockResolvedValue(undefined)), + getEgressRecoveryJob: jest.fn().mockReturnValue(jest.fn().mockResolvedValue(undefined)), }; - const egressService = { checkRecovery: jest.fn().mockResolvedValue(undefined) }; const queueWorkersRepository = { upsert: jest.fn().mockResolvedValue(undefined), deleteById: jest.fn().mockResolvedValue(undefined), @@ -135,7 +135,6 @@ const createWorker = (overrides?: { queueMode?: QueueMode; queuePrimaryProcesses geoCheckPipeline, dispatcher, helper, - egressService, queueWorkersRepository, logger, ...overrides?.mocks, @@ -442,11 +441,17 @@ describe("DBQueueWorker", () => { expect(retentionFn).toHaveBeenCalled(); }); - it("egress job runs the egress recovery check", async () => { + it("egress job runs the helper's egress recovery function with the claimed job", async () => { const job = makeJob({ id: "egress", type: "egress", refId: null, intervalMs: 30000 }); - const { mocks } = await start({ mocks: { jobsRepository: { ...createWorker().mocks.jobsRepository, claimDueBatch: claimOnce(job) } } }); + const egressFn = jest.fn().mockResolvedValue(undefined); + const helper = { + getCleanupOrphanedJob: jest.fn().mockReturnValue(jest.fn().mockResolvedValue(undefined)), + getCleanupRetentionJob: jest.fn().mockReturnValue(jest.fn().mockResolvedValue(undefined)), + getEgressRecoveryJob: jest.fn().mockReturnValue(egressFn), + }; + const { mocks } = await start({ mocks: { jobsRepository: { ...createWorker().mocks.jobsRepository, claimDueBatch: claimOnce(job) }, helper } }); - expect(mocks.egressService.checkRecovery).toHaveBeenCalledTimes(1); + expect(egressFn).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", type: "egress" })); expect(mocks.jobsRepository.recordSuccess).toHaveBeenCalledWith(job.id, job.nextScheduledAt, job.intervalMs, expect.any(Number)); }); diff --git a/server/test/unit/services/egressService.test.ts b/server/test/unit/services/egressService.test.ts index ffe124495..8aee753d7 100644 --- a/server/test/unit/services/egressService.test.ts +++ b/server/test/unit/services/egressService.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, jest } from "@jest/globals"; import { EgressService } from "../../../src/domain/egress/egress.service.ts"; import type { EgressState } from "../../../src/domain/egress/egress.type.ts"; +import type { Job } from "../../../src/domain/jobs/job.type.ts"; import { createMockLogger } from "../../helpers/createMockLogger.ts"; // ── Helpers ────────────────────────────────────────────────────────────────── @@ -25,6 +26,23 @@ const makeSettings = (overrides?: Record) => ({ ...overrides, }); +// The claimed "egress" job row as the queue hands it to the handler +const makeJob = (overrides?: Partial): Job => ({ + id: "egress", + type: "egress", + refId: null, + isActive: true, + nextScheduledAt: 1_000, + intervalMs: 30_000, + lockedBy: "worker-1", + lockedUntil: 61_000, + runCount: 0, + failCount: 0, + lastFinishedAt: null, + lastFailReason: null, + ...overrides, +}); + // requestStatus mock that answers per target url const statusFor = (reachableTargets: string[]) => jest.fn().mockImplementation(async (monitor: any) => ({ @@ -37,6 +55,15 @@ const statusFor = (reachableTargets: string[]) => responseTime: 12, })); +const degradedRepository = (overrides?: Record) => ({ + findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded", degradedSince: "2026-01-01T10:00:00.000Z" })), + recordProbe: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), + markDegraded: jest.fn().mockResolvedValue(null), + markRecovered: jest.fn().mockResolvedValue(makeState({ lastRecoveredAt: "2026-01-01T10:05:00.000Z", degradedSince: "2026-01-01T10:00:00.000Z" })), + reset: jest.fn(), + ...overrides, +}); + const createService = (overrides?: Record) => { const defaults = { settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings()) }, @@ -45,12 +72,15 @@ const createService = (overrides?: Record) => { recordProbe: jest.fn().mockResolvedValue(makeState()), markDegraded: jest.fn().mockResolvedValue(makeState({ status: "degraded", degradedSince: "2026-01-01T10:00:00.000Z" })), markRecovered: jest.fn().mockResolvedValue(makeState({ lastRecoveredAt: "2026-01-01T10:05:00.000Z" })), + reset: jest.fn(), }, jobsRepository: { upsertJob: jest.fn().mockResolvedValue(true), - deleteByIdAndType: jest.fn().mockResolvedValue(true), + upsertCleanupJob: jest.fn().mockResolvedValue(true), + deleteGlobalJobIfUnchanged: jest.fn().mockResolvedValue(true), }, networkService: { requestStatus: statusFor(["1.1.1.1", "8.8.8.8"]) }, + proxyResolver: { resolve: jest.fn().mockResolvedValue(undefined) }, logger: createMockLogger(), ...overrides, }; @@ -59,18 +89,14 @@ const createService = (overrides?: Record) => { defaults.egressStateRepository as any, defaults.jobsRepository as any, defaults.networkService as any, + defaults.proxyResolver as any, defaults.logger as any ); return { service, defaults }; }; -const degradedRepository = (overrides?: Record) => ({ - findSingleton: jest.fn().mockResolvedValue(makeState({ status: "degraded", degradedSince: "2026-01-01T10:00:00.000Z" })), - recordProbe: jest.fn().mockResolvedValue(makeState({ status: "degraded" })), - markDegraded: jest.fn().mockResolvedValue(null), - markRecovered: jest.fn().mockResolvedValue(makeState({ lastRecoveredAt: "2026-01-01T10:05:00.000Z", degradedSince: "2026-01-01T10:00:00.000Z" })), - ...overrides, -}); +const probedUrls = (defaults: { networkService: { requestStatus: unknown } }) => + (defaults.networkService.requestStatus as jest.Mock).mock.calls.map((call) => (call[0] as any).url); // ── Tests ──────────────────────────────────────────────────────────────────── @@ -85,7 +111,7 @@ describe("EgressService", () => { it("maps a bare host to a ping monitor, host:port to a port monitor and a URL to an http monitor", async () => { const { service, defaults } = createService(); - await service.probeTargets(["1.1.1.1", "8.8.8.8:53", "https://example.com/health", "[2606:4700::1111]:443"]); + await service.probeTargets(["1.1.1.1", "8.8.8.8:53", "https://example.com/health", "[2606:4700::1111]:443", "2606:4700::1111"]); const monitors = (defaults.networkService.requestStatus as jest.Mock).mock.calls.map((call) => call[0] as any); expect(monitors).toEqual([ @@ -93,9 +119,22 @@ describe("EgressService", () => { expect.objectContaining({ type: "port", url: "8.8.8.8", port: 53 }), expect.objectContaining({ type: "http", url: "https://example.com/health", method: "GET", useAdvancedMatching: false }), expect.objectContaining({ type: "port", url: "2606:4700::1111", port: 443 }), + expect.objectContaining({ type: "ping", url: "2606:4700::1111" }), ]); }); + it("passes the resolved proxy to the provider so http probes honour the global proxy", async () => { + const resolve = jest.fn().mockResolvedValue("http://proxy.internal:3128"); + const { service, defaults } = createService({ proxyResolver: { resolve } }); + + await service.probeTargets(["https://example.com/health"]); + + expect(resolve).toHaveBeenCalledWith(expect.objectContaining({ type: "http", proxyMode: "inherit" })); + expect(defaults.networkService.requestStatus).toHaveBeenCalledWith(expect.objectContaining({ type: "http" }), { + proxyUrl: "http://proxy.internal:3128", + }); + }); + it("reports reachable only when the provider says status is true", async () => { const { service } = createService({ networkService: { requestStatus: statusFor(["1.1.1.1"]) } }); @@ -107,6 +146,15 @@ describe("EgressService", () => { ]); }); + it("counts an unparseable stored target as unreachable without probing", async () => { + const { service, defaults } = createService(); + + const results = await service.probeTargets(["[1.1.1.1"]); + + expect(results).toEqual([expect.objectContaining({ target: "[1.1.1.1", reachable: false, message: "Invalid egress target" })]); + expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); + }); + it("counts a thrown provider error as unreachable", async () => { const { service } = createService({ networkService: { requestStatus: jest.fn().mockRejectedValue(new Error("ENETUNREACH")) } }); @@ -154,9 +202,11 @@ describe("EgressService", () => { expect.any(Date) ); expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled(); + expect(defaults.jobsRepository.upsertJob).not.toHaveBeenCalled(); + expect(defaults.jobsRepository.upsertCleanupJob).not.toHaveBeenCalled(); }); - it("marks degraded, warns, inserts the recovery job and returns degraded when every target is unreachable", async () => { + it("marks degraded, warns, schedules the recovery job and returns degraded when every target is unreachable", async () => { const { service, defaults } = createService({ networkService: { requestStatus: statusFor([]) } }); const result = await service.assessAfterFailure(); @@ -168,7 +218,8 @@ describe("EgressService", () => { ); expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); expect(defaults.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("egress degraded") })); - expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith({ + // The worker that performed the transition sets the schedule + expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith({ id: "egress", type: "egress", refId: null, @@ -176,9 +227,26 @@ describe("EgressService", () => { nextScheduledAt: expect.any(Number), intervalMs: 30_000, }); + expect(defaults.jobsRepository.upsertJob).not.toHaveBeenCalled(); }); - it("returns degraded without probing when the persisted state is already degraded, re-arming the recovery job", async () => { + it("only makes sure the job row exists when another worker performed the transition", async () => { + const { service, defaults } = createService({ + networkService: { requestStatus: statusFor([]) }, + egressStateRepository: degradedRepository({ + findSingleton: jest.fn().mockResolvedValue(makeState()), + markDegraded: jest.fn().mockResolvedValue(null), + }), + }); + + 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(); + }); + + it("returns degraded without probing when the persisted state is already degraded, without touching the job's schedule", async () => { const { service, defaults } = createService({ egressStateRepository: degradedRepository() }); const result = await service.assessAfterFailure(); @@ -188,6 +256,7 @@ describe("EgressService", () => { expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled(); expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 })); + expect(defaults.jobsRepository.upsertCleanupJob).not.toHaveBeenCalled(); }); it("uses the default poll interval for the recovery job when the setting is missing or invalid", async () => { @@ -198,41 +267,7 @@ describe("EgressService", () => { await service.assessAfterFailure(); - expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ intervalMs: 30_000 })); - }); - - it("shares one probe between concurrent failures", async () => { - const { service, defaults } = createService({ networkService: { requestStatus: statusFor(["8.8.8.8"]) } }); - - const results = await Promise.all([service.assessAfterFailure(), service.assessAfterFailure(), service.assessAfterFailure()]); - - expect(results).toEqual(["ok", "ok", "ok"]); - expect(defaults.settingsService.getDBSettings).toHaveBeenCalledTimes(1); - expect(defaults.networkService.requestStatus).toHaveBeenCalledTimes(2); // one call per target, once - expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); - }); - - it("reuses a settled assessment within the cache window and probes again after it", async () => { - jest.useFakeTimers(); - const { service, defaults } = createService({ networkService: { requestStatus: statusFor(["8.8.8.8"]) } }); - - await service.assessAfterFailure(); - await jest.advanceTimersByTimeAsync(1000); - await service.assessAfterFailure(); - expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); - - await jest.advanceTimersByTimeAsync(5000); - await service.assessAfterFailure(); - expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(2); - }); - - it("does not cache an internal failure", async () => { - const getDBSettings = jest.fn().mockRejectedValueOnce(new Error("db down")).mockResolvedValue(makeSettings()); - const { service, defaults } = createService({ settingsService: { getDBSettings } }); - - expect(await service.assessAfterFailure()).toBeNull(); - expect(await service.assessAfterFailure()).toBe("ok"); - expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); + expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith(expect.objectContaining({ intervalMs: 30_000 })); }); it("falls back to the default targets when the configured list is empty", async () => { @@ -242,8 +277,7 @@ describe("EgressService", () => { await service.assessAfterFailure(); - const urls = (defaults.networkService.requestStatus as jest.Mock).mock.calls.map((call) => (call[0] as any).url); - expect(urls).toEqual(["1.1.1.1", "8.8.8.8"]); + expect(probedUrls(defaults)).toEqual(["1.1.1.1", "8.8.8.8"]); }); it("returns null and logs when the assessment itself fails", async () => { @@ -258,34 +292,97 @@ describe("EgressService", () => { }); }); + // ── shared assessment ───────────────────────────────────────────────────── + + describe("shared assessment", () => { + it("shares one probe between concurrent failures", async () => { + const { service, defaults } = createService({ networkService: { requestStatus: statusFor(["8.8.8.8"]) } }); + + const results = await Promise.all([service.assessAfterFailure(), service.assessAfterFailure(), service.assessAfterFailure()]); + + expect(results).toEqual(["ok", "ok", "ok"]); + expect(defaults.settingsService.getDBSettings).toHaveBeenCalledTimes(1); + expect(defaults.networkService.requestStatus).toHaveBeenCalledTimes(2); // one call per target, once + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); + }); + + it("keeps sharing an in-flight assessment that outlives the reuse window", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ networkService: { requestStatus: jest.fn().mockReturnValue(new Promise(() => {})) } }); + + const first = service.assessAfterFailure(); + await jest.advanceTimersByTimeAsync(4_900); // probes still hanging, past the 5 s reuse window from the first call + const second = service.assessAfterFailure(); + await jest.advanceTimersByTimeAsync(200); // probe timeout fires, both settle + + expect(await Promise.all([first, second])).toEqual(["degraded", "degraded"]); + expect(defaults.networkService.requestStatus).toHaveBeenCalledTimes(2); // one round of two targets, not two rounds + }); + + it("reuses a settled assessment within the reuse window and probes again after it", async () => { + jest.useFakeTimers(); + const { service, defaults } = createService({ networkService: { requestStatus: statusFor(["8.8.8.8"]) } }); + + await service.assessAfterFailure(); + await jest.advanceTimersByTimeAsync(1_000); + await service.assessAfterFailure(); + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); + + await jest.advanceTimersByTimeAsync(5_000); + await service.assessAfterFailure(); + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(2); + }); + + it("reuses a disabled result instead of re-reading settings for every failure", async () => { + const getDBSettings = jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false })); + const { service } = createService({ settingsService: { getDBSettings } }); + + expect(await service.assessAfterFailure()).toBeNull(); + expect(await service.assessAfterFailure()).toBeNull(); + + expect(getDBSettings).toHaveBeenCalledTimes(1); + }); + + it("does not cache an internal failure", async () => { + const getDBSettings = jest.fn().mockRejectedValueOnce(new Error("db down")).mockResolvedValue(makeSettings()); + const { service, defaults } = createService({ settingsService: { getDBSettings } }); + + expect(await service.assessAfterFailure()).toBeNull(); + expect(await service.assessAfterFailure()).toBe("ok"); + expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); + }); + }); + // ── checkRecovery (the "egress" job) ────────────────────────────────────── describe("checkRecovery", () => { - it("marks recovered, removes the job and logs when a target becomes reachable", async () => { + it("marks recovered, releases the job and logs when a target becomes reachable", async () => { const { service, defaults } = createService({ egressStateRepository: degradedRepository(), networkService: { requestStatus: statusFor(["1.1.1.1"]) }, }); + const job = makeJob({ nextScheduledAt: 123_456 }); - await service.checkRecovery(); + await service.checkRecovery(job); expect(defaults.egressStateRepository.markRecovered).toHaveBeenCalledWith( [expect.objectContaining({ target: "1.1.1.1", reachable: true }), expect.objectContaining({ target: "8.8.8.8", reachable: false })], expect.any(Date) ); - expect(defaults.jobsRepository.deleteByIdAndType).toHaveBeenCalledWith(null, "egress"); + // Conditional on the schedule this run claimed, so a concurrent new episode keeps its job + expect(defaults.jobsRepository.deleteGlobalJobIfUnchanged).toHaveBeenCalledWith("egress", 123_456); expect(defaults.logger.info).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("Instance egress recovered") })); }); - it("removes the job but does not log a recovery when another process already recorded it", async () => { + it("releases the job but does not log a recovery when another process already recorded it", async () => { const { service, defaults } = createService({ egressStateRepository: degradedRepository({ markRecovered: jest.fn().mockResolvedValue(null) }), networkService: { requestStatus: statusFor(["1.1.1.1"]) }, }); - await service.checkRecovery(); + await service.checkRecovery(makeJob()); - expect(defaults.jobsRepository.deleteByIdAndType).toHaveBeenCalledWith(null, "egress"); + expect(defaults.jobsRepository.deleteGlobalJobIfUnchanged).toHaveBeenCalledWith("egress", 1_000); expect(defaults.logger.info).not.toHaveBeenCalledWith( expect.objectContaining({ message: expect.stringContaining("Instance egress recovered") }) ); @@ -297,21 +394,33 @@ describe("EgressService", () => { networkService: { requestStatus: statusFor([]) }, }); - await service.checkRecovery(); + await service.checkRecovery(makeJob()); expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1); expect(defaults.egressStateRepository.markRecovered).not.toHaveBeenCalled(); - expect(defaults.jobsRepository.deleteByIdAndType).not.toHaveBeenCalled(); + expect(defaults.jobsRepository.deleteGlobalJobIfUnchanged).not.toHaveBeenCalled(); }); - it("removes the job without probing when the persisted state is no longer degraded", async () => { + it("releases the job without probing when the persisted state is no longer degraded", async () => { const { service, defaults } = createService(); - await service.checkRecovery(); + await service.checkRecovery(makeJob()); expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); expect(defaults.egressStateRepository.markRecovered).not.toHaveBeenCalled(); - expect(defaults.jobsRepository.deleteByIdAndType).toHaveBeenCalledWith(null, "egress"); + expect(defaults.jobsRepository.deleteGlobalJobIfUnchanged).toHaveBeenCalledWith("egress", 1_000); + }); + + it("releases the job without probing when the feature has been disabled mid-episode", async () => { + const { service, defaults } = createService({ + settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false })) }, + egressStateRepository: degradedRepository(), + }); + + await service.checkRecovery(makeJob()); + + expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); + expect(defaults.jobsRepository.deleteGlobalJobIfUnchanged).toHaveBeenCalledWith("egress", 1_000); }); it("lets errors propagate so the queue records the failure and retries", async () => { @@ -319,7 +428,7 @@ describe("EgressService", () => { egressStateRepository: degradedRepository({ findSingleton: jest.fn().mockRejectedValue(new Error("db down")) }), }); - await expect(service.checkRecovery()).rejects.toThrow("db down"); + await expect(service.checkRecovery(makeJob())).rejects.toThrow("db down"); }); }); }); diff --git a/server/test/unit/services/egressStateService.test.ts b/server/test/unit/services/egressStateService.test.ts index 85c32f32d..a75c0de1f 100644 --- a/server/test/unit/services/egressStateService.test.ts +++ b/server/test/unit/services/egressStateService.test.ts @@ -9,12 +9,18 @@ const createEgressStateRepo = () => ({ recordProbe: jest.fn(), markDegraded: jest.fn(), markRecovered: jest.fn(), + reset: jest.fn(), +}); + +const createJobsRepo = () => ({ + deleteByIdAndType: jest.fn().mockResolvedValue(true), }); const createService = () => { const egressStateRepository = createEgressStateRepo(); - const service = new EgressStateService(egressStateRepository as any); - return { service, egressStateRepository }; + const jobsRepository = createJobsRepo(); + const service = new EgressStateService(egressStateRepository as any, jobsRepository as any); + return { service, egressStateRepository, jobsRepository }; }; const makeState = (overrides?: Partial): EgressState => ({ @@ -67,4 +73,28 @@ describe("EgressStateService", () => { await expect(service.getState()).rejects.toThrow("db down"); }); }); + + describe("reset", () => { + it("removes any pending recovery job and returns the state to ok", async () => { + const { service, egressStateRepository, jobsRepository } = createService(); + const reset = makeState(); + (egressStateRepository.reset as jest.Mock).mockResolvedValue(reset); + + const result = await service.reset(); + + expect(jobsRepository.deleteByIdAndType).toHaveBeenCalledWith(null, "egress"); + expect(egressStateRepository.reset).toHaveBeenCalledTimes(1); + expect(result).toEqual(reset); + }); + + it("does not touch the probe-driven transitions", async () => { + const { service, egressStateRepository } = createService(); + (egressStateRepository.reset as jest.Mock).mockResolvedValue(makeState()); + + await service.reset(); + + expect(egressStateRepository.markDegraded).not.toHaveBeenCalled(); + expect(egressStateRepository.markRecovered).not.toHaveBeenCalled(); + }); + }); }); diff --git a/server/test/unit/services/notificationsService.test.ts b/server/test/unit/services/notificationsService.test.ts index 8e1633803..f2462866b 100644 --- a/server/test/unit/services/notificationsService.test.ts +++ b/server/test/unit/services/notificationsService.test.ts @@ -26,8 +26,10 @@ const createMonitorsRepo = () => ({ removeNotificationFromMonitors: jest.fn(), }); -const createSettingsService = (clientHost = "https://app.example.com") => ({ +const createSettingsService = (clientHost = "https://app.example.com", egressNotifications: string[] = []) => ({ getSettings: jest.fn().mockReturnValue({ clientHost }), + getDBSettings: jest.fn().mockResolvedValue({ egressNotifications }), + updateDbSettings: jest.fn().mockResolvedValue({ egressNotifications: [] }), }); const createMessageBuilder = () => ({ @@ -371,5 +373,25 @@ describe("NotificationsService", () => { expect(result).toBe(deleted); 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 }); + (notificationsRepository.deleteById as jest.Mock).mockResolvedValue(makeNotification()); + + await service.deleteById("notif-1", "team-1"); + + expect(settingsService.updateDbSettings).toHaveBeenCalledWith({ egressNotifications: ["notif-0"] }); + }); + + 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()); + + await service.deleteById("notif-1", "team-1"); + + expect(settingsService.updateDbSettings).not.toHaveBeenCalled(); + }); }); }); diff --git a/server/test/unit/services/workerHelper.test.ts b/server/test/unit/services/workerHelper.test.ts index 749804768..93e224491 100644 --- a/server/test/unit/services/workerHelper.test.ts +++ b/server/test/unit/services/workerHelper.test.ts @@ -41,6 +41,9 @@ const createHelper = (overrides?: Record) => { const checkService = { deleteOlderThan: jest.fn().mockResolvedValue(0), }; + const egressService = { + checkRecovery: jest.fn().mockResolvedValue(undefined), + }; const defaults = { logger: createMockLogger(), @@ -54,6 +57,7 @@ const createHelper = (overrides?: Record) => { incidentsRepository, geoChecksRepository, dockerLogsRepository, + egressService, ...overrides, }; @@ -68,7 +72,8 @@ const createHelper = (overrides?: Record) => { defaults.checksRepository as any, defaults.incidentsRepository as any, defaults.geoChecksRepository as any, - defaults.dockerLogsRepository as any + defaults.dockerLogsRepository as any, + defaults.egressService as any ); return { helper, defaults }; }; @@ -184,4 +189,22 @@ describe("WorkerHelper", () => { expect(defaults.logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: "Unknown error", stack: undefined })); }); }); + + describe("getEgressRecoveryJob", () => { + it("returns a function that hands the claimed job to the egress service", async () => { + const { helper, defaults } = createHelper(); + const job = { id: "egress", type: "egress", refId: null, nextScheduledAt: 1000 } as any; + + await helper.getEgressRecoveryJob()(job); + + expect(defaults.egressService.checkRecovery).toHaveBeenCalledWith(job); + }); + + it("lets the egress service's error propagate so the queue records the failure", async () => { + const { helper, defaults } = createHelper(); + (defaults.egressService.checkRecovery as jest.Mock).mockRejectedValue(new Error("db down")); + + await expect(helper.getEgressRecoveryJob()({ id: "egress" } as any)).rejects.toThrow("db down"); + }); + }); }); diff --git a/server/test/unit/utils/egressTarget.test.ts b/server/test/unit/utils/egressTarget.test.ts new file mode 100644 index 000000000..22c567bae --- /dev/null +++ b/server/test/unit/utils/egressTarget.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "@jest/globals"; +import { parseEgressTarget } from "../../../src/utils/egressTarget.ts"; + +describe("parseEgressTarget", () => { + it("parses http(s) URLs", () => { + expect(parseEgressTarget("https://example.com/health")).toEqual({ kind: "http", url: "https://example.com/health" }); + expect(parseEgressTarget("HTTP://example.com")).toEqual({ kind: "http", url: "HTTP://example.com" }); + expect(parseEgressTarget("http://[2606:4700::1111]:8080/")).toEqual({ kind: "http", url: "http://[2606:4700::1111]:8080/" }); + }); + + it("parses host:port with a hostname, IPv4 address, localhost or bracketed IPv6 address", () => { + expect(parseEgressTarget("dns.google:53")).toEqual({ kind: "port", host: "dns.google", port: 53 }); + expect(parseEgressTarget("8.8.8.8:53")).toEqual({ kind: "port", host: "8.8.8.8", port: 53 }); + expect(parseEgressTarget("localhost:8080")).toEqual({ kind: "port", host: "localhost", port: 8080 }); + expect(parseEgressTarget("[2606:4700::1111]:443")).toEqual({ kind: "port", host: "2606:4700::1111", port: 443 }); + }); + + it("parses bare hosts and addresses as ping targets", () => { + expect(parseEgressTarget("1.1.1.1")).toEqual({ kind: "ping", host: "1.1.1.1" }); + expect(parseEgressTarget("example.com")).toEqual({ kind: "ping", host: "example.com" }); + expect(parseEgressTarget("2606:4700:4700::1111")).toEqual({ kind: "ping", host: "2606:4700:4700::1111" }); + expect(parseEgressTarget("[2606:4700:4700::1111]")).toEqual({ kind: "ping", host: "2606:4700:4700::1111" }); + expect(parseEgressTarget(" 1.1.1.1 ")).toEqual({ kind: "ping", host: "1.1.1.1" }); + }); + + it("rejects anything the probe could not reach", () => { + for (const target of [ + "", + "ftp://example.com", + "http://", + "bad host", + "1.1.1.1;rm", + "[1.1.1.1", + "1.1.1.1]", + "[::1", + "::1]", + "[[::1]]", + "[not-an-address]:53", + "[1.1.1.1]:53", + "999.1.1.1", + "1.1.1.1:0", + "1.1.1.1:70000", + "1.1.1.1:", + "host-without-tld", + ]) { + expect(parseEgressTarget(target)).toBeNull(); + } + }); +}); diff --git a/server/test/unit/validation/settingsValidation.test.ts b/server/test/unit/validation/settingsValidation.test.ts index 36e25c3b2..745cbd81e 100644 --- a/server/test/unit/validation/settingsValidation.test.ts +++ b/server/test/unit/validation/settingsValidation.test.ts @@ -34,8 +34,30 @@ describe("settingsValidation", () => { expect(() => updateAppSettingsBodyValidation.parse({ egressCheckTargets: targets })).toThrow(); }); + it("accepts IPv6 targets, bare or bracketed with a port", () => { + const targets = ["2606:4700:4700::1111", "[2606:4700:4700::1111]", "[2606:4700:4700::1111]:53", "localhost", "localhost:8080"]; + expect(updateAppSettingsBodyValidation.parse({ egressCheckTargets: targets }).egressCheckTargets).toEqual(targets); + }); + it("rejects targets that are not a host, host:port or http(s) URL", () => { - for (const target of ["", " ", "ftp://example.com", "bad host", "1.1.1.1;rm"]) { + for (const target of ["", " ", "ftp://example.com", "bad host", "1.1.1.1;rm", "http://"]) { + expect(() => updateAppSettingsBodyValidation.parse({ egressCheckTargets: [target] })).toThrow(); + } + }); + + it("rejects malformed addresses that the probe could never reach", () => { + for (const target of [ + "[1.1.1.1", + "1.1.1.1]", + "[::1", + "::1]", + "[[::1]]", + "[not-an-address]:53", + "999.1.1.1", + "1.1.1.1:0", + "1.1.1.1:70000", + "[::1]:", + ]) { expect(() => updateAppSettingsBodyValidation.parse({ egressCheckTargets: [target] })).toThrow(); } }); @@ -59,6 +81,11 @@ describe("settingsValidation", () => { expect(updateAppSettingsBodyValidation.parse({ egressNotifications: [] }).egressNotifications).toEqual([]); }); + it("accepts upper-case hex notification ids", () => { + const id = "64B7F0C2A1D2E3F4A5B6C7D8"; + expect(updateAppSettingsBodyValidation.parse({ egressNotifications: [id] }).egressNotifications).toEqual([id]); + }); + it("rejects notification ids that are not 24-character hex", () => { for (const id of ["not-an-id", "64b7f0c2a1d2e3f4a5b6c7d", "zzzzzzzzzzzzzzzzzzzzzzzz", 123]) { expect(() => updateAppSettingsBodyValidation.parse({ egressNotifications: [id] })).toThrow(); From b68367b04f4ac1921dc2b8e70179857e940f584f Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Tue, 15 Sep 2026 08:59:13 +0100 Subject: [PATCH 4/4] Harden the egress recovery job after a second review pass (#3937) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Schedule the recovery job before recording the degraded transition, so a crash between the two leaves a stray row (removed on the job's first run) rather than a degraded state nothing is polling - Count any HTTP response, including 4xx/5xx, as reachable for http targets; only a transport failure counts against egress - Cascade a deleted notification out of egressNotifications with an atomic $pull in the settings repository, after the team-scoped delete - Handle two processes racing the first egress state insert - Address global job rows by one key (deleteGlobalJob on _id) and restore the deleteByIdAndType signature - Inject net into PingProvider like PortProvider - Integration tests for MongoEgressStateRepository and the global-row delete methods on the jobs repository 🔧 --- server/src/config/services.worker.ts | 2 +- .../app-settings-repository.interface.ts | 1 + .../app-settings.repository.mongo.ts | 4 + .../app-settings/app-settings.service.ts | 5 + .../egress/egress-state.repository.mongo.ts | 21 ++- .../src/domain/egress/egress-state.service.ts | 2 +- server/src/domain/egress/egress.service.ts | 12 +- .../domain/jobs/job.repository.interface.ts | 6 +- .../src/domain/jobs/job.repository.mongo.ts | 9 +- .../notifications/notification.service.ts | 14 +- server/src/service/network/PingProvider.ts | 10 +- .../integration/egressStateRepository.test.ts | 130 ++++++++++++++++++ .../test/integration/jobsRepository.test.ts | 43 ++++++ .../providers/network/pingProvider.test.ts | 29 ++-- .../test/unit/services/egressService.test.ts | 46 ++++++- .../unit/services/egressStateService.test.ts | 4 +- .../services/notificationsService.test.ts | 26 ++-- .../unit/services/settingsService.test.ts | 17 +++ 18 files changed, 319 insertions(+), 62 deletions(-) create mode 100644 server/test/integration/egressStateRepository.test.ts 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"); + }); + }); });