Skip to content

Commit d91818f

Browse files
claude[bot]claude
andauthored
fix(webapp): remove unawaited task list metrics promises (#4380)
<!-- ccr-slack-attribution --> _Requested via [Slack thread](https://triggerdotdev.slack.com/archives/C097ZHVKZFA/p1785082528841609)_ `TaskListPresenter` created promises that nothing ever consumed. Two of the three deferred metrics promises it returned had no reader, no `await` and no `.catch()`, so when the query behind one of them failed the rejection had nowhere to go. ## Before / After **Before** - `TaskListPresenter.call()` returned four things: `tasks`, `activity`, `runningStats` and `durations`. Its only caller reads `tasks` and `runningStats`. - Every load of the tasks page therefore fired two ClickHouse queries whose results were thrown away. - If either of those two queries failed, the resulting promise rejection was unhandled — nothing was awaiting it and nothing had attached an error handler, so it surfaced as an unhandled rejection at the process level rather than as an error anyone could attribute to a request. **After** - `TaskListPresenter.call()` returns `tasks` and `runningStats` only. - Two fewer queries run per tasks-page load. - There is no longer an unconsumed promise that can reject without a handler. `runningStats` is awaited by its caller, so its failures continue to be handled the way they always were. Nothing changes on screen: the tasks page renders `hourlyActivity` and `runningStates`, and neither of the removed values fed either of those. ## How The removed values were verified unreferenced before deleting anything: - `TaskListPresenter` has exactly one caller, `UnifiedTaskListPresenter`, which reads `taskResult.tasks` and `taskResult.runningStats` and nothing else. - No file anywhere in the repo — app code, tests, or type re-exports — reads an `activity` or `durations` field off the presenter's result. - `UnifiedTaskListPresenter` builds its own `unifiedTaskListHourlyActivity` query for the 24h chart the page actually renders, which is what made the presenter's separate 7-day daily activity data redundant. - `getDailyTaskActivity` and `getAverageDurations` on `ClickHouseEnvironmentMetricsRepository` had no callers other than the two lines being deleted, so they and their now-orphaned helpers and types were removed too. Changes: - `apps/webapp/app/presenters/v3/TaskListPresenter.server.ts` — drop the `activity` and `durations` fields (both from the main return and from the no-current-worker early return) and the two repository calls behind them. Drop the unreferenced `TaskActivity` type alias. The "don't await this" comment on the remaining `runningStats` promise now spells out that the caller has to consume it. - `apps/webapp/app/services/environmentMetricsRepository.server.ts` — remove `getDailyTaskActivity` and `getAverageDurations` from the `EnvironmentMetricsRepository` interface and its ClickHouse implementation, along with `fillInDailyTaskActivity` and the `DailyTaskActivity` / `AverageDurations` types. `getCurrentRunningStats` is the control that shows the diagnosis is right. It throws on query failure in exactly the same way as the two removed methods — `if (queryError) throw queryError` — but it never produced an unhandled rejection, because `UnifiedTaskListPresenter` passes its promise into a `Promise.all(...).then(...)` chain that the route then awaits. Same failure mode, opposite outcome, and the only difference is whether anything consumes the promise. Follow-ups, not in this PR: - `AgentListPresenter` returns three sparkline promises in the same shape and they look similarly unconsumed. Left alone here to keep this change reviewable. - With these two callers gone, the `getTaskActivity` and `getAverageDurations` query builders in `@internal/clickhouse` have no remaining callers in this repo. Whether to remove them is a separate call for someone who owns that package. ## ✅ Checklist - [x] I have followed every step in the [contributing guide](https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md) - [x] The PR title follows the convention. - [x] I ran and tested the code works --- ## Testing - `pnpm run typecheck --filter webapp` — passes. This is the meaningful check here: it proves nothing still references the removed fields, methods or types. - `pnpm run format` and `pnpm run lint:fix` — clean, no changes produced. - No test file referenced the removed symbols, so no test needed updating. --- ## Changelog Server-only change, so this carries a `.server-changes/` note rather than a changeset: `.server-changes/task-list-remove-unused-metrics-queries.md`. > The tasks page no longer runs two queries whose results were never displayed, cutting wasted work on every page load and removing a source of hidden server errors --- ## Screenshots _No visual change — the removed data was never rendered._ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4d8b5d6 commit d91818f

3 files changed

Lines changed: 11 additions & 158 deletions

File tree

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+
The tasks page no longer runs two queries whose results were never displayed, cutting wasted work on every page load and removing a source of hidden server errors

apps/webapp/app/presenters/v3/TaskListPresenter.server.ts

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@ import {
66
import { $replica } from "~/db.server";
77
import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server";
88
import {
9-
type AverageDurations,
109
ClickHouseEnvironmentMetricsRepository,
1110
type CurrentRunningStats,
12-
type DailyTaskActivity,
1311
} from "~/services/environmentMetricsRepository.server";
1412
import { singleton } from "~/utils/singleton";
1513
import { findCurrentWorkerFromEnvironment } from "~/v3/models/workerDeployment.server";
@@ -21,8 +19,6 @@ export type TaskListItem = {
2119
triggerSource: TaskTriggerSource;
2220
};
2321

24-
export type TaskActivity = DailyTaskActivity[string];
25-
2622
export class TaskListPresenter {
2723
constructor(private readonly _replica: PrismaClientOrTransaction) {}
2824

@@ -55,9 +51,7 @@ export class TaskListPresenter {
5551
if (!currentWorker) {
5652
return {
5753
tasks: [],
58-
activity: Promise.resolve({} as DailyTaskActivity),
5954
runningStats: Promise.resolve({} as CurrentRunningStats),
60-
durations: Promise.resolve({} as AverageDurations),
6155
};
6256
}
6357

@@ -89,16 +83,10 @@ export class TaskListPresenter {
8983
clickhouse,
9084
});
9185

92-
// IMPORTANT: Don't await these, we want to return the promises
93-
// so we can defer the loading of the data
94-
const activity = environmentMetricsRepository.getDailyTaskActivity({
95-
organizationId,
96-
projectId,
97-
environmentId,
98-
days: 6, // This actually means 7 days, because we want to show the current day too
99-
tasks: slugs,
100-
});
101-
86+
// IMPORTANT: Don't await this, we want to return the promise
87+
// so we can defer the loading of the data. The caller is responsible for
88+
// consuming it — an unconsumed promise here would become an unhandled
89+
// rejection if the underlying query fails.
10290
const runningStats = environmentMetricsRepository.getCurrentRunningStats({
10391
organizationId,
10492
projectId,
@@ -107,15 +95,7 @@ export class TaskListPresenter {
10795
tasks: slugs,
10896
});
10997

110-
const durations = environmentMetricsRepository.getAverageDurations({
111-
organizationId,
112-
projectId,
113-
environmentId,
114-
days: 6,
115-
tasks: slugs,
116-
});
117-
118-
return { tasks, activity, runningStats, durations };
98+
return { tasks, runningStats };
11999
}
120100
}
121101

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

Lines changed: 0 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,16 @@ import { type ClickHouse } from "@internal/clickhouse";
22
import type { TaskRunStatus } from "@trigger.dev/database";
33
import { QUEUED_STATUSES } from "~/components/runs/v3/TaskRunStatus";
44

5-
export type DailyTaskActivity = Record<string, ({ day: string } & Record<TaskRunStatus, number>)[]>;
65
export type CurrentRunningStats = Record<string, { queued: number; running: number }>;
7-
export type AverageDurations = Record<string, number>;
86

97
export interface EnvironmentMetricsRepository {
10-
getDailyTaskActivity(options: {
11-
organizationId: string;
12-
projectId: string;
13-
environmentId: string;
14-
days: number;
15-
tasks: string[];
16-
}): Promise<DailyTaskActivity>;
17-
188
getCurrentRunningStats(options: {
199
organizationId: string;
2010
projectId: string;
2111
environmentId: string;
2212
days: number;
2313
tasks: string[];
2414
}): Promise<CurrentRunningStats>;
25-
26-
getAverageDurations(options: {
27-
organizationId: string;
28-
projectId: string;
29-
environmentId: string;
30-
days: number;
31-
tasks: string[];
32-
}): Promise<AverageDurations>;
3315
}
3416

3517
export type ClickHouseEnvironmentMetricsRepositoryOptions = {
@@ -39,45 +21,6 @@ export type ClickHouseEnvironmentMetricsRepositoryOptions = {
3921
export class ClickHouseEnvironmentMetricsRepository implements EnvironmentMetricsRepository {
4022
constructor(private readonly options: ClickHouseEnvironmentMetricsRepositoryOptions) {}
4123

42-
public async getDailyTaskActivity({
43-
organizationId,
44-
projectId,
45-
environmentId,
46-
days,
47-
tasks,
48-
}: {
49-
organizationId: string;
50-
projectId: string;
51-
environmentId: string;
52-
days: number;
53-
tasks: string[];
54-
}): Promise<DailyTaskActivity> {
55-
if (tasks.length === 0) {
56-
return {};
57-
}
58-
59-
const [queryError, activity] = await this.options.clickhouse.taskRuns.getTaskActivity({
60-
organizationId,
61-
projectId,
62-
environmentId,
63-
days,
64-
});
65-
66-
if (queryError) {
67-
throw queryError;
68-
}
69-
70-
return fillInDailyTaskActivity(
71-
activity.map((a) => ({
72-
taskIdentifier: a.task_identifier,
73-
status: a.status as TaskRunStatus,
74-
day: new Date(a.day),
75-
count: BigInt(a.count),
76-
})),
77-
days
78-
);
79-
}
80-
8124
public async getCurrentRunningStats({
8225
organizationId,
8326
projectId,
@@ -115,82 +58,6 @@ export class ClickHouseEnvironmentMetricsRepository implements EnvironmentMetric
11558
tasks
11659
);
11760
}
118-
119-
public async getAverageDurations({
120-
organizationId,
121-
projectId,
122-
environmentId,
123-
days,
124-
tasks,
125-
}: {
126-
organizationId: string;
127-
projectId: string;
128-
environmentId: string;
129-
days: number;
130-
tasks: string[];
131-
}): Promise<AverageDurations> {
132-
if (tasks.length === 0) {
133-
return {};
134-
}
135-
136-
const [queryError, durations] = await this.options.clickhouse.taskRuns.getAverageDurations({
137-
organizationId,
138-
projectId,
139-
environmentId,
140-
days,
141-
});
142-
143-
if (queryError) {
144-
throw queryError;
145-
}
146-
147-
return Object.fromEntries(durations.map((d) => [d.task_identifier, Number(d.duration)]));
148-
}
149-
}
150-
151-
type TaskActivityResults = Array<{
152-
taskIdentifier: string;
153-
status: TaskRunStatus;
154-
day: Date;
155-
count: bigint;
156-
}>;
157-
158-
function fillInDailyTaskActivity(activity: TaskActivityResults, days: number): DailyTaskActivity {
159-
//today with no time
160-
const today = new Date();
161-
today.setUTCHours(0, 0, 0, 0);
162-
163-
return activity.reduce((acc, a) => {
164-
let existingTask = acc[a.taskIdentifier];
165-
166-
if (!existingTask) {
167-
existingTask = [];
168-
//populate the array with the past 7 days
169-
for (let i = days; i >= 0; i--) {
170-
const day = new Date(today);
171-
day.setUTCDate(today.getDate() - i);
172-
day.setUTCHours(0, 0, 0, 0);
173-
174-
existingTask.push({
175-
day: day.toISOString(),
176-
["COMPLETED_SUCCESSFULLY"]: 0,
177-
} as { day: string } & Record<TaskRunStatus, number>);
178-
}
179-
180-
acc[a.taskIdentifier] = existingTask;
181-
}
182-
183-
const dayString = a.day.toISOString();
184-
const day = existingTask.find((d) => d.day === dayString);
185-
186-
if (!day) {
187-
return acc;
188-
}
189-
190-
day[a.status] = Number(a.count);
191-
192-
return acc;
193-
}, {} as DailyTaskActivity);
19461
}
19562

19663
type CurrentRunningStatsResults = Array<{

0 commit comments

Comments
 (0)