Skip to content

feat(frontend): improve runners errors ui#5026

Merged
jog1t merged 1 commit into
mainfrom
05-06-feat_frontend_improve_runners_errors_ui
May 15, 2026
Merged

feat(frontend): improve runners errors ui#5026
jog1t merged 1 commit into
mainfrom
05-06-feat_frontend_improve_runners_errors_ui

Conversation

@jog1t
Copy link
Copy Markdown
Contributor

@jog1t jog1t commented May 11, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 11, 2026

🚅 Deployed to the rivet-pr-5026 environment in rivet-frontend

Service Status Web Updated (UTC)
kitchen-sink ❌ Build Failed (View Logs) Web May 15, 2026 at 7:44 pm
ladle 🕒 Building (View Logs) Web May 15, 2026 at 7:43 pm
frontend-cloud 🕒 Building (View Logs) Web May 15, 2026 at 7:43 pm
frontend-inspector 🕒 Building (View Logs) Web May 15, 2026 at 7:43 pm
website 😴 Sleeping (View Logs) Web May 11, 2026 at 6:50 pm
mcp-hub ✅ Success (View Logs) Web May 11, 2026 at 6:42 pm

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 11, 2026

PR 5026 Review — feat(frontend): improve runners errors UI

Overview

This PR replaces a one-off inline error tooltip in runner-config-table.tsx with a standalone, reusable RunnerPoolErrorPopover component. The component fingerprints and groups errors by region, shows a tabbed view when multiple distinct error types exist, and surfaces a persistent indicator next to the sidebar Settings link. Additional small fixes include correcting the max-concurrent-actors description ("per runner" to "in total"), renaming the "Slots" column to "Actors", and allowing HeaderLink to forward a className prop.


Positives

  • Good component design. Extracting the shared error display into RunnerPoolErrorPopover eliminates duplicated tooltip markup and makes it easy to place the indicator in new locations.
  • Ladle stories coverage. Seven distinct stories covering single-region, multi-region same error, mixed errors, connection error, warning-only, string error, icon-only, with edit action, and gallery match the new frontend/CLAUDE.md requirement.
  • Error classification is thorough. classifyRunnerError handles all current RivetActorError variants and provides a safe fallback for unknown future variants.
  • Memoization is correctly applied. groupErrors and totalRegions are wrapped in useMemo inside the component.

Issues

1. describeKind uses default on a closed discriminated union — violates CLAUDE.md

The describeKind switch uses a default branch to catch unhandled kinds. CLAUDE.md says:

Never use a fall-through arm when matching on a TypeScript discriminated union. Enumerate every variant explicitly so adding a new variant later is a compile error instead of a silent behavior change.

kind is a closed literal-union type — "serverless_http", "serverless_connection", "serverless_invalid_sse", and "unknown" are all unhandled and fall through to default. The switch should enumerate all 7 variants explicitly (e.g. using an assertNever helper for the compile-time safety guarantee).

2. setTimeout in ErrorBody is never cleared

const handleCopied = () => {
  setCopied(true);
  setTimeout(() => setCopied(false), 1500); // no cleanup
};

If the popover closes before 1500 ms elapses the callback fires on an unmounted component. Fix with a ref + useEffect cleanup:

const timerRef = useRef<ReturnType<typeof setTimeout>>();
const handleCopied = () => {
  clearTimeout(timerRef.current);
  setCopied(true);
  timerRef.current = setTimeout(() => setCopied(false), 1500);
};
useEffect(() => () => clearTimeout(timerRef.current), []);

3. Duplicate react import blocks

runner-pool-error-popover.tsx has import { useMemo, useState } from "react" near the top and import type { ReactNode } from "react" buried after the @/components imports. Per CLAUDE.md conventions, consolidate into one line:

import { useMemo, useState, type ReactNode } from "react";

Minor Notes

  • Fixed popover width (w-[420px]). Can overflow on narrow or zoomed-in viewports. Consider max-w-[min(420px,calc(100vw-2rem))].
  • formatBody heuristic. The {} / [] prefix guard before JSON.parse is redundant since JSON.parse is already in a try/catch. Attempting JSON.parse directly would be simpler.
  • RunnerConfigErrorIndicator deduplication. The select callback silently drops subsequent errors for the same datacenter. A brief comment would aid future readers.

Summary

The core refactor is well-structured and the Ladle story coverage is excellent. The two items most worth addressing are the exhaustive-union violation in describeKind (silently swallows future error kinds) and the uncleared setTimeout in ErrorBody.

(This PR is already merged — flagging for follow-up if any of the above are worth addressing in a subsequent PR.)

@jog1t jog1t force-pushed the 05-06-fix_frontend_404_navigation_issues branch from 21070c2 to 9a22de6 Compare May 11, 2026 18:49
@jog1t jog1t force-pushed the 05-06-feat_frontend_improve_runners_errors_ui branch from 998ae2d to 3e2bff1 Compare May 11, 2026 18:49
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5026 May 11, 2026 18:49 Destroyed
@jog1t jog1t force-pushed the 05-06-feat_frontend_improve_runners_errors_ui branch from 3e2bff1 to 919b36c Compare May 15, 2026 19:30
@jog1t jog1t force-pushed the 05-06-fix_frontend_404_navigation_issues branch from 9a22de6 to 1b3211e Compare May 15, 2026 19:30
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5026 May 15, 2026 19:30 Destroyed
@jog1t jog1t mentioned this pull request May 15, 2026
11 tasks
Copy link
Copy Markdown
Contributor Author

jog1t commented May 15, 2026

Merge activity

  • May 15, 7:36 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 15, 7:44 PM UTC: Graphite rebased this pull request as part of a merge.
  • May 15, 7:44 PM UTC: @jog1t merged this pull request with Graphite.

@jog1t jog1t changed the base branch from 05-06-fix_frontend_404_navigation_issues to graphite-base/5026 May 15, 2026 19:41
@jog1t jog1t changed the base branch from graphite-base/5026 to main May 15, 2026 19:42
@jog1t jog1t force-pushed the 05-06-feat_frontend_improve_runners_errors_ui branch from 919b36c to 272227c Compare May 15, 2026 19:43
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5026 May 15, 2026 19:43 Destroyed
@jog1t jog1t merged commit 13fbc0f into main May 15, 2026
6 of 13 checks passed
@jog1t jog1t deleted the 05-06-feat_frontend_improve_runners_errors_ui branch May 15, 2026 19:44
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.

1 participant