Skip to content

feat(error): add technical crash report to generic error screen#2225

Merged
peppescg merged 1 commit into
mainfrom
worktree-issue-2210-error-boundaries
May 12, 2026
Merged

feat(error): add technical crash report to generic error screen#2225
peppescg merged 1 commit into
mainfrom
worktree-issue-2210-error-boundaries

Conversation

@peppescg
Copy link
Copy Markdown
Collaborator

@peppescg peppescg commented May 12, 2026

Summary

Kapture.2026-05-12.at.12.33.31.mp4
  • Enhance the GenericError screen with a collapsible "Show details" section that surfaces the error message, stack trace, and environment metadata (desktop/CLI versions, platform, ToolHive running status, container engine status)
  • Add a one-click copy button that copies a formatted crash report to clipboard for easy pasting into GitHub issues or Slack
  • Widen the error card from max-w-md to max-w-lg to accommodate technical details

Changes

File Change
error/technical-details.tsx New component: collapsible details with error message, metadata badges, stack trace, and copy-to-clipboard
error/generic-error.tsx Accept error prop and render <TechnicalDetails>
error/index.tsx Pass error through to <GenericError>
error/base-error-screen.tsx Widen card from max-w-md to max-w-lg
error/__tests__/technical-details.test.tsx 5 unit tests covering collapsed/expanded states, clipboard copy, metadata, and graceful fallback

How it works

  • Default view: "Oops, something went wrong" + "Show details" toggle — non-technical users are not overwhelmed
  • Expanded view: error message with copy icon, environment metadata as badges (Desktop, CLI, Platform, ToolHive status, Engine status), full stack trace
  • Copy button: copies a full crash report (timestamp, versions, platform, ToolHive/engine status, user agent, error message, stack trace) — users can paste it directly into a bug report

Closes #2210

Test plan

  • pnpm run lint passes
  • pnpm run type-check passes
  • 23 unit tests pass (5 new + 18 existing unchanged)
  • Manually verified in Electron dev build: error message displays, details expand/collapse, copy produces formatted report

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 12, 2026 10:35
@peppescg peppescg self-assigned this May 12, 2026
@peppescg
Copy link
Copy Markdown
Collaborator Author

the copy to clipboard details

ToolHive Crash Report
=====================
Timestamp: 2026-05-12T10:39:05.900Z
Desktop Version: 0.0.0-dev
CLI Version: v0.27.1
Platform: darwin
ToolHive Running: no
Container Engine: available
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) ToolHive/0.0.0-dev Chrome/146.0.7680.216 Electron/41.5.0 Safari/537.36

Error: Health check failed

