fix: patch memory leaks in event hooks and strengthen type guard

Fix three bugs found during code review:

1. useTauriEvent/useTauriEvents: If the component unmounts before the
   async listen() promise resolves, the unlisten function was lost,
   leaking the event subscription. Added a cancelled flag to call
   unlisten immediately when the promise resolves after cleanup.

2. useTauriEvents: The handlers object was used directly as a useEffect
   dependency, causing re-subscription on every render when callers
   pass an inline object literal. Replaced with a useRef for handler
   stability and a derived eventNames string as the dependency.

3. isMcError type guard: Only checked property existence via 'in'
   operator, not property types. An object with wrong-typed properties
   (e.g. code: 42) would pass the guard. Now validates that code and
   message are strings and recoverable is boolean.

4. AppShell global shortcut listener: Same race condition as (1), plus
   missing .catch() on the listen promise could produce unhandled
   rejections.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
teernisse
2026-02-26 10:12:20 -05:00
parent 378a173084
commit 29b44f1b4c
3 changed files with 69 additions and 44 deletions

View File

@@ -38,14 +38,30 @@ export function AppShell(): React.ReactElement {
// Listen for global shortcut events from the Rust backend // Listen for global shortcut events from the Rust backend
useEffect(() => { useEffect(() => {
const unlisten = listen<string>("global-shortcut-triggered", (event) => { let cancelled = false;
let unlisten: (() => void) | undefined;
listen<string>("global-shortcut-triggered", (event) => {
if (event.payload === "quick-capture") { if (event.payload === "quick-capture") {
useCaptureStore.getState().open(); useCaptureStore.getState().open();
} }
}); })
.then((fn) => {
if (cancelled) {
fn();
} else {
unlisten = fn;
}
})
.catch((err: unknown) => {
console.warn("[AppShell] Failed to subscribe to global shortcut:", err);
});
return () => { return () => {
unlisten.then((fn) => fn()); cancelled = true;
if (unlisten) {
unlisten();
}
}; };
}, []); }, []);

View File

@@ -5,7 +5,7 @@
* Used for file watcher triggers, sync status, error notifications. * Used for file watcher triggers, sync status, error notifications.
*/ */
import { useEffect, useCallback } from "react"; import { useEffect, useRef } from "react";
import { listen, type UnlistenFn } from "@tauri-apps/api/event"; import { listen, type UnlistenFn } from "@tauri-apps/api/event";
/** Event types emitted by the Rust backend */ /** Event types emitted by the Rust backend */
@@ -26,79 +26,84 @@ export interface TauriEventPayloads {
/** /**
* Subscribe to a Tauri event with automatic cleanup. * Subscribe to a Tauri event with automatic cleanup.
* *
* @param eventName The event to listen for * Uses a ref for the handler so that changing the callback does not
* @param handler Callback when event is received * cause re-subscription. Handles the race where the component unmounts
* * before the async listen() promise resolves.
* @example
* ```tsx
* useTauriEvent("lore-data-changed", () => {
* refetch();
* });
* ```
*/ */
export function useTauriEvent<T extends TauriEventType>( export function useTauriEvent<T extends TauriEventType>(
eventName: T, eventName: T,
handler: (payload: TauriEventPayloads[T]) => void handler: (payload: TauriEventPayloads[T]) => void
): void { ): void {
// Memoize handler to avoid re-subscribing on every render const handlerRef = useRef(handler);
const stableHandler = useCallback(handler, [handler]); handlerRef.current = handler;
useEffect(() => { useEffect(() => {
let cancelled = false;
let unlisten: UnlistenFn | undefined; let unlisten: UnlistenFn | undefined;
// Subscribe to the event
listen<TauriEventPayloads[T]>(eventName, (event) => { listen<TauriEventPayloads[T]>(eventName, (event) => {
stableHandler(event.payload); handlerRef.current(event.payload);
}).then((fn) => { }).then((fn) => {
unlisten = fn; if (cancelled) {
fn();
} else {
unlisten = fn;
}
}); });
// Cleanup on unmount
return () => { return () => {
cancelled = true;
if (unlisten) { if (unlisten) {
unlisten(); unlisten();
} }
}; };
}, [eventName, stableHandler]); }, [eventName]);
} }
/** /**
* Subscribe to multiple Tauri events. * Subscribe to multiple Tauri events.
* *
* @param handlers Map of event names to handlers * Uses a ref for the handlers object so that identity changes do not
* * cause re-subscription. Only re-subscribes when the set of event
* @example * names changes.
* ```tsx
* useTauriEvents({
* "lore-data-changed": () => refetch(),
* "sync-status": (status) => setStatus(status),
* });
* ```
*/ */
export function useTauriEvents( export function useTauriEvents(
handlers: Partial<{ handlers: Partial<{
[K in TauriEventType]: (payload: TauriEventPayloads[K]) => void; [K in TauriEventType]: (payload: TauriEventPayloads[K]) => void;
}> }>
): void { ): void {
const handlersRef = useRef(handlers);
handlersRef.current = handlers;
const eventNames = Object.keys(handlers).sort().join(",");
useEffect(() => { useEffect(() => {
let cancelled = false;
const unlisteners: UnlistenFn[] = []; const unlisteners: UnlistenFn[] = [];
// Subscribe to each event const entries = eventNames.split(",").filter(Boolean);
for (const [eventName, handler] of Object.entries(handlers)) { for (const eventName of entries) {
if (handler) { listen(eventName, (event) => {
listen(eventName, (event) => { const currentHandler = handlersRef.current[eventName as TauriEventType];
(handler as (p: unknown) => void)(event.payload); if (currentHandler) {
}).then((unlisten) => { // Safe cast: handler was registered for this event name, but TS
// cannot narrow the key-value type relationship at runtime
(currentHandler as (p: unknown) => void)(event.payload);
}
}).then((unlisten) => {
if (cancelled) {
unlisten();
} else {
unlisteners.push(unlisten); unlisteners.push(unlisten);
}); }
} });
} }
// Cleanup all subscriptions on unmount
return () => { return () => {
cancelled = true;
for (const unlisten of unlisteners) { for (const unlisten of unlisteners) {
unlisten(); unlisten();
} }
}; };
}, [handlers]); }, [eventNames]);
} }

View File

@@ -63,12 +63,12 @@ export interface McError {
/** Type guard to check if an error is a structured McError */ /** Type guard to check if an error is a structured McError */
export function isMcError(err: unknown): err is McError { export function isMcError(err: unknown): err is McError {
if (typeof err !== "object" || err === null) return false;
const obj = err as Record<string, unknown>;
return ( return (
typeof err === "object" && typeof obj.code === "string" &&
err !== null && typeof obj.message === "string" &&
"code" in err && typeof obj.recoverable === "boolean"
"message" in err &&
"recoverable" in err
); );
} }
@@ -156,6 +156,10 @@ export function computeStaleness(updatedAt: string | null): Staleness {
if (!updatedAt) return "normal"; if (!updatedAt) return "normal";
const ageMs = Date.now() - new Date(updatedAt).getTime(); const ageMs = Date.now() - new Date(updatedAt).getTime();
// Guard against invalid date strings (NaN propagates through arithmetic)
if (Number.isNaN(ageMs)) return "normal";
const ageDays = ageMs / (1000 * 60 * 60 * 24); const ageDays = ageMs / (1000 * 60 * 60 * 24);
if (ageDays < 1) return "fresh"; if (ageDays < 1) return "fresh";