Skip to content

fix: guard clipboard copy actions#2292

Merged
kodiakhq[bot] merged 4 commits into
hyperdxio:mainfrom
AjTheSpidey:codex/clipboard-copy-fallback
May 19, 2026
Merged

fix: guard clipboard copy actions#2292
kodiakhq[bot] merged 4 commits into
hyperdxio:mainfrom
AjTheSpidey:codex/clipboard-copy-fallback

Conversation

@AjTheSpidey
Copy link
Copy Markdown

Summary

Fixes #2135.

Adds a shared copyTextToClipboard helper for the trace/event copy actions. It now:

  • uses navigator.clipboard.writeText when the browser permits it
  • falls back to a hidden textarea + document.execCommand('copy') path when the Clipboard API is unavailable, such as insecure/non-HTTPS contexts
  • shows a clear error notification if both copy methods fail instead of throwing Cannot read properties of undefined (reading 'writeText')

Screenshots or video

No screenshot included. This changes the failure/fallback path for browser clipboard permissions rather than the normal visible layout.

How to test on Vercel preview

Preview routes: /search

Steps:

  1. Open a search result row with event or trace attributes.
  2. Use the copy button for a row value or row JSON.
  3. Verify copying succeeds in secure contexts, or shows Could not access the clipboard. Check browser permissions or use HTTPS. if clipboard access is unavailable.

References

  • Linear Issue: N/A
  • Related PRs: N/A

Validation

  • yarn build:common-utils
  • yarn workspace @hyperdx/app jest --runTestsByPath src/utils/clipboard.test.ts src/components/DBRowJsonViewer.test.tsx --coverage=false (passes; existing Mantine notification act(...) warnings are printed)
  • yarn workspace @hyperdx/app exec tsc --noEmit
  • yarn workspace @hyperdx/app exec npx eslint --quiet src/utils/clipboard.ts src/utils/clipboard.test.ts src/components/DBRowJsonViewer.tsx src/components/DBTable/DBRowTableFieldWithPopover.tsx src/components/DBTable/DBRowTableRowButtons.tsx

Note: full yarn workspace @hyperdx/app lint is blocked locally on Windows by unrelated CRLF Prettier errors across existing files in the checkout.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 15, 2026

@AjTheSpidey is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 15, 2026

🦋 Changeset detected

Latest commit: 7ca4004

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026

PR Review

✅ No critical issues found.

The change cleanly extracts a shared copyTextToClipboard helper with a document.execCommand('copy') fallback for insecure contexts, restores prior selection/focus, and now surfaces a user-visible Mantine notification on failure instead of throwing. Call sites in DBRowJsonViewer.tsx, DBRowTableFieldWithPopover.tsx, and DBRowTableRowButtons.tsx all consistently consume the boolean result. Tests in packages/app/src/utils/__tests__/clipboard.test.ts cover the API path, the fallback path, both rejection branches, the selection-restore success/failure cases, and the dual-failure case.

Minor (non-blocking) observations:

  • DBRowTableRowButtons.copyRowData keeps a wrapping try/catch that now only really protects JSON.stringify (circular refs, etc.) — surfacing CLIPBOARD_ERROR_MESSAGE for a stringify failure is a slight messaging mismatch, but harmless.
  • copyTextToClipboard never rejects, so the await + if (!copied) pattern at all call sites is correct; consider documenting that contract with a brief JSDoc on the helper if you want to make that obvious to future readers.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026

Deep Review

✅ No critical (P0/P1) issues found. The Clipboard API fallback is correctly structured: insecure-context detection via optional chaining, rejection-to-textarea fallback, SSR guard, textarea cleanup in finally, and best-effort selection/focus restoration. The new utility carries 8 focused unit tests.

🟡 P2 — recommended

  • packages/app/src/components/DBTable/DBRowTableRowButtons.tsx:71 — The outer try { ... } catch { notifications.show({ color: 'red', message: CLIPBOARD_ERROR_MESSAGE }) } in copyRowData now wraps code whose only remaining throw paths are JSON.stringify / Object.entries.reduce failures (since copyTextToClipboard never throws), so a circular-ref serialization error would surface to the user as a misleading "Could not access the clipboard" toast; the sibling copyRowUrl was correctly stripped of its catch, making the asymmetry the smell.
    • Fix: Drop the outer try/catch in copyRowData and rely on the !copied branch for clipboard failures, or scope it tightly around JSON.stringify with a distinct non-clipboard message.
    • kieran-typescript, maintainability
  • packages/app/src/components/DBRowJsonViewer.tsx:232 — None of the three new red-notification branches in DBRowJsonViewer (HyperJsonMenu row-copy, handleCopyObject, Copy Value action), nor the ones in DBRowTableFieldWithPopover.copyFieldValue and DBRowTableRowButtons.copyRowData/copyRowUrl, are exercised by any component test — the existing DBRowJsonViewer.test.tsx clipboard suite mocks navigator.clipboard.writeText so copyTextToClipboard always resolves true, and the other two components have no test files at all.
    • Fix: Add at least one component test per call-site file that forces copyTextToClipboard to resolve false and asserts the red CLIPBOARD_ERROR_MESSAGE notification fires while the green success notification does not.
    • testing, kieran-typescript, maintainability, julik-frontend-races
  • packages/app/src/utils/clipboard.ts:9 — The inner try { await navigator.clipboard.writeText(text) } catch { return copyTextWithTextarea(text) } silently discards the underlying DOMException, removing the previous console.error('Failed to copy ...', error) breadcrumb across all four call sites; if clipboard failures are reported in the field there will be no diagnostic signal distinguishing permission denial, missing focus, secure-context blocking, or transient errors.
    • Fix: Log the caught error before falling back (e.g., console.warn('clipboard.writeText rejected, falling back', error)), or return a discriminated result so callers can branch on cause.
  • packages/app/src/utils/clipboard.ts:1 — The module exports CLIPBOARD_ERROR_MESSAGE and a Promise<boolean>, leaving six call sites to repeat the same if (!copied) { notifications.show({ color: 'red', message: CLIPBOARD_ERROR_MESSAGE }); return; } block; a future call site that forgets the return, omits the import, or drifts the color will silently desync from the others, defeating part of the centralization the helper was extracted to provide.
    • Fix: Introduce a thin copyTextToClipboardWithFeedback(text, { successMessage? }) wrapper that owns the error notification, and make CLIPBOARD_ERROR_MESSAGE a private detail of that wrapper.
    • kieran-typescript, maintainability