Stack Trace:
Error: Health check failed
    at checkHealth (http://localhost:5173/src/routes/root/guards/check-health.ts:39:9)
    at async Object.beforeLoad (http://localhost:5173/src/routes/__root.tsx?t=1778582110042:118:3)
    at async loadMatches (http://localhost:5173/@fs/Users/giuseppe/workspace/toolhive-react/.claude/worktrees/issue-2210-error-boundaries/node_modules/.vite/deps/index.dev-BVmqXupt.js?v=2939491d:1169:31)
    at async http://localhost:5173/@fs/Users/giuseppe/workspace/toolhive-react/.claude/worktrees/issue-2210-error-boundaries/node_modules/.vite/deps/index.dev-BVmqXupt.js?v=2939491d:2229:7

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a collapsible technical crash-report section to the generic error screen so users can view/copy actionable diagnostics (error message/stack + environment metadata) without overwhelming the default error UI.

Changes:

  • Introduces TechnicalDetails (collapsible UI + copy-to-clipboard crash report).
  • Threads an optional error through the error components to render technical details when available.
  • Widens the base error card layout and adds unit tests for the new technical details behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
renderer/src/common/components/error/technical-details.tsx New collapsible technical details UI, environment probing, and crash report copy logic.
renderer/src/common/components/error/index.tsx Passes the caught error through to GenericError.
renderer/src/common/components/error/generic-error.tsx Accepts optional error prop and renders TechnicalDetails when present.
renderer/src/common/components/error/base-error-screen.tsx Increases error card max width to better fit technical content.
renderer/src/common/components/error/tests/technical-details.test.tsx Adds tests for collapsed/expanded behavior, metadata, clipboard copy, and missing stack handling.

Comment thread renderer/src/common/components/error/technical-details.tsx Outdated
Comment thread renderer/src/common/components/error/technical-details.tsx Outdated
Comment thread renderer/src/common/components/error/technical-details.tsx
Comment thread renderer/src/common/components/error/__tests__/technical-details.test.tsx Outdated
Comment thread renderer/src/common/components/error/__tests__/technical-details.test.tsx Outdated
@peppescg peppescg force-pushed the worktree-issue-2210-error-boundaries branch 9 times, most recently from ab5478a to f551e59 Compare May 12, 2026 11:03
@peppescg
Copy link
Copy Markdown
Collaborator Author

@claude succint review

@claude
Copy link
Copy Markdown

claude Bot commented May 12, 2026

Claude finished @peppescg's task in 1m 5s —— View job


Succinct review

Overall this is a clean, well-tested addition. Most of the Copilot reviewer's flags are already addressed in the current code (aria-label is set, error.toString() is used everywhere, the missing-stack test uses container.querySelector('code')). A few real things to consider:

Substantive

  1. Timestamp drifts on every renderrenderer/src/common/components/error/technical-details.tsx:100 calls buildCrashReport in the render body, and buildCrashReport does new Date().toISOString() (utils.ts:22). Every re-render produces a new timestamp, so what the user copies is "time the component last rendered," not "time the error happened." Capture the timestamp once with useMemo/useRef, or move the build into the click handler. Fix this →

  2. setTimeout can fire after unmounttechnical-details.tsx:30 schedules setCopied(false) 2s later with no cleanup. If the user collapses the panel or navigates away within the window, React will warn (and in strict mode, this is a real leak). Clear the timeout in a useEffect cleanup, or guard with a ref. Fix this →

  3. Stack trace uses break-alltechnical-details.tsx:144. break-all will split mid-identifier, making file paths and symbols hard to read. break-words (or just whitespace-pre-wrap with horizontal scroll) is friendlier for stack traces.

Minor / nits

  1. useQuery for diagnostics has no staleTime/gcTime and no enabled guard. Fine in practice (component only mounts on error), but consider staleTime: Infinity so it doesn't refetch on focus.
  2. Inconsistent optional chaining: window.electronAPI?.platform (line 85) vs window.electronAPI.getToolhiveStatus() (line 61). Pick one.
  3. Test at __tests__/technical-details.test.tsx:73–78 only wraps the first badge assertion in waitFor; the rest rely on React having flushed by then. Works today, but wrapping all four in the same waitFor block is more robust.
  4. The Copilot suggestion about surfacing the User-Agent in the badges is reasonable but a UX call — current behavior (UA in copied report only) is defensible.

Looks good

  • Promise.allSettled for diagnostics so a broken IPC channel doesn't crash the error screen itself — nice defensive choice given this renders because something already failed.

  • Test coverage hits the meaningful states (collapsed, expanded, copy, copied-feedback, missing stack).

  • error.toString() includes the error name (e.g. TypeError:) — Copilot's point [Task]: Add hey-api client, automate fetching the openapi.json #2 is already addressed.

  • Read PR context

  • Read changed files

  • Post succinct review
    • branch: worktree-issue-2210-error-boundaries

@peppescg peppescg force-pushed the worktree-issue-2210-error-boundaries branch from f551e59 to 8b3565e Compare May 12, 2026 11:09
@peppescg peppescg requested a review from Copilot May 12, 2026 11:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Comment thread renderer/src/common/components/error/technical-details.tsx
Comment thread renderer/src/common/components/error/technical-details.tsx Outdated
Comment thread renderer/src/common/components/error/technical-details.tsx
Comment thread renderer/src/common/components/error/technical-details.tsx
Comment thread renderer/src/common/components/error/utils.ts Outdated
Enhance the GenericError screen with a collapsible "Show details" section
that surfaces the error message, stack trace, environment metadata
(desktop version, CLI version, platform), and a one-click "Copy error
report" button. This allows users to provide actionable bug reports
without needing to open browser dev tools.

Closes #2210

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@peppescg peppescg force-pushed the worktree-issue-2210-error-boundaries branch from 8b3565e to 1af1c61 Compare May 12, 2026 11:15
Copy link
Copy Markdown
Collaborator

@samuv samuv left a comment

Choose a reason for hiding this comment

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

looks good!

@peppescg peppescg merged commit ea652e1 into main May 12, 2026
17 checks passed
@peppescg peppescg deleted the worktree-issue-2210-error-boundaries branch May 12, 2026 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Story] - Implement Detailed Error Boundaries and Technical Crash Reports

3 participants