diff --git a/desktop/src/app/useHuddlePresentation.ts b/desktop/src/app/useHuddlePresentation.ts index a82916d044..37fdcf77ab 100644 --- a/desktop/src/app/useHuddlePresentation.ts +++ b/desktop/src/app/useHuddlePresentation.ts @@ -14,6 +14,7 @@ import { channelMessagesKey, channelWindowKey, } from "@/features/messages/lib/messageQueryKeys"; +import { safeUnlisten } from "@/shared/lib/safeUnlisten"; type HuddleTranscriptRouteState = { phase: @@ -80,13 +81,14 @@ export function useHuddlePresentation() { void listen("huddle-state-changed", (event) => syncRoute(event.payload), ).then((cleanup) => { - if (cancelled) cleanup(); + if (cancelled) safeUnlisten(cleanup); else unlisten = cleanup; }); return () => { cancelled = true; - unlisten?.(); + safeUnlisten(unlisten); + unlisten = null; }; }, [huddleRoomChannelId, isHuddleRoom]); @@ -368,12 +370,13 @@ export function useHuddlePresentation() { console.error("Failed to open huddle in the main app:", error); }); }).then((cleanup) => { - if (cancelled) cleanup(); + if (cancelled) safeUnlisten(cleanup); else unlisten = cleanup; }); return () => { cancelled = true; - unlisten?.(); + safeUnlisten(unlisten); + unlisten = null; }; }, [isHuddleRoom, showHuddleInMainApp]); @@ -429,12 +432,13 @@ export function useHuddlePresentation() { void queryClient.invalidateQueries({ queryKey: channelsQueryKey }); } }).then((cleanup) => { - if (cancelled) cleanup(); + if (cancelled) safeUnlisten(cleanup); else unlisten = cleanup; }); return () => { cancelled = true; - unlisten?.(); + safeUnlisten(unlisten); + unlisten = null; }; }, [ hideHuddleChannel, diff --git a/desktop/src/features/huddle/components/HuddleBar.tsx b/desktop/src/features/huddle/components/HuddleBar.tsx index d5a0423cf7..a5f56da175 100644 --- a/desktop/src/features/huddle/components/HuddleBar.tsx +++ b/desktop/src/features/huddle/components/HuddleBar.tsx @@ -22,6 +22,7 @@ import type { RelayEvent } from "@/shared/api/types"; import { KIND_HUDDLE_REACTION } from "@/shared/constants/kinds"; import { cn } from "@/shared/lib/cn"; import { rewriteRelayUrl } from "@/shared/lib/mediaUrl"; +import { safeUnlisten } from "@/shared/lib/safeUnlisten"; import { useDocumentVisible } from "@/shared/lib/useDocumentVisible"; import { Button } from "@/shared/ui/button"; import { useEmojiBurst } from "@/shared/ui/EmojiBurstProvider"; @@ -248,7 +249,7 @@ export function HuddleBar({ applyIncomingState(event.payload); } }).then((fn) => { - if (cancelled) fn(); + if (cancelled) safeUnlisten(fn); else unlisten = fn; }); @@ -263,7 +264,8 @@ export function HuddleBar({ return () => { cancelled = true; - unlisten?.(); + safeUnlisten(unlisten); + unlisten = null; if (id !== null) window.clearInterval(id); }; }, [applyIncomingState, documentVisible]); diff --git a/desktop/src/features/huddle/components/HuddleIndicator.tsx b/desktop/src/features/huddle/components/HuddleIndicator.tsx index 6f11d84731..e3878ded10 100644 --- a/desktop/src/features/huddle/components/HuddleIndicator.tsx +++ b/desktop/src/features/huddle/components/HuddleIndicator.tsx @@ -7,6 +7,7 @@ import { useQueryClient } from "@tanstack/react-query"; import { relayClient } from "@/shared/api/relayClient"; import type { RelayEvent } from "@/shared/api/types"; import { cn } from "@/shared/lib/cn"; +import { safeUnlisten } from "@/shared/lib/safeUnlisten"; import { HUDDLE_SHORTCUT_EVENT, type HuddleShortcutDetail, @@ -206,13 +207,14 @@ export function HuddleIndicator({ setActiveHuddle(null); } }).then((fn) => { - if (cancelled) fn(); + if (cancelled) safeUnlisten(fn); else unlisten = fn; }); return () => { cancelled = true; - unlisten?.(); + safeUnlisten(unlisten); + unlisten = null; }; }, []); diff --git a/desktop/src/features/huddle/components/HuddleProfileControl.tsx b/desktop/src/features/huddle/components/HuddleProfileControl.tsx index dbf3d5f6b8..9cd583e50e 100644 --- a/desktop/src/features/huddle/components/HuddleProfileControl.tsx +++ b/desktop/src/features/huddle/components/HuddleProfileControl.tsx @@ -4,6 +4,7 @@ import { Headphones } from "lucide-react"; import * as React from "react"; import type { Channel } from "@/shared/api/types"; +import { safeUnlisten } from "@/shared/lib/safeUnlisten"; import { Button } from "@/shared/ui/button"; import { useHuddle, useHuddleLevels } from "../HuddleContext"; import { MicControls } from "./MicControls"; @@ -69,13 +70,14 @@ export function HuddleProfileControl({ void listen("huddle-state-changed", (event) => { if (!disposed) setState(event.payload); }).then((cleanup) => { - if (disposed) cleanup(); + if (disposed) safeUnlisten(cleanup); else unlisten = cleanup; }); return () => { disposed = true; - unlisten?.(); + safeUnlisten(unlisten); + unlisten = null; }; }, []); diff --git a/desktop/src/features/huddle/components/HuddleRoomHeader.tsx b/desktop/src/features/huddle/components/HuddleRoomHeader.tsx index 0cf9c735da..f45875ea06 100644 --- a/desktop/src/features/huddle/components/HuddleRoomHeader.tsx +++ b/desktop/src/features/huddle/components/HuddleRoomHeader.tsx @@ -4,6 +4,7 @@ import * as React from "react"; import { useProfileQuery, useSelfProfileCache } from "@/features/profile/hooks"; import { useIdentityQuery } from "@/shared/api/hooks"; +import { safeUnlisten } from "@/shared/lib/safeUnlisten"; import { useHuddle, useHuddleLevels } from "../HuddleContext"; import { useHuddleParticipantRoster } from "../hooks/useHuddleParticipantRoster"; import type { HuddleAgentVoiceSettings } from "./AgentVoiceMenu"; @@ -92,13 +93,14 @@ export function HuddleRoomHeader() { void listen("huddle-state-changed", (event) => { if (!disposed) setState(event.payload); }).then((cleanup) => { - if (disposed) cleanup(); + if (disposed) safeUnlisten(cleanup); else unlisten = cleanup; }); return () => { disposed = true; - unlisten?.(); + safeUnlisten(unlisten); + unlisten = null; }; }, []); diff --git a/desktop/src/features/huddle/lib/useTtsSubscription.ts b/desktop/src/features/huddle/lib/useTtsSubscription.ts index 1744cd7c1b..f6235763b1 100644 --- a/desktop/src/features/huddle/lib/useTtsSubscription.ts +++ b/desktop/src/features/huddle/lib/useTtsSubscription.ts @@ -2,6 +2,7 @@ import { invoke } from "@tauri-apps/api/core"; import { listen } from "@tauri-apps/api/event"; import * as React from "react"; +import { safeUnlisten } from "@/shared/lib/safeUnlisten"; import { isDocumentVisible, subscribeDocumentVisibility, @@ -253,7 +254,7 @@ export function useTtsSubscription( }) .then((unlisten) => { if (disposed) { - unlisten(); + safeUnlisten(unlisten); return; } unlistenHuddleState = unlisten; @@ -324,7 +325,8 @@ export function useTtsSubscription( disposed = true; speakInOrder.setEnabled(false); cleanup?.(); - unlistenHuddleState?.(); + safeUnlisten(unlistenHuddleState); + unlistenHuddleState = null; unsubscribeDocumentVisibility(); if (agentRefreshId !== null) window.clearInterval(agentRefreshId); if (agentVerificationRetryId !== null) { diff --git a/desktop/src/features/messages/lib/mountedEditorView.test.mjs b/desktop/src/features/messages/lib/mountedEditorView.test.mjs new file mode 100644 index 0000000000..39b0da8b97 --- /dev/null +++ b/desktop/src/features/messages/lib/mountedEditorView.test.mjs @@ -0,0 +1,51 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { getMountedView } from "./mountedEditorView.ts"; + +// Mirrors tiptap v3's unmounted-view proxy: it stubs a few keys and throws for +// everything else, so reading `dom` is what blows up in production. +function unmountedViewProxy() { + const stubs = { state: {}, composing: false, editable: true }; + return new Proxy(stubs, { + get: (target, key) => { + if (key in target) return Reflect.get(target, key); + throw new Error( + `[tiptap error]: The editor view is not available. Cannot access view['${String(key)}'].`, + ); + }, + }); +} + +test("returns the view once it is mounted", () => { + const view = { dom: { nodeType: 1 } }; + const editor = { isDestroyed: false, view }; + + assert.equal(getMountedView(editor), view); +}); + +test("returns null instead of throwing while the view is unmounted", () => { + const editor = { isDestroyed: false, view: unmountedViewProxy() }; + + assert.equal(getMountedView(editor), null); +}); + +test("returns null for a destroyed editor without touching the view", () => { + let viewReads = 0; + const editor = { + isDestroyed: true, + get view() { + viewReads += 1; + throw new Error("view read on a destroyed editor"); + }, + }; + + assert.equal(getMountedView(editor), null); + assert.equal(viewReads, 0); +}); + +test("returns null when the view has no dom element", () => { + const editor = { isDestroyed: false, view: { dom: null } }; + + assert.equal(getMountedView(editor), null); +}); diff --git a/desktop/src/features/messages/lib/mountedEditorView.ts b/desktop/src/features/messages/lib/mountedEditorView.ts new file mode 100644 index 0000000000..c86ec5f741 --- /dev/null +++ b/desktop/src/features/messages/lib/mountedEditorView.ts @@ -0,0 +1,21 @@ +import type { EditorView } from "@tiptap/pm/view"; +import type { Editor } from "@tiptap/react"; + +/** + * Resolve an editor's ProseMirror view, or `null` when it is not mounted. + * + * A tiptap v3 `Editor` outlives its view: before `EditorContent` mounts it and + * after the subtree unmounts, `editor.view` is a proxy that *throws* for every + * key it does not stub — including `dom`, `domAtPos`, and `coordsAtPos`. A + * non-null `editor` therefore does not imply a usable view, so any code that + * reaches past the editor into the view must go through this guard. + */ +export function getMountedView(editor: Editor): EditorView | null { + if (editor.isDestroyed) return null; + try { + return editor.view.dom ? editor.view : null; + } catch { + // Throwing proxy — the view is detached right now. + return null; + } +} diff --git a/desktop/src/features/messages/ui/SelectionFormattingTray.tsx b/desktop/src/features/messages/ui/SelectionFormattingTray.tsx index 84934d1826..ca1f99a5a0 100644 --- a/desktop/src/features/messages/ui/SelectionFormattingTray.tsx +++ b/desktop/src/features/messages/ui/SelectionFormattingTray.tsx @@ -1,8 +1,10 @@ import * as React from "react"; import { createPortal } from "react-dom"; +import type { EditorView } from "@tiptap/pm/view"; import type { Editor } from "@tiptap/react"; import { cn } from "@/shared/lib/cn"; +import { getMountedView } from "../lib/mountedEditorView"; import { FormattingToolbar } from "./FormattingToolbar"; type SelectionFormattingTrayProps = { @@ -25,13 +27,13 @@ function clamp(value: number, min: number, max: number): number { return Math.min(Math.max(value, min), max); } -function getSelectionRect(editor: Editor): DOMRect | null { +function getSelectionRect(editor: Editor, view: EditorView): DOMRect | null { const { from, to } = editor.state.selection; try { const range = document.createRange(); - const start = editor.view.domAtPos(from); - const end = editor.view.domAtPos(to); + const start = view.domAtPos(from); + const end = view.domAtPos(to); range.setStart(start.node, start.offset); range.setEnd(end.node, end.offset); @@ -46,19 +48,25 @@ function getSelectionRect(editor: Editor): DOMRect | null { // Fall back to the caret coordinates below. } - const startCoords = editor.view.coordsAtPos(from); - const endCoords = editor.view.coordsAtPos(to); - const left = Math.min(startCoords.left, endCoords.left); - const right = Math.max(startCoords.right, endCoords.right); - const top = Math.min(startCoords.top, endCoords.top); - const bottom = Math.max(startCoords.bottom, endCoords.bottom); + try { + const startCoords = view.coordsAtPos(from); + const endCoords = view.coordsAtPos(to); + const left = Math.min(startCoords.left, endCoords.left); + const right = Math.max(startCoords.right, endCoords.right); + const top = Math.min(startCoords.top, endCoords.top); + const bottom = Math.max(startCoords.bottom, endCoords.bottom); - if (right <= left && bottom <= top) return null; - return new DOMRect(left, top, Math.max(1, right - left), bottom - top); + if (right <= left && bottom <= top) return null; + return new DOMRect(left, top, Math.max(1, right - left), bottom - top); + } catch { + // The view detached mid-measurement; leave the tray hidden. + return null; + } } function getTrayPosition( editor: Editor, + view: EditorView, trayWidth: number, ): TrayPosition | null { const { selection } = editor.state; @@ -72,7 +80,7 @@ function getTrayPosition( ); if (selectedText.trim().length === 0) return null; - const rect = getSelectionRect(editor); + const rect = getSelectionRect(editor, view); if (!rect) return null; const viewportWidth = window.innerWidth; @@ -118,6 +126,9 @@ export function SelectionFormattingTray({ const suppressRightClickUpdatesRef = React.useRef(false); const trayRef = React.useRef(null); const [trayWidth, setTrayWidth] = React.useState(0); + // The view attaches and detaches independently of the editor, so track it as + // state rather than reading `editor.view` at wiring time. + const [mountedView, setMountedView] = React.useState(null); const cancelScheduledUpdate = React.useCallback(() => { if (rafRef.current === null) return; @@ -136,8 +147,12 @@ export function SelectionFormattingTray({ setPosition(null); return; } - setPosition(getTrayPosition(editor, trayWidth)); - }, [disabled, editor, trayWidth]); + if (!mountedView) { + setPosition(null); + return; + } + setPosition(getTrayPosition(editor, mountedView, trayWidth)); + }, [disabled, editor, mountedView, trayWidth]); const scheduleUpdate = React.useCallback(() => { if (suppressRightClickUpdatesRef.current) { @@ -152,16 +167,36 @@ export function SelectionFormattingTray({ }); }, [cancelScheduledUpdate, updatePosition]); + // React can reconnect these effects while the composer subtree is hidden, at + // which point `EditorContent` has already torn the view down. Follow tiptap's + // mount/unmount events instead of reading the throwing view proxy on demand. + React.useEffect(() => { + if (!editor) { + setMountedView(null); + return; + } + + const syncView = () => setMountedView(getMountedView(editor)); + syncView(); + editor.on("mount", syncView); + editor.on("unmount", syncView); + + return () => { + editor.off("mount", syncView); + editor.off("unmount", syncView); + }; + }, [editor]); + React.useEffect(() => { suppressRightClickUpdatesRef.current = false; - if (!editor) { + if (!editor || !mountedView) { cancelScheduledUpdate(); setPosition(null); return; } - const editorDom = editor.view.dom; + const editorDom = mountedView.dom; const hide = () => setPosition(null); const handleContextMenu = () => { suppressRightClickUpdatesRef.current = true; @@ -199,7 +234,7 @@ export function SelectionFormattingTray({ window.removeEventListener("resize", scheduleUpdate); window.removeEventListener("scroll", scheduleUpdate, true); }; - }, [cancelScheduledUpdate, editor, scheduleUpdate]); + }, [cancelScheduledUpdate, editor, mountedView, scheduleUpdate]); React.useLayoutEffect(() => { if (!position || !trayRef.current) return; diff --git a/desktop/src/shared/lib/safeUnlisten.test.mjs b/desktop/src/shared/lib/safeUnlisten.test.mjs new file mode 100644 index 0000000000..a1a03a2800 --- /dev/null +++ b/desktop/src/shared/lib/safeUnlisten.test.mjs @@ -0,0 +1,93 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { safeUnlisten } from "./safeUnlisten.ts"; + +// Retries are driven through an injected host, so the tests never wait on real +// timers; handlers run immediately and we flush the microtask queue instead. +function immediateHost() { + const delays = []; + return { + delays, + host: { + setTimeout: (handler, ms) => { + delays.push(ms); + handler(); + }, + }, + }; +} + +const flush = () => new Promise((resolve) => setImmediate(resolve)); + +test("calls the unlisten function once when it succeeds", async () => { + const { host, delays } = immediateHost(); + let calls = 0; + + safeUnlisten(() => { + calls += 1; + }, host); + await flush(); + + assert.equal(calls, 1); + assert.deepEqual(delays, [], "no retry was scheduled"); +}); + +test("retries past the listener-registration race and stops on success", async () => { + const { host, delays } = immediateHost(); + let calls = 0; + + safeUnlisten(() => { + calls += 1; + // Tauri's injected unregisterListener throws until the backend eval that + // publishes this listener's id has landed in the webview. + if (calls < 3) throw new TypeError("listeners[eventId] is undefined"); + }, host); + await flush(); + + assert.equal(calls, 3, "keeps trying until the listener is actually removed"); + assert.deepEqual(delays, [0, 16], "backs off between attempts"); +}); + +test("retries a rejected async unlisten", async () => { + const { host } = immediateHost(); + let calls = 0; + + safeUnlisten(async () => { + calls += 1; + if (calls < 2) throw new Error("not registered yet"); + }, host); + await flush(); + + assert.equal(calls, 2); +}); + +test("gives up after exhausting retries without rejecting", async () => { + const { host, delays } = immediateHost(); + const originalDebug = console.debug; + const debugCalls = []; + console.debug = (...args) => debugCalls.push(args); + let calls = 0; + + try { + safeUnlisten(() => { + calls += 1; + throw new Error("always fails"); + }, host); + await flush(); + } finally { + console.debug = originalDebug; + } + + assert.equal(calls, 5, "one initial attempt plus every configured retry"); + assert.deepEqual(delays, [0, 16, 64, 256]); + assert.equal(debugCalls.length, 1, "logs once instead of throwing"); +}); + +test("ignores a missing unlisten function", async () => { + const { host } = immediateHost(); + + safeUnlisten(null, host); + safeUnlisten(undefined, host); + await flush(); +}); diff --git a/desktop/src/shared/lib/safeUnlisten.ts b/desktop/src/shared/lib/safeUnlisten.ts new file mode 100644 index 0000000000..6b964e7d4c --- /dev/null +++ b/desktop/src/shared/lib/safeUnlisten.ts @@ -0,0 +1,53 @@ +import type { UnlistenFn } from "@tauri-apps/api/event"; + +/** + * `listen()` resolves with its event id as soon as the backend command + * returns, but the backend publishes that id into the webview through a + * separate `eval`. An unlisten issued before that eval lands throws from + * Tauri's injected `unregisterListener` (`listeners[eventId]` is still + * undefined) *before* it invokes `plugin:event|unlisten`, so the backend + * listener survives and keeps calling the handler. Retrying past the gap is + * what actually removes it; swallowing the error would leak the listener. + */ +const RETRY_DELAYS_MS = [0, 16, 64, 256] as const; + +export type UnlistenRetryHost = { + setTimeout: (handler: () => void, ms: number) => void; +}; + +const defaultHost: UnlistenRetryHost = { + setTimeout: (handler, ms) => { + window.setTimeout(handler, ms); + }, +}; + +async function attemptUnlisten( + unlisten: UnlistenFn, + attempt: number, + host: UnlistenRetryHost, +): Promise { + try { + await unlisten(); + } catch (error) { + const delayMs = RETRY_DELAYS_MS[attempt]; + if (delayMs === undefined) { + console.debug("[tauri] gave up removing an event listener:", error); + return; + } + await new Promise((resolve) => host.setTimeout(resolve, delayMs)); + await attemptUnlisten(unlisten, attempt + 1, host); + } +} + +/** + * Removes a Tauri event listener without letting the teardown race surface as + * an unhandled rejection. Safe to call from an effect cleanup and safe to call + * more than once for the same listener. + */ +export function safeUnlisten( + unlisten: UnlistenFn | null | undefined, + host: UnlistenRetryHost = defaultHost, +): void { + if (!unlisten) return; + void attemptUnlisten(unlisten, 0, host); +}