Merge pull request #3992 from bluewave-labs/chore/refactor-cleanup

chore: refactor cleanup
This commit is contained in:
Alexander Holliday authored and GitHub committed 2026-09-24 14:10:19 -07:00
commit 1ef540679b
10 files changed
+112 -189

No files matched your search

@@ -39,7 +39,7 @@ export class NotificationMessageBuilder implements INotificationMessageBuilder {
clientHost,
metadata: {
teamId: monitor.teamId,
notificationReason: decision.notificationReason || "status_change",
notificationReason: decision.notificationReason,
},
};
}
@@ -1,4 +1,5 @@
import type { HardwareMetricKey } from "@/domain/monitors/monitor.type.js";
import { MonitorActionDecision } from "@/worker/worker.interface.js";
export const NotificationChannels = [
"email",
@@ -119,6 +120,6 @@ export interface NotificationMessage {
clientHost: string;
metadata: {
teamId: string;
notificationReason: string;
notificationReason: MonitorActionDecision["notificationReason"];
};
}
+54 -89
View File
@@ -21,7 +21,6 @@ const SERVICE_NAME = "StatusService";
const HARDWARE_ALERT_COUNTER_START = 5;
export interface IStatusService {
updateRunningStats(check: Check, monitor: Monitor): Promise<boolean>;
updateMonitorStatus(check: Check, monitor: Monitor): Promise<StatusChangeResult>;
}
@@ -37,32 +36,19 @@ export class StatusService implements IStatusService {
this.monitorStatsRepository = monitorStatsRepository;
}
async updateRunningStats(check: Check, monitor: Monitor) {
private tryUpdateRunningStats = async (check: Check, monitor: Monitor) => {
try {
await this.monitorStatsRepository.updateByMonitorId(monitor.id, {
status: check.status === true,
responseTime: check.responseTime ?? 0,
now: Date.now(),
});
return true;
} catch (error: unknown) {
this.logger.error({
service: SERVICE_NAME,
message: error instanceof Error ? error.message : "Unknown error",
method: "updateRunningStats",
stack: error instanceof Error ? error.stack : undefined,
});
return false;
}
}
private tryUpdateRunningStats = async (check: Check, monitor: Monitor) => {
const statsOk = await this.updateRunningStats(check, monitor);
if (!statsOk) {
this.logger.warn({
service: SERVICE_NAME,
method: "updateMonitorStatus",
message: `Stats update failed for monitor ${monitor.id}`,
message: error instanceof Error ? error.message : "Unknown error",
method: "tryUpdateRunningStats",
stack: error instanceof Error ? error.stack : undefined,
});
}
};
@@ -160,7 +146,7 @@ export class StatusService implements IStatusService {
updateMonitorStatus = async (check: Check, monitor: Monitor): Promise<StatusChangeResult> => {
try {
const { status, statusCode } = check;
const { status } = check;
// Update running stats
await this.tryUpdateRunningStats(check, monitor);
@@ -188,81 +174,62 @@ export class StatusService implements IStatusService {
statusChanged = newStatus === "down";
}
// Not enough data points yet — record the check and return
if (projectedWindow.length < monitor.statusWindowSize) {
const updated = await this.monitorsRepository.updateStatusWindowAndChecks(
monitor.id,
monitor.teamId,
check.status,
checkSnapshot,
monitor.statusWindowSize,
MAX_RECENT_CHECKS,
patch
);
return {
monitor: updated,
statusChanged,
prevStatus,
code: statusCode,
timestamp: Date.now(),
};
}
// First evaluate reachability status changes, which apply to all monitor types
// and take precedence over hardware breaches.
const reachabilityResult = this.computeReachability(newStatus, projectedWindow, monitor.statusWindowThreshold);
if (reachabilityResult.transitioned) {
newStatus = reachabilityResult.nextStatus;
statusChanged = true;
}
// Evaluate hardware threshold breaches
let thresholdBreaches: HardwareBreaches | undefined;
if (monitor.type === "hardware") {
const hardware = this.computeHardwareStatus({
currentStatus: newStatus,
reachabilityDown: newStatus === "down",
metrics: check,
thresholds: {
cpu: monitor.cpuAlertThreshold,
memory: monitor.memoryAlertThreshold,
disk: monitor.diskAlertThreshold,
temp: monitor.tempAlertThreshold,
},
counters: {
cpu: monitor.cpuAlertCounter,
memory: monitor.memoryAlertCounter,
disk: monitor.diskAlertCounter,
temp: monitor.tempAlertCounter,
},
});
patch.cpuAlertCounter = hardware.nextCounters.cpu;
patch.memoryAlertCounter = hardware.nextCounters.memory;
patch.diskAlertCounter = hardware.nextCounters.disk;
patch.tempAlertCounter = hardware.nextCounters.temp;
thresholdBreaches = hardware.breaches;
if (hardware.transitioned) {
newStatus = hardware.nextStatus;
if (projectedWindow.length >= monitor.statusWindowSize) {
// First evaluate reachability status changes, which apply to all monitor types
// and take precedence over hardware breaches.
const reachabilityResult = this.computeReachability(newStatus, projectedWindow, monitor.statusWindowThreshold);
if (reachabilityResult.transitioned) {
newStatus = reachabilityResult.nextStatus;
statusChanged = true;
}
}
if (monitor.type === "docker" && check.containers) {
const breaches = findContainerBreaches(monitor, check.containers);
const docker = this.computeDockerStatus({
currentStatus: newStatus,
reachabilityDown: newStatus === "down",
breaching: breaches.length > 0,
});
if (docker.transitioned) {
newStatus = docker.nextStatus;
statusChanged = true;
// Evaluate hardware threshold breaches
if (monitor.type === "hardware") {
const hardware = this.computeHardwareStatus({
currentStatus: newStatus,
reachabilityDown: newStatus === "down",
metrics: check,
thresholds: {
cpu: monitor.cpuAlertThreshold,
memory: monitor.memoryAlertThreshold,
disk: monitor.diskAlertThreshold,
temp: monitor.tempAlertThreshold,
},
counters: {
cpu: monitor.cpuAlertCounter,
memory: monitor.memoryAlertCounter,
disk: monitor.diskAlertCounter,
temp: monitor.tempAlertCounter,
},
});
patch.cpuAlertCounter = hardware.nextCounters.cpu;
patch.memoryAlertCounter = hardware.nextCounters.memory;
patch.diskAlertCounter = hardware.nextCounters.disk;
patch.tempAlertCounter = hardware.nextCounters.temp;
thresholdBreaches = hardware.breaches;
if (hardware.transitioned) {
newStatus = hardware.nextStatus;
statusChanged = true;
}
}
}
patch.status = newStatus;
if (monitor.type === "docker" && check.containers) {
const breaches = findContainerBreaches(monitor, check.containers);
const docker = this.computeDockerStatus({
currentStatus: newStatus,
reachabilityDown: newStatus === "down",
breaching: breaches.length > 0,
});
if (docker.transitioned) {
newStatus = docker.nextStatus;
statusChanged = true;
}
}
patch.status = newStatus;
}
// Single atomic write: push arrays + set status/counters
const updated = await this.monitorsRepository.updateStatusWindowAndChecks(
@@ -279,8 +246,6 @@ export class StatusService implements IStatusService {
monitor: updated,
statusChanged,
prevStatus,
code: statusCode,
timestamp: new Date().getTime(),
thresholdBreaches,
};
} catch (error: unknown) {
-2
View File
@@ -182,8 +182,6 @@ export type StatusChangeResult = {
monitor: Monitor;
statusChanged: boolean;
prevStatus: MonitorStatus;
code: number;
timestamp: number;
thresholdBreaches?: HardwareBreaches;
};
@@ -7,7 +7,6 @@ export class NotificationReactor implements IMonitorReactor {
constructor(private notificationService: INotificationsService) {}
react = async (evaluation: MonitorEvaluation) => {
if (!evaluation.decision.shouldSendNotification) return;
await this.notificationService.handleNotifications(evaluation.monitor, evaluation.check, evaluation.decision);
};
}
+9 -1
View File
@@ -10,6 +10,8 @@ import { InMemoryMonitorsRepository } from "./InMemoryMonitorsRepository.ts";
import { InMemoryIncidentsRepository } from "./InMemoryIncidentsRepository.ts";
import { createMockLogger } from "./createMockLogger.ts";
import type { Monitor } from "../../src/domain/monitors/monitor.type.ts";
import type { Check } from "../../src/domain/checks/check.type.ts";
import type { MonitorActionDecision } from "../../src/worker/worker.interface.ts";
import type { MonitorStatusResponse } from "../../src/types/network.ts";
import type { MaintenanceWindow } from "../../src/domain/maintenance-windows/maintenance-window.type.ts";
import type { EgressStatus } from "../../src/domain/egress/egress.type.ts";
@@ -117,7 +119,13 @@ export function createHeartbeatTestHarness(): HeartbeatTestHarness {
nextEgressStatus = status;
};
const notificationReactor = new NotificationReactor(notificationsService as any);
// The real NotificationsService ignores decisions with shouldSendNotification false; the stub is reached only for
// the ones it would act on, so tests can assert on handleNotifications calls as "notifications sent".
const guardedNotificationsService = {
handleNotifications: (monitor: Monitor, check: Check, decision: MonitorActionDecision) =>
decision.shouldSendNotification ? notificationsService.handleNotifications(monitor, check, decision) : Promise.resolve(false),
};
const notificationReactor = new NotificationReactor(guardedNotificationsService as any);
const incidentReactor = new IncidentReactor(incidentService as any);
const reactorDispatcher = new ReactorDispatcher(logger, [notificationReactor, incidentReactor]);
@@ -133,7 +133,7 @@ describe("Evaluate cursor with more than one processing worker", () => {
statusService: {
updateMonitorStatus: async (check: Check, monitor: Monitor) => {
evaluatedCheckIds.push(check.id);
return { monitor, statusChanged: false, prevStatus: monitor.status, code: 200, timestamp: 0 };
return { monitor, statusChanged: false, prevStatus: monitor.status };
},
} as any,
dispatcher: { dispatch: jest.fn<any>().mockResolvedValue(undefined) } as any,
@@ -144,15 +144,6 @@ describe("NotificationMessageBuilder", () => {
expect(msg.severity).toBe("success");
expect(msg.content.title).toBe("Thresholds Resolved: Test Monitor");
});
it("uses notificationReason from decision, falling back to status_change", () => {
const monitor = makeMonitor({ status: "down" });
const decision = makeDecision({ notificationReason: null });
const msg = builder.buildMessage(monitor, makeCheck(), decision, "https://app.example.com");
expect(msg.metadata.notificationReason).toBe("status_change");
});
});
// ── determineNotificationType (via buildMessage) ─────────────────────
+40 -77
View File
@@ -131,92 +131,86 @@ const makeExistingStats = (overrides?: Record<string, unknown>) => ({
// ── Tests ────────────────────────────────────────────────────────────────────
describe("StatusService", () => {
// ── updateRunningStats ───────────────────────────────────────────────────
// ── running stats (via updateMonitorStatus) ──────────────────────────────
describe("updateRunningStats", () => {
// The service now delegates the entire running-stats computation to the repository's
// atomic `updateByMonitorId`, so the service-level tests only verify forwarding and error
// handling. The math (running average, uptimePercentage, timeOfLastFailure state machine)
// is a repository-level concern and is exercised by the repository tests.
describe("running stats", () => {
// The service delegates the running-stats computation to the repository's atomic `updateByMonitorId`,
// so these cases only verify what is forwarded and that a failure never blocks the status update.
// The math (running average, uptimePercentage, timeOfLastFailure) is exercised by the repository tests.
const run = async (check: Check) => {
const monitor = makeMonitor();
const deps = createService();
(deps.monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor);
(deps.monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({});
const result = await deps.service.updateMonitorStatus(check, monitor);
return { ...deps, result };
};
it("forwards a successful check to updateByMonitorId", async () => {
const { service, monitorStatsRepository } = createService();
(monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({});
const { monitorStatsRepository } = await run(makeCheck({ responseTime: 50 }));
const result = await service.updateRunningStats(makeCheck({ responseTime: 50 }), makeMonitor());
expect(result).toBe(true);
expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith(
"mon-1",
expect.objectContaining({
status: true,
responseTime: 50,
now: expect.any(Number),
})
expect.objectContaining({ status: true, responseTime: 50, now: expect.any(Number) })
);
});
it("forwards a failed check to updateByMonitorId", async () => {
const { service, monitorStatsRepository } = createService();
(monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({});
const { monitorStatsRepository } = await run(makeCheck({ status: false, responseTime: 100 }));
const result = await service.updateRunningStats(makeCheck({ status: false, responseTime: 100 }), makeMonitor());
expect(result).toBe(true);
expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ status: false, responseTime: 100 }));
});
it("passes 0 when responseTime is undefined", async () => {
const { service, monitorStatsRepository } = createService();
(monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({});
await service.updateRunningStats(makeCheck({ responseTime: undefined as any }), makeMonitor());
const { monitorStatsRepository } = await run(makeCheck({ responseTime: undefined as any }));
expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ responseTime: 0 }));
});
it("forwards responseTime of 0 as 0 (falsy but defined)", async () => {
const { service, monitorStatsRepository } = createService();
(monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({});
await service.updateRunningStats(makeCheck({ responseTime: 0 }), makeMonitor());
const { monitorStatsRepository } = await run(makeCheck({ responseTime: 0 }));
expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ responseTime: 0 }));
});
it("coerces a non-true status to false (e.g. undefined)", async () => {
const { service, monitorStatsRepository } = createService();
(monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({});
await service.updateRunningStats(makeCheck({ status: undefined as any }), makeMonitor());
const { monitorStatsRepository } = await run(makeCheck({ status: undefined as any }));
expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ status: false }));
});
it("returns false and logs error when updateByMonitorId throws", async () => {
const { service, logger, monitorStatsRepository } = createService();
(monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue(new Error("db write failed"));
it("warns once and still updates the status when updateByMonitorId throws", async () => {
const monitor = makeMonitor({ statusWindow: [true, true, true, true], statusWindowSize: 5, status: "up" });
const { service, logger, monitorsRepository, monitorStatsRepository } = createService();
(monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor);
(monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue(new Error("stats db down"));
const result = await service.updateRunningStats(makeCheck(), makeMonitor());
const result = await service.updateMonitorStatus(makeCheck({ status: true }), monitor);
expect(result).toBe(false);
expect(logger.error).toHaveBeenCalledWith(
expect(result.monitor.status).toBe("up");
expect(monitorsRepository.updateStatusWindowAndChecks).toHaveBeenCalledTimes(1);
expect(logger.warn).toHaveBeenCalledTimes(1);
expect(logger.warn).toHaveBeenCalledWith(
expect.objectContaining({
service: "StatusService",
method: "updateRunningStats",
message: "db write failed",
method: "tryUpdateRunningStats",
message: "stats db down",
stack: expect.any(String),
})
);
expect(logger.error).not.toHaveBeenCalled();
});
it("logs error with 'Unknown error' for non-Error exceptions", async () => {
const { service, logger, monitorStatsRepository } = createService();
it("warns with 'Unknown error' for non-Error rejections", async () => {
const monitor = makeMonitor();
const { service, logger, monitorsRepository, monitorStatsRepository } = createService();
(monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor);
(monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue("string error");
const result = await service.updateRunningStats(makeCheck(), makeMonitor());
await service.updateMonitorStatus(makeCheck(), monitor);
expect(result).toBe(false);
expect(logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: "Unknown error", stack: undefined }));
expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: "Unknown error", stack: undefined }));
});
});
@@ -640,37 +634,6 @@ describe("StatusService", () => {
expect(result.monitor.status).toBe("down");
});
it("logs a warning but still succeeds when running-stats update fails mid-flight", async () => {
const monitor = makeMonitor({ statusWindow: [true, true, true, true], statusWindowSize: 5, status: "up" });
const { service, logger, monitorsRepository, monitorStatsRepository } = createService();
(monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor);
(monitorsRepository.updateById as jest.Mock).mockImplementation((_id: unknown, _tid: unknown, m: unknown) => Promise.resolve(m));
(monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue(new Error("stats db down"));
const result = await service.updateMonitorStatus(makeCheck({ status: true }), monitor);
expect(result.monitor.status).toBe("up");
expect(logger.warn).toHaveBeenCalledWith(
expect.objectContaining({
service: "StatusService",
method: "updateMonitorStatus",
message: expect.stringContaining("Stats update failed"),
})
);
});
it("returns code and timestamp in result", async () => {
const monitor = makeMonitor({ statusWindow: [true, true, true, true], statusWindowSize: 5 });
const { service, monitorsRepository } = createService();
(monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor);
(monitorsRepository.updateById as jest.Mock).mockImplementation((_id: unknown, _tid: unknown, m: unknown) => Promise.resolve(m));
const result = await service.updateMonitorStatus(makeCheck({ statusCode: 201 }), monitor);
expect(result.code).toBe(201);
expect(result.timestamp).toBeGreaterThan(0);
});
// ── Hardware threshold breach tests ──────────────────────────────────
describe("hardware threshold breaches", () => {
@@ -64,8 +64,6 @@ const makeStatusChange = (overrides: Partial<StatusChangeResult> & { status?: st
monitor: { id: "m1", status } as any,
statusChanged: false,
prevStatus: "up",
code: 200,
timestamp: 0,
...rest,
} as StatusChangeResult;
};
@@ -706,7 +704,7 @@ describe("WorkerPipeline", () => {
it("returns an evaluation that opens an incident on a down transition", async () => {
const check = makeCheck({ status: false, statusCode: 500, message: "Error" });
const statusChange = makeStatusChange({ monitor: makeMonitor({ status: "down" }), statusChanged: true, prevStatus: "up", code: 500 });
const statusChange = makeStatusChange({ monitor: makeMonitor({ status: "down" }), statusChanged: true, prevStatus: "up" });
const { pipeline } = createPipeline({
statusService: { updateMonitorStatus: jest.fn<any>().mockResolvedValue(statusChange) },
});
@@ -802,7 +800,7 @@ describe("WorkerPipeline", () => {
});
it("creates incident and notifies when monitor goes down", async () => {
const decision = await decide(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up", code: 500 }));
const decision = await decide(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up" }));
expect(decision).toMatchObject({
shouldCreateIncident: true,
shouldSendNotification: true,
@@ -854,7 +852,7 @@ describe("WorkerPipeline", () => {
it("leaves threshold breaches off a down decision even when the status change carries them", async () => {
const thresholdBreaches = { cpu: true, memory: false, disk: false, temp: false };
const decision = await decide(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up", code: 500, thresholdBreaches }));
const decision = await decide(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up", thresholdBreaches }));
expect(decision).not.toHaveProperty("thresholdBreaches");
});
@@ -876,8 +874,8 @@ describe("WorkerPipeline", () => {
it("starts every decision clean after a transition on the same pipeline", async () => {
const updateMonitorStatus = jest
.fn<any>()
.mockResolvedValueOnce(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up", code: 500 }))
.mockResolvedValueOnce(makeStatusChange({ status: "down", statusChanged: false, prevStatus: "down", code: 500 }));
.mockResolvedValueOnce(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up" }))
.mockResolvedValueOnce(makeStatusChange({ status: "down", statusChanged: false, prevStatus: "down" }));
const { pipeline } = createPipeline({ statusService: { updateMonitorStatus } });
const failing = makeCheck({ status: false, statusCode: 500, message: "Error" });