-
Notifications
You must be signed in to change notification settings - Fork 953
fix(desktop): handle clipboard and prompt errors from Sentry #2700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a306fa9
6e0a51b
6538fb6
6e4a9d7
3514fd4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,27 @@ | ||||||||||||||||||||||||
| import { useCallback, useState } from "react"; | ||||||||||||||||||||||||
| import { electronTrpc } from "renderer/lib/electron-trpc"; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Copy text to clipboard via Electron's native clipboard API (IPC). | ||||||||||||||||||||||||
| * | ||||||||||||||||||||||||
| * Unlike `navigator.clipboard.writeText`, this works regardless of | ||||||||||||||||||||||||
| * document focus — no DOMException when a terminal or webview has focus. | ||||||||||||||||||||||||
| * | ||||||||||||||||||||||||
| * Returns `{ copyToClipboard, copied }` where `copied` is true for | ||||||||||||||||||||||||
| * `timeout` ms after a successful write. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| export function useCopyToClipboard(timeout = 2000) { | ||||||||||||||||||||||||
| const { mutateAsync } = electronTrpc.external.copyPath.useMutation(); | ||||||||||||||||||||||||
| const [copied, setCopied] = useState(false); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const copyToClipboard = useCallback( | ||||||||||||||||||||||||
| async (text: string) => { | ||||||||||||||||||||||||
| await mutateAsync(text); | ||||||||||||||||||||||||
| setCopied(true); | ||||||||||||||||||||||||
| setTimeout(() => setCopied(false), timeout); | ||||||||||||||||||||||||
|
Comment on lines
+19
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Handle (Based on your team's feedback about awaiting/catching rejecting async calls.) Prompt for AI agents
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Consecutive copy actions race because previous reset timers are not cleared, so Prompt for AI agents |
||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| [mutateAsync, timeout], | ||||||||||||||||||||||||
|
Comment on lines
+17
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat -n apps/desktop/src/renderer/hooks/useCopyToClipboard.tsRepository: superset-sh/superset Length of output: 1102 🏁 Script executed: # Search for usages of useCopyToClipboard hook in the codebase
rg "useCopyToClipboard" --type ts --type tsx -B 2 -A 2Repository: superset-sh/superset Length of output: 91 🏁 Script executed: # Check if there are any test files for this hook
fd "useCopyToClipboard" --type fRepository: superset-sh/superset Length of output: 117 🏁 Script executed: # Search for usages of useCopyToClipboard hook in the codebase (correcting the type)
rg "useCopyToClipboard" --type ts -B 2 -A 2Repository: superset-sh/superset Length of output: 41235 🏁 Script executed: # Check the trpc mutation to understand what mutateAsync does
rg "copyPath" --type ts -B 3 -A 3Repository: superset-sh/superset Length of output: 42055 🏁 Script executed: # Check how callbacks that return promises are typically handled in this codebase
rg "useCallback.*async" --type ts -A 5 | head -50Repository: superset-sh/superset Length of output: 4807 🏁 Script executed: # Check a few specific call sites to confirm none are awaiting
rg "copyToClipboard\(" apps/desktop/src/renderer -A 1 | head -40Repository: superset-sh/superset Length of output: 4450 🏁 Script executed: # Look for any error boundaries or error handlers around these calls
rg -B 5 "copyToClipboard\(" apps/desktop/src/renderer/hooks/useCopyToClipboard.ts -A 5Repository: superset-sh/superset Length of output: 46 Add error handling and timer cleanup to useCopyToClipboard hook. The hook currently lacks error handling for Proposed fix-import { useCallback, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
import { electronTrpc } from "renderer/lib/electron-trpc";
@@
export function useCopyToClipboard(timeout = 2000) {
const { mutateAsync } = electronTrpc.external.copyPath.useMutation();
const [copied, setCopied] = useState(false);
+ const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+ useEffect(
+ () => () => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ },
+ [],
+ );
const copyToClipboard = useCallback(
async (text: string) => {
- await mutateAsync(text);
- setCopied(true);
- setTimeout(() => setCopied(false), timeout);
+ try {
+ await mutateAsync(text);
+ setCopied(true);
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => setCopied(false), timeout);
+ return true;
+ } catch {
+ return false;
+ }
},
[mutateAsync, timeout],
);🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return { copyToClipboard, copied }; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t clear
oauthErrorbefore copy success is known.setOauthError(null)runs regardless of copy outcome. This can suppress a useful error state when copying fails.Proposed fix
🤖 Prompt for AI Agents