Skip to content

Commit b71a752

Browse files
committed
fix(webapp): let a wait-time watch end when its queue is gone or unreadable
The reader always reported a live queue, so a queue_oldest_age watch on a deleted queue sat pending for its whole window and an engine failure read as a healthy zero wait.
1 parent feadd6f commit b71a752

3 files changed

Lines changed: 130 additions & 10 deletions

File tree

apps/webapp/app/services/dashboardAgentWatchChecks.server.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,16 @@ export async function readWatchQueueOldestAge(
148148
now: Date = new Date()
149149
): Promise<WatchQueueOldestAge | null> {
150150
const [breakdown, oldestQueuedAt] = await Promise.all([
151-
engine.concurrencyKeyBreakdown(environment, queueName, { limit: OLDEST_AGE_CK_LIMIT }),
152-
engine.oldestMessageInQueue(environment, queueName),
151+
engine
152+
.concurrencyKeyBreakdown(environment, queueName, { limit: OLDEST_AGE_CK_LIMIT })
153+
.catch(() => null),
154+
engine.oldestMessageInQueue(environment, queueName).catch(() => null),
153155
]);
154156

157+
// A partial read would under-report the wait and silently miss the SLA, so either read
158+
// failing makes the whole reading unavailable rather than a healthy zero.
159+
if (breakdown === null || oldestQueuedAt === null) return null;
160+
155161
const waitingKeys = breakdown.keys.filter((key) => key.queued > 0);
156162
const ageMs =
157163
waitingKeys.length > 0

apps/webapp/app/services/dashboardAgentWatchQueueChecks.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -262,24 +262,26 @@ export async function checkQueueOldestAge(
262262
thresholdMinutes: spec.thresholdMinutes,
263263
});
264264

265+
const gone = (): WatchCheckOutcome => ({
266+
result: "terminal_unsatisfied",
267+
facts: { queue: spec.queue, reason: "queue_not_found" },
268+
observed: unobserved(true),
269+
});
270+
265271
const reading = await deps.readQueueOldestAge(spec.queue);
266272

267273
if (reading === null) {
268-
const exists = await deps.queueExists(spec.queue);
269-
if (!exists) {
270-
return {
271-
result: "terminal_unsatisfied",
272-
facts: { queue: spec.queue, reason: "queue_not_found" },
273-
observed: unobserved(true),
274-
};
275-
}
274+
if (!(await deps.queueExists(spec.queue))) return gone();
276275
return {
277276
result: "unavailable",
278277
facts: { queue: spec.queue, reason: "age_unavailable" },
279278
observed: unobserved(false),
280279
};
281280
}
282281

282+
// Nothing waiting reads the same as a deleted queue, and only the second is terminal.
283+
if (reading.ageMs === null && !(await deps.queueExists(spec.queue))) return gone();
284+
283285
const facts = {
284286
queue: spec.queue,
285287
ageMs: reading.ageMs,
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* A wait-time watch has to be able to end. The reader must say "unavailable" when the engine
3+
* can't answer rather than report a healthy zero, and a queue that no longer exists has to
4+
* resolve the watch instead of leaving it pending for its whole window.
5+
*/
6+
7+
import { beforeEach, describe, expect, test, vi } from "vitest";
8+
import type { AuthenticatedEnvironment } from "~/services/apiAuth.server";
9+
import type { WatchCheckDeps } from "~/services/dashboardAgentWatchChecks";
10+
import type { WatchSpec } from "@internal/dashboard-agent-contracts";
11+
12+
const ctx = vi.hoisted(() => ({
13+
breakdown: null as null | { keys: Array<{ queued: number; oldestEnqueuedAt: number }> },
14+
breakdownThrows: false,
15+
oldest: undefined as number | undefined,
16+
oldestThrows: false,
17+
}));
18+
19+
vi.mock("~/v3/runEngine.server", () => ({
20+
engine: {
21+
concurrencyKeyBreakdown: async () => {
22+
if (ctx.breakdownThrows) throw new Error("redis is down");
23+
return ctx.breakdown ?? { keys: [] };
24+
},
25+
oldestMessageInQueue: async () => {
26+
if (ctx.oldestThrows) throw new Error("redis is down");
27+
return ctx.oldest;
28+
},
29+
},
30+
}));
31+
32+
const { readWatchQueueOldestAge } = await import("~/services/dashboardAgentWatchChecks.server");
33+
const { checkWatch } = await import("~/services/dashboardAgentWatchChecks");
34+
35+
const NOW = new Date("2026-08-07T12:00:00.000Z");
36+
const environment = {
37+
id: "env_1",
38+
organizationId: "org_1",
39+
projectId: "proj_1",
40+
} as AuthenticatedEnvironment;
41+
42+
const SPEC: WatchSpec = {
43+
kind: "queue_oldest_age",
44+
queue: "task/send-receipt",
45+
thresholdMinutes: 5,
46+
checkEveryMinutes: 5,
47+
maxHours: 1,
48+
note: "tell me if anything waits too long",
49+
};
50+
51+
function deps(overrides: Partial<WatchCheckDeps> = {}): WatchCheckDeps {
52+
return {
53+
readRun: async () => null,
54+
queueExists: async () => true,
55+
readQueueDepth: async () => null,
56+
readQueueOldestAge: (queueName: string) => readWatchQueueOldestAge(environment, queueName, NOW),
57+
readErrorRecurrence: async () => null,
58+
readHealth: async () => null,
59+
...overrides,
60+
};
61+
}
62+
63+
beforeEach(() => {
64+
ctx.breakdown = null;
65+
ctx.breakdownThrows = false;
66+
ctx.oldest = undefined;
67+
ctx.oldestThrows = false;
68+
});
69+
70+
describe("the wait-time reading", () => {
71+
test("is unavailable when an engine read fails, not a zero wait", async () => {
72+
ctx.oldestThrows = true;
73+
expect(await readWatchQueueOldestAge(environment, "task/send-receipt", NOW)).toBeNull();
74+
75+
ctx.oldestThrows = false;
76+
ctx.breakdownThrows = true;
77+
expect(await readWatchQueueOldestAge(environment, "task/send-receipt", NOW)).toBeNull();
78+
});
79+
80+
test("reports an empty queue as a reading with no age", async () => {
81+
expect(await readWatchQueueOldestAge(environment, "task/send-receipt", NOW)).toMatchObject({
82+
ageMs: null,
83+
source: "live_queue",
84+
current: true,
85+
});
86+
});
87+
});
88+
89+
describe("a wait-time watch on a queue that is no longer there", () => {
90+
test("resolves terminally instead of sitting pending", async () => {
91+
const outcome = await checkWatch(SPEC, deps({ queueExists: async () => false }), {
92+
now: NOW,
93+
since: NOW,
94+
});
95+
96+
expect(outcome.result).toBe("terminal_unsatisfied");
97+
expect(outcome.facts).toMatchObject({ reason: "queue_not_found" });
98+
});
99+
100+
test("stays pending while the queue exists and is simply empty", async () => {
101+
const outcome = await checkWatch(SPEC, deps(), { now: NOW, since: NOW });
102+
expect(outcome.result).toBe("pending");
103+
});
104+
105+
test("is unavailable, not terminal, when the engine is down but the queue exists", async () => {
106+
ctx.breakdownThrows = true;
107+
const outcome = await checkWatch(SPEC, deps(), { now: NOW, since: NOW });
108+
109+
expect(outcome.result).toBe("unavailable");
110+
expect(outcome.facts).toMatchObject({ reason: "age_unavailable" });
111+
});
112+
});

0 commit comments

Comments
 (0)