Skip to content

Commit e109a24

Browse files
committed
fix(webapp): read the alert channel and the watch target on the primary
1 parent fc4381d commit e109a24

6 files changed

Lines changed: 152 additions & 12 deletions
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Unsubscribing from watch notifications now takes effect immediately, so no alert arrives after you opt out. You can also set a watch on a run the moment you trigger it, instead of being told the run doesn't exist.

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55

66
import type { AuthenticatedEnvironment } from "~/services/apiAuth.server";
7-
import { $replica } from "~/db.server";
7+
import { $replica, prisma } from "~/db.server";
88
import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server";
99
import { ReportPresenter } from "~/presenters/v3/reports/ReportPresenter.server";
1010
import { engine } from "~/v3/runEngine.server";
@@ -52,6 +52,30 @@ export async function watchQueueExists(environmentId: string, queueName: string)
5252
return queue !== null;
5353
}
5454

55+
/** The same run read on the primary, for a target that may have been created a moment ago. */
56+
export async function readWatchRunOnPrimary(
57+
runFriendlyId: string,
58+
environmentId: string
59+
): Promise<WatchRunRow | null> {
60+
const run = await runStore.findRunOnPrimary(
61+
{ friendlyId: runFriendlyId, runtimeEnvironmentId: environmentId },
62+
{ select: WATCH_RUN_SELECT }
63+
);
64+
return run ?? null;
65+
}
66+
67+
/** The same queue read on the primary. */
68+
export async function watchQueueExistsOnPrimary(
69+
environmentId: string,
70+
queueName: string
71+
): Promise<boolean> {
72+
const queue = await prisma.taskQueue.findFirst({
73+
where: { runtimeEnvironmentId: environmentId, name: queueName },
74+
select: { id: true },
75+
});
76+
return queue !== null;
77+
}
78+
5579
/** How far back the ClickHouse depth fallback looks when the live counter is down. */
5680
const DEPTH_FALLBACK_MINUTES = 10;
5781
const DEPTH_FALLBACK_BUCKET_SECONDS = 60;
@@ -290,3 +314,18 @@ export function watchCheckDeps(
290314
readHealth: () => readWatchHealth(environment),
291315
};
292316
}
317+
318+
/**
319+
* Creation-time deps. The target reads go to the primary, so a run or queue created moments
320+
* ago is visible instead of failing as a non-existent target inside the replication window.
321+
*/
322+
export function watchCreationCheckDeps(
323+
environment: AuthenticatedEnvironment,
324+
now: Date = new Date()
325+
): WatchCheckDeps {
326+
return {
327+
...watchCheckDeps(environment, now),
328+
readRun: (runId) => readWatchRunOnPrimary(runId, environment.id),
329+
queueExists: (queue) => watchQueueExistsOnPrimary(environment.id, queue),
330+
};
331+
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import {
4141
type WatchCheckDeps,
4242
type WatchCheckOutcome,
4343
} from "~/services/dashboardAgentWatchChecks";
44-
import { watchCheckDeps } from "~/services/dashboardAgentWatchChecks.server";
44+
import { watchCreationCheckDeps } from "~/services/dashboardAgentWatchChecks.server";
4545
import {
4646
mintDashboardAgentWatchBatchToken,
4747
mintDashboardAgentWatchToken,
@@ -214,7 +214,8 @@ export async function createDashboardAgentWatch(params: {
214214
}): Promise<CreateDashboardAgentWatchResult> {
215215
const { environment, userId, chatId, spec } = params;
216216
const now = params.now ?? new Date();
217-
const buildCheckDeps = params.deps?.checkDeps ?? watchCheckDeps;
217+
// Creation reads the target on the primary; the polling checks stay on the replica.
218+
const buildCheckDeps = params.deps?.checkDeps ?? watchCreationCheckDeps;
218219
const scheduleTick = params.deps?.scheduleTick ?? scheduleWatchTick;
219220
const isDashboardAgentConfigured = params.deps?.configured ?? isDashboardAgentConfiguredDefault;
220221
const checkDeps = buildCheckDeps(environment, now);

apps/webapp/app/v3/services/alerts/deliverDashboardAgentWatchAlert.server.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,10 @@ export class DeliverDashboardAgentWatchAlertService {
180180
/** One channel's delivery. A retry here can only re-send this channel. */
181181
export class DeliverDashboardAgentWatchChannelAlertService {
182182
async call(payload: DashboardAgentWatchChannelAlertPayload): Promise<void> {
183-
// Re-read the channel rather than trusting the fan-out's snapshot: an
184-
// unsubscribe between fan-out and delivery should stop the alert.
185-
const channel = await $replica.projectAlertChannel.findFirst({
183+
// Re-read the channel rather than trusting the fan-out's snapshot: an unsubscribe
184+
// between fan-out and delivery should stop the alert. The primary, since the
185+
// unsubscribe writes there and replica lag would send the mail anyway.
186+
const channel = await prisma.projectAlertChannel.findFirst({
186187
where: {
187188
id: payload.channelId,
188189
projectId: payload.projectId,

apps/webapp/test/dashboardAgentWatchAlertFanout.test.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ const WEBHOOK_CHANNEL = {
2323

2424
const ctx = vi.hoisted(() => ({
2525
channels: [] as Array<{ id: string; type: string; properties: unknown }>,
26+
/** Set to model replica lag: what the replica still sees. Null means "same as primary". */
27+
replicaChannels: null as Array<{ id: string; type: string; properties: unknown }> | null,
2628
gateAllowed: true,
2729
webhookFails: false,
2830
}));
@@ -37,6 +39,11 @@ const safeWebhookFetch = vi.hoisted(() =>
3739
);
3840

3941
vi.mock("~/db.server", () => {
42+
const channelReader = (rows: () => Array<{ id: string; type: string; properties: unknown }>) => ({
43+
findMany: async () => rows(),
44+
findFirst: async ({ where }: { where: { id: string } }) =>
45+
rows().find((channel) => channel.id === where.id) ?? null,
46+
});
4047
const db = {
4148
runtimeEnvironment: {
4249
findFirst: async () => ({
@@ -51,11 +58,6 @@ vi.mock("~/db.server", () => {
5158
},
5259
}),
5360
},
54-
projectAlertChannel: {
55-
findMany: async () => ctx.channels,
56-
findFirst: async ({ where }: { where: { id: string } }) =>
57-
ctx.channels.find((channel) => channel.id === where.id) ?? null,
58-
},
5961
organizationIntegration: {
6062
findFirst: async () => ({
6163
id: "int_1",
@@ -65,7 +67,14 @@ vi.mock("~/db.server", () => {
6567
}),
6668
},
6769
};
68-
return { prisma: db, $replica: db, sqlDatabaseSchema: undefined };
70+
return {
71+
prisma: { ...db, projectAlertChannel: channelReader(() => ctx.channels) },
72+
$replica: {
73+
...db,
74+
projectAlertChannel: channelReader(() => ctx.replicaChannels ?? ctx.channels),
75+
},
76+
sqlDatabaseSchema: undefined,
77+
};
6978
});
7079

7180
vi.mock("~/v3/canAccessDashboardAgent.server", () => ({
@@ -111,6 +120,7 @@ const payload = {
111120

112121
beforeEach(() => {
113122
ctx.channels = [EMAIL_CHANNEL, SLACK_CHANNEL, WEBHOOK_CHANNEL];
123+
ctx.replicaChannels = null;
114124
ctx.gateAllowed = true;
115125
ctx.webhookFails = false;
116126
enqueue.mockClear();
@@ -195,6 +205,18 @@ describe("dashboard agent watch alert per-channel delivery", () => {
195205
expect(bodies[1].created).toBe(bodies[0].created);
196206
});
197207

208+
test("an unsubscribe the replica hasn't caught up on still stops the email", async () => {
209+
ctx.channels = [];
210+
ctx.replicaChannels = [EMAIL_CHANNEL];
211+
212+
await new DeliverDashboardAgentWatchChannelAlertService().call({
213+
...payload,
214+
channelId: "chan_email",
215+
});
216+
217+
expect(sendAlertEmail).not.toHaveBeenCalled();
218+
});
219+
198220
test("an unsubscribed channel delivers nothing", async () => {
199221
ctx.channels = [];
200222
await new DeliverDashboardAgentWatchChannelAlertService().call({
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { beforeEach, describe, expect, test, vi } from "vitest";
2+
import type { AuthenticatedEnvironment } from "~/services/apiAuth.server";
3+
4+
// The replica lags: it has neither the just-triggered run nor the just-created queue.
5+
const ctx = vi.hoisted(() => ({
6+
primaryReads: [] as string[],
7+
replicaReads: [] as string[],
8+
}));
9+
10+
vi.mock("~/db.server", () => ({
11+
prisma: {
12+
taskQueue: {
13+
findFirst: async () => {
14+
ctx.primaryReads.push("queue");
15+
return { id: "queue_1" };
16+
},
17+
},
18+
},
19+
$replica: {
20+
taskQueue: {
21+
findFirst: async () => {
22+
ctx.replicaReads.push("queue");
23+
return null;
24+
},
25+
},
26+
},
27+
sqlDatabaseSchema: undefined,
28+
}));
29+
30+
vi.mock("~/v3/runStore.server", () => ({
31+
runStore: {
32+
findRunOnPrimary: async () => {
33+
ctx.primaryReads.push("run");
34+
return { friendlyId: "run_1", status: "PENDING" };
35+
},
36+
findRun: async () => {
37+
ctx.replicaReads.push("run");
38+
return null;
39+
},
40+
},
41+
}));
42+
43+
const { watchCheckDeps, watchCreationCheckDeps } =
44+
await import("~/services/dashboardAgentWatchChecks.server");
45+
46+
const environment = { id: "env_1" } as AuthenticatedEnvironment;
47+
48+
beforeEach(() => {
49+
ctx.primaryReads = [];
50+
ctx.replicaReads = [];
51+
});
52+
53+
describe("the watch target reads", () => {
54+
test("creation reads the run and the queue on the primary", async () => {
55+
const deps = watchCreationCheckDeps(environment);
56+
57+
expect(await deps.readRun("run_1")).not.toBeNull();
58+
expect(await deps.queueExists("task/my-task")).toBe(true);
59+
expect(ctx.primaryReads).toEqual(["run", "queue"]);
60+
expect(ctx.replicaReads).toEqual([]);
61+
});
62+
63+
test("polling keeps both reads on the replica", async () => {
64+
const deps = watchCheckDeps(environment);
65+
66+
expect(await deps.readRun("run_1")).toBeNull();
67+
expect(await deps.queueExists("task/my-task")).toBe(false);
68+
expect(ctx.replicaReads).toEqual(["run", "queue"]);
69+
expect(ctx.primaryReads).toEqual([]);
70+
});
71+
});

0 commit comments

Comments
 (0)