Skip to content

fix(webapp): remove unawaited task list metrics promises - #4380

Merged
matt-aitken merged 1 commit into
mainfrom
fix/task-list-presenter-unconsumed-promises
Jul 27, 2026
Merged

fix(webapp): remove unawaited task list metrics promises#4380
matt-aitken merged 1 commit into
mainfrom
fix/task-list-presenter-unconsumed-promises

Conversation

@claude

@claude claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Requested via Slack thread

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

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • 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.

`TaskListPresenter.call` returned three deferred promises — `activity`,
`runningStats` and `durations` — but its only caller,
`UnifiedTaskListPresenter`, reads just `tasks` and `runningStats`. The
`activity` and `durations` promises were never awaited, read or given a
rejection handler, so a failing query became an unhandled promise
rejection, and the two ClickHouse round-trips behind them ran on every
task list page load for results nothing rendered.

`UnifiedTaskListPresenter` builds its own hourly activity query for the
chart the page actually shows, so the daily activity and average
duration data had no consumer left.

Remove both fields and the calls that feed them, plus the
`getDailyTaskActivity` and `getAverageDurations` methods on
`ClickHouseEnvironmentMetricsRepository` (and the dead `TaskActivity` /
`DailyTaskActivity` / `AverageDurations` types), which had no other
callers. `runningStats` is kept and is unaffected: it is consumed by
`UnifiedTaskListPresenter`, which is why its rejections were already
handled.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PtQ34usZSzrSALPVBmoLYA
@changeset-bot

changeset-bot Bot commented Jul 26, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b2d5bf8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@claude
claude Bot marked this pull request as ready for review July 26, 2026 21:36

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@matt-aitken
matt-aitken merged commit d91818f into main Jul 27, 2026
58 of 60 checks passed
@matt-aitken
matt-aitken deleted the fix/task-list-presenter-unconsumed-promises branch July 27, 2026 10:39
@github-actions github-actions Bot mentioned this pull request Jul 27, 2026
carderne pushed a commit that referenced this pull request Jul 27, 2026
## Summary
2 new features, 9 improvements, 3 bug fixes.

## Highlights

- Allow additional environment API keys to create scoped public access
tokens through the Trigger.dev API. Use server-issued public access
tokens for batch operations so environment-scoped API keys can read
batch results.
([#4387](#4387))

## Improvements
- Preserve the partial assistant message when a chat turn's model stream
fails mid-response. `chat.agent` now passes the recovered partial to
`onTurnComplete`, and `chat.createSession`'s `turn.complete()` keeps it
before rethrowing, instead of dropping the streamed-so-far output.
([#4348](#4348))

## Server changes

These changes affect the self-hosted Docker image and Trigger.dev Cloud:

- Favorite any dashboard page to a new Favorites section in the side
menu, and customize the sidebar by renaming favorites, hiding items, and
reordering items and sections.
([#4375](#4375))
- List API endpoints now clamp the page size to a maximum of 100.
Requests asking for a larger page size return up to 100 items and keep
paginating, rather than pulling an unbounded page.
([#4360](#4360))
- Organizations without billing alerts now get default spend alert
thresholds, so you're notified before usage grows unexpectedly. The
billing limit page no longer pre-selects an option before you've set a
limit and prompts you to configure one. Alert previews now update
immediately after you change your billing limit.
([#4328](#4328))
- When you create a Personal Access Token, the generated token now shows
its first and last few characters instead of being fully hidden, so you
can confirm you copied the right value.
([#4363](#4363))
- Add metrics to the realtime backend that measure how often a single
changed run is served to multiple subscriptions in one batch.
([#4341](#4341))
- Realtime run subscriptions can now be configured to read run data
straight from the primary database, so a run's latest state is never
served from a lagging replica. Off by default; replica reads are
unchanged unless you turn it on.
([#4378](#4378))
- SSO and Directory Sync are no longer restricted to Enterprise plans —
get in touch and we can turn them on for your organization whatever plan
you're on.
([#4393](#4393))
- Improved supervisor observability: it now reports metrics for its
outbound requests, making failed calls to upstream services easier to
monitor.
([#4350](#4350))
- The runs list on a task's page now updates live — run statuses change
and newly triggered runs appear without a manual refresh, matching the
main Runs page.
([#4377](#4377))
- Speed up the Batches list page for environments with a large number of
batches, which could previously time out while loading.
([#4361](#4361))
- Container startup no longer prints database and ClickHouse connection
strings (with credentials) to the logs.
([#4346](#4346))
- 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
([#4380](#4380))

<details>
<summary>Raw changeset output</summary>

# Releases
## @trigger.dev/build@4.5.8

### Patch Changes

- Updated dependencies:
  - `@trigger.dev/core@4.5.8`
## trigger.dev@4.5.8

### Patch Changes

- Updated dependencies:
  - `@trigger.dev/core@4.5.8`
  - `@trigger.dev/build@4.5.8`
  - `@trigger.dev/schema-to-json@4.5.8`
## @trigger.dev/core@4.5.8

### Patch Changes

- Allow additional environment API keys to create scoped public access
tokens through the Trigger.dev API. Use server-issued public access
tokens for batch operations so environment-scoped API keys can read
batch results.
([#4387](#4387))
## @trigger.dev/python@4.5.8

### Patch Changes

- Updated dependencies:
  - `@trigger.dev/sdk@4.5.8`
  - `@trigger.dev/core@4.5.8`
  - `@trigger.dev/build@4.5.8`
## @trigger.dev/react-hooks@4.5.8

### Patch Changes

- Updated dependencies:
  - `@trigger.dev/core@4.5.8`
## @trigger.dev/redis-worker@4.5.8

### Patch Changes

- Updated dependencies:
  - `@trigger.dev/core@4.5.8`
## @trigger.dev/rsc@4.5.8

### Patch Changes

- Updated dependencies:
  - `@trigger.dev/core@4.5.8`
## @trigger.dev/schema-to-json@4.5.8

### Patch Changes

- Updated dependencies:
  - `@trigger.dev/core@4.5.8`
## @trigger.dev/sdk@4.5.8

### Patch Changes

- Preserve the partial assistant message when a chat turn's model stream
fails mid-response. `chat.agent` now passes the recovered partial to
`onTurnComplete`, and `chat.createSession`'s `turn.complete()` keeps it
before rethrowing, instead of dropping the streamed-so-far output.
([#4348](#4348))
- Allow additional environment API keys to create scoped public access
tokens through the Trigger.dev API. Use server-issued public access
tokens for batch operations so environment-scoped API keys can read
batch results.
([#4387](#4387))
- Updated dependencies:
  - `@trigger.dev/core@4.5.8`

</details>

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants