diff --git a/server/src/worker/worker.check-producer.ts b/server/src/worker/worker.check-producer.ts index d7b93ae3d..1d25bdd92 100644 --- a/server/src/worker/worker.check-producer.ts +++ b/server/src/worker/worker.check-producer.ts @@ -133,10 +133,19 @@ export class CheckProducer implements ICheckProducer { if (egressStatus !== null) { check.egressStatus = egressStatus; } + // Step 2c: Add to buffer this.bufferService.addToBuffer(check); - // Step 2d: Handle docker logs + // Step 2d: As with the maintenance gate above, the window is cleared at the point the monitor stops + // being evaluated, so the results either side of a spell never end up adjacent and cross the threshold + // on stale data. The length guard keeps this to one write per spell rather than one per degraded check, + // and it runs after the check is buffered so that a failed write cannot cost us the check. + if (egressStatus === "degraded" && monitor.statusWindow?.length) { + await this.monitorsRepository.updateById(monitor.id, monitor.teamId, { statusWindow: [] }); + } + + // Step 2e: Handle docker logs if (status.type === "docker") { const dockerLogs = await this.dockerLogsService.buildDockerLogs(status as MonitorStatusResponse); for (const dockerLog of dockerLogs) { diff --git a/server/test/integration/heartbeatJobDegradedEgress.test.ts b/server/test/integration/heartbeatJobDegradedEgress.test.ts index df8378790..f4eb11cdd 100644 --- a/server/test/integration/heartbeatJobDegradedEgress.test.ts +++ b/server/test/integration/heartbeatJobDegradedEgress.test.ts @@ -33,7 +33,8 @@ describe("Heartbeat job: degraded egress", () => { // Nothing downstream moved const storedMonitor = await h.monitorsRepo.findById("mon-1", "team-1"); expect(storedMonitor.status).toBe("up"); - expect(storedMonitor.statusWindow).toEqual([true, true, true, true, true]); + // The window is cleared once as the spell starts, as entering a maintenance window does. + expect(storedMonitor.statusWindow).toEqual([]); expect(h.incidentsRepo.getAll()).toHaveLength(0); expect(h.notificationsService.handleNotifications).not.toHaveBeenCalled(); }); @@ -158,7 +159,7 @@ describe("Heartbeat job: degraded egress", () => { expect(h.notificationsService.handleNotifications).toHaveBeenCalledTimes(1); }); - it("keeps counting real failures taken either side of a degraded spell", async () => { + it("starts a fresh status window after a degraded spell", async () => { const monitor = makeMonitor(); h.monitorsRepo.seed(monitor); @@ -167,16 +168,22 @@ describe("Heartbeat job: degraded egress", () => { await h.heartbeatJob(monitor); await h.heartbeatJob(monitor); - // A degraded spell must not push the window over the threshold h.setEgressStatus("degraded"); await h.heartbeatJob(monitor); await h.heartbeatJob(monitor); + expect((await h.monitorsRepo.findById("mon-1", "team-1")).statusWindow).toEqual([]); + + // A third real failure would have taken the old window to 60%. Against a window that starts again + // at the end of the spell it is one result, and the results either side are never adjacent. + h.setEgressStatus(null); + await h.heartbeatJob(monitor); expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("up"); expect(h.incidentsRepo.getAll()).toHaveLength(0); - // 3rd real failure: window [t, t, f, f, f] = 60% >= 60% - h.setEgressStatus(null); - await h.heartbeatJob(monitor); + // The monitor still goes down, once the rebuilt window holds enough real failures to cross. + for (let i = 0; i < 4; i++) { + await h.heartbeatJob(monitor); + } expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("down"); expect(h.incidentsRepo.getAll()).toHaveLength(1); diff --git a/server/test/unit/services/checkProducer.test.ts b/server/test/unit/services/checkProducer.test.ts index c83a890cc..85763bfa9 100644 --- a/server/test/unit/services/checkProducer.test.ts +++ b/server/test/unit/services/checkProducer.test.ts @@ -373,6 +373,46 @@ describe("CheckProducer", () => { expect(result?.check.egressStatus).toBe("degraded"); }); + // ── status window across a spell ───────────────────────────────────────── + // Matching the maintenance gate: cleared once as the monitor stops being evaluated, so the results either + // side of a spell are never adjacent. + + it("clears the status window on the first degraded check of a spell", async () => { + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue({ id: "check-1" }) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ statusWindow: [true, true, true, false, false] })); + + expect(defaults.monitorsRepository.updateById).toHaveBeenCalledWith("m1", "team", { statusWindow: [] }); + }); + + it("does not re-write the status window for later degraded checks in the same spell", async () => { + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue({ id: "check-1" }) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ statusWindow: [] })); + + expect(defaults.monitorsRepository.updateById).not.toHaveBeenCalled(); + }); + + it("leaves the status window alone when the probe found egress fine", async () => { + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue({ id: "check-1" }) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("ok") }, + }); + + await producer.produce(makeMonitor({ statusWindow: [true, true, true, false, false] })); + + expect(defaults.monitorsRepository.updateById).not.toHaveBeenCalled(); + }); + it("flags a failing check as ok when the probe found egress fine", async () => { const check: Record = { id: "check-1" }; const { producer } = createProducer({