🔵 P3 nitpicks (5)
  • packages/app/src/utils/clipboard.ts:26 — The hidden textarea is anchored at top: '0'; left: '0' with a 1×1 px footprint, which can flash a sliver at the viewport origin on slow paints and, on iOS Safari, can still steal focus despite readonly.
    • Fix: Move the textarea off-screen with left: '-9999px' (or equivalent) and keep the size small.
    • julik-frontend-races, kieran-typescript
  • packages/app/src/utils/clipboard.ts:45textArea.select() blurs the previously focused element and fires its onBlur handler before the finally restores focus, so an open Mantine Autocomplete/Combobox/Select dropdown will close and any onBlur-commit input will commit during a fallback copy.
    • Fix: Acknowledge the side effect in a code comment and exercise an autocomplete-open + fallback-copy path in manual QA on non-HTTPS.
  • packages/app/src/utils/clipboard.ts:9writeText is retried via execCommand on every rejection, which conflates "API genuinely unavailable" (the only case the fix targets) with permission denial / unfocused-document / transient errors where the deprecated execCommand will usually also return false.
    • Fix: Only fall back when navigator.clipboard?.writeText is absent; on rejection, surface the failure directly so the diagnostic from the previous finding is meaningful.
  • packages/app/src/components/DBTable/DBRowTableFieldWithPopover.tsx:103 — The setTimeout(() => setIsCopied(false), 2000) after the new await is fire-and-forget — neither tracked in a ref nor cleared on unmount — so closing the popover within 2 s leaves an orphan timer that calls setIsCopied(false) on an unmounted component; the same pattern exists at DBRowTableRowButtons.tsx:70 and :96.
    • Fix: Store the timeout id in a ref, clear it at the start of each copy handler, and clear it in a useEffect cleanup; mirror the existing hoverDisableTimeoutRef pattern in the same component.
  • packages/app/src/utils/clipboard.ts:58 — The restored previousRange is best-effort: if a node inside its startContainer was detached by an onBlur handler that ran during textArea.select(), selection.addRange(previousRange) will throw and the selection silently won't be restored (the existing try/catch handles the throw cleanly).
    • Fix: Add a one-line comment noting the restoration is best-effort and intentional to prevent future "fix" PRs from removing the try/catch.

Reviewers (4): julik-frontend-races, testing, kieran-typescript, maintainability.

Testing gaps:

  • No component-level assertion that notifications.show({ color: 'red', message: CLIPBOARD_ERROR_MESSAGE }) fires when copyTextToClipboard resolves false at any of the six call sites.
  • clipboard.test.ts does not cover the SSR guard branch (typeof document === 'undefined') or active-element focus restoration (selection restoration is covered).
  • DBRowTableRowButtons.copyRowData outer catch is reachable only via JSON.stringify failure (e.g., circular ref) and has no test forcing that path.
  • A correctness-focused persona did not return findings this run (capacity error during dispatch); the four returned lenses + direct file reads covered the clipboard.ts logic, but a follow-up correctness sweep would harden confidence in the rejection-vs-undefined branch.

@AjTheSpidey AjTheSpidey force-pushed the codex/clipboard-copy-fallback branch from d7405ce to 87f079b Compare May 16, 2026 10:23
@AjTheSpidey
Copy link
Copy Markdown
Author

Pushed an update after rebasing on latest main.

I changed the clipboard helper so it only uses the textarea fallback when the Clipboard API is missing. If navigator.clipboard.writeText exists but rejects, it now returns false instead of trying the fallback after the async rejection. I also added coverage for that case and for execCommand throwing while making sure the temporary textarea is removed.

Checked locally:

  • jest src/utils/clipboard.test.ts --runInBand
  • focused eslint on the touched app files
  • tsc --noEmit --pretty false
  • git diff --check

@AjTheSpidey
Copy link
Copy Markdown
Author

Pushed another small follow-up for the deep-review notes.

This now tries the textarea fallback if navigator.clipboard.writeText rejects, moves the util test into src/utils/tests, adds coverage for selection restore / cleanup, keeps the row JSON copy path guarded, and updates the changeset quote style.

Checked locally:

  • jest src/utils/tests/clipboard.test.ts --runInBand
  • focused eslint on the touched app files
  • tsc --noEmit --pretty false
  • git diff --check

Copy link
Copy Markdown
Member

@brandon-pereira brandon-pereira left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks!

@vercel
Copy link
Copy Markdown

vercel Bot commented May 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment May 19, 2026 3:02pm

Request Review

@kodiakhq kodiakhq Bot merged commit b1004b7 into hyperdxio:main May 19, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Copying trace attributes/columns as JSON doesn't work

3 participants