fix(openUrl): fall back to window.open when CEF IPC handle not ready (#1472) (#1491)

app/src/components/settings/SettingsHome.tsx

@@ -209,7 +209,7 @@ const SettingsHome = () => {
             </svg>
           ),
           onClick: () => {
-            void openUrl(BILLING_DASHBOARD_URL);
+            openUrl(BILLING_DASHBOARD_URL).catch(() => {});
           },
         },
         {

app/src/components/settings/__tests__/SettingsHome.test.tsx

@@ -32,7 +32,7 @@ vi.mock('../../../store', () => ({ persistor: { purge: vi.fn().mockResolvedValue
 
 vi.mock('../../../utils/links', () => ({ BILLING_DASHBOARD_URL: 'https://billing.example.com' }));
 
-vi.mock('../../../utils/openUrl', () => ({ openUrl: vi.fn() }));
+vi.mock('../../../utils/openUrl', () => ({ openUrl: vi.fn().mockResolvedValue(undefined) }));
 
 vi.mock('../../../utils/tauriCommands', () => ({
   resetOpenHumanDataAndRestartCore: vi.fn().mockResolvedValue(undefined),

app/src/utils/openUrl.test.ts

@@ -1,17 +1,24 @@
 /**
  * Unit tests for `openUrl`. The Tauri path is exercised in callers'
- * integration tests; here we focus on the browser fallback so the
- * non-Tauri branch (used by dev preview builds) doesn't regress.
+ * integration tests; here we focus on the browser fallback and the
+ * CEF-IPC-not-ready recovery so the non-Tauri branch (used by dev
+ * preview builds) and the CEF gap window (#1472 / REACT-T/S/R) do
+ * not regress.
  */
 import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from 'vitest';
 
 const isTauriMock = vi.fn();
 const tauriOpenUrlMock = vi.fn();
+const addBreadcrumbMock = vi.fn();
 
 vi.mock('@tauri-apps/api/core', () => ({ isTauri: () => isTauriMock() }));
 
 vi.mock('@tauri-apps/plugin-opener', () => ({ openUrl: (url: string) => tauriOpenUrlMock(url) }));
 
+vi.mock('@sentry/react', () => ({
+  addBreadcrumb: (...args: unknown[]) => addBreadcrumbMock(...args),
+}));
+
 describe('openUrl', () => {
   let originalWindowOpen: typeof window.open;
   let windowOpenMock: Mock;
@@ -35,9 +42,9 @@ describe('openUrl', () => {
     await openUrl('https://example.com/page');
 
     expect(tauriOpenUrlMock).toHaveBeenCalledWith('https://example.com/page');
-    // Browser fallback must NOT fire under Tauri — it would spawn a
-    // new webview window with no useful behaviour for custom schemes.
+    // Browser fallback must NOT fire when the Tauri call succeeded.
     expect(windowOpenMock).not.toHaveBeenCalled();
+    expect(addBreadcrumbMock).not.toHaveBeenCalled();
   });
 
   it('falls back to window.open in a browser context (non-Tauri)', async () => {
@@ -52,17 +59,67 @@ describe('openUrl', () => {
       'noopener,noreferrer'
     );
     expect(tauriOpenUrlMock).not.toHaveBeenCalled();
+    expect(addBreadcrumbMock).not.toHaveBeenCalled();
   });
 
-  it('propagates Tauri opener errors to the caller (no silent fallback)', async () => {
-    // Regression guard: the previous implementation swallowed the
-    // error and called window.open, which spawned a useless webview
-    // window for unhandled custom schemes (`obsidian://...`).
+  it('propagates Tauri opener errors for non-http schemes (no silent fallback)', async () => {
+    // Regression guard: `window.open` cannot launch custom-scheme
+    // URLs (`obsidian://`, `mailto:`, …) — it spawns a useless Tauri
+    // webview window. For those we MUST propagate the error to the
+    // caller, even when the failure is the CEF IPC race.
     isTauriMock.mockReturnValue(true);
     tauriOpenUrlMock.mockRejectedValue(new Error('scheme not allowed'));
 
     const { openUrl } = await import('./openUrl');
-    await expect(openUrl('obsidian://open?path=/x')).rejects.toThrow('scheme not allowed');
+    await expect(openUrl('obsidian://open?path=/Users/me/Vault')).rejects.toThrow(
+      'scheme not allowed'
+    );
     expect(windowOpenMock).not.toHaveBeenCalled();
+    // Non-http schemes log only the protocol — the rest of the URL (here the
+    // vault path) is the payload itself and must not leak to Sentry.
+    expect(addBreadcrumbMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        category: 'ipc',
+        level: 'warning',
+        message: 'tauriOpenUrl failed; evaluating fallback',
+        data: expect.objectContaining({ url: 'obsidian:' }),
+      })
+    );
+    const call = addBreadcrumbMock.mock.calls[0]?.[0] as { data?: { url?: string } } | undefined;
+    expect(call?.data?.url).not.toContain('Vault');
+    expect(call?.data?.url).not.toContain('/Users/me');
+  });
+
+  it('falls back to window.open when tauriOpenUrl rejects on an http URL (CEF IPC race recovery, #1472)', async () => {
+    // Concrete repro for OPENHUMAN-REACT-T/S/R: CEF embedder
+    // injects `window.ipc.postMessage` after `on_after_created`. A
+    // click landing in that gap causes `tauriOpenUrl` to reject with
+    // a TypeError. For http(s) URLs the safe recovery is to hand off
+    // to `window.open` so the Billing dashboard still opens.
+    isTauriMock.mockReturnValue(true);
+    const ipcError = new TypeError("Cannot read properties of undefined (reading 'postMessage')");
+    tauriOpenUrlMock.mockRejectedValue(ipcError);
+
+    const { openUrl } = await import('./openUrl');
+    await openUrl('https://tinyhumans.ai/dashboard?token=secret-redact-me');
+
+    expect(windowOpenMock).toHaveBeenCalledWith(
+      'https://tinyhumans.ai/dashboard?token=secret-redact-me',
+      '_blank',
+      'noopener,noreferrer'
+    );
+    // Breadcrumb keeps only origin for http(s) — pathname + query (which may
+    // carry tokens / emails / vault paths) must not be sent to Sentry.
+    expect(addBreadcrumbMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        category: 'ipc',
+        level: 'warning',
+        message: 'tauriOpenUrl failed; evaluating fallback',
+        data: expect.objectContaining({ url: 'https://tinyhumans.ai' }),
+      })
+    );
+    const call = addBreadcrumbMock.mock.calls[0]?.[0] as { data?: { url?: string } } | undefined;
+    expect(call?.data?.url).not.toContain('secret-redact-me');
+    expect(call?.data?.url).not.toContain('/dashboard');
   });
 });

app/src/utils/openUrl.ts

@@ -1,6 +1,29 @@
+import * as Sentry from '@sentry/react';
 import { isTauri } from '@tauri-apps/api/core';
 import { openUrl as tauriOpenUrl } from '@tauri-apps/plugin-opener';
 
+const isHttpUrl = (url: string): boolean => /^https?:\/\//i.test(url);
+
+/**
+ * Returns a low-PII representation of `url` for telemetry breadcrumbs.
+ * For http(s) we keep only the origin so the host is identifiable but the
+ * pathname/query/fragment (which may carry tokens, emails, or local paths)
+ * never leave the device. For other schemes (`mailto:`, `obsidian://`, …)
+ * we keep only the protocol — the rest of the URL is the payload itself
+ * (the email address, the vault path) and must not be logged.
+ */
+const getTelemetryUrl = (url: string): string => {
+  try {
+    const parsed = new URL(url);
+    if (parsed.protocol === 'http:' || parsed.protocol === 'https:') {
+      return parsed.origin;
+    }
+    return parsed.protocol;
+  } catch {
+    return 'invalid-url';
+  }
+};
+
 /**
  * Opens a URL using the host OS's default handler.
  *
@@ -10,19 +33,36 @@ import { openUrl as tauriOpenUrl } from '@tauri-apps/plugin-opener';
  * registered application instead of staying inside the embedded
  * webview.
  *
- * On the Tauri side errors propagate to the caller — we deliberately
- * do NOT fall back to `window.open` for desktop. The fallback would
- * spawn a Tauri webview window that has no useful behaviour for
- * custom schemes (Obsidian, mailto, etc.) and the call would appear
- * to "open in a new window" instead of handing off to the OS.
+ * CEF embedder note: the IPC bridge (`window.ipc.postMessage`) is
+ * injected on the renderer-side after `on_after_created` fires.
+ * A click landing in that gap causes the plugin's `invoke()` glue
+ * to reject with `TypeError: Cannot read properties of undefined
+ * (reading 'postMessage')`. For http(s) URLs we recover by falling
+ * back to `window.open` so the user-facing flow still works. For
+ * non-http schemes we re-throw — `window.open` would spawn a Tauri
+ * webview window that cannot handle custom schemes, which is worse
+ * UX than a propagated error the caller can surface.
  *
  * In a browser context (no Tauri) we keep the `window.open` path so
  * `https://` / `mailto:` links still work for dev/preview builds.
  */
 export const openUrl = async (url: string): Promise<void> => {
   if (isTauri()) {
-    await tauriOpenUrl(url);
-    return;
+    try {
+      await tauriOpenUrl(url);
+      return;
+    } catch (err) {
+      Sentry.addBreadcrumb({
+        category: 'ipc',
+        level: 'warning',
+        message: 'tauriOpenUrl failed; evaluating fallback',
+        data: { url: getTelemetryUrl(url), error: String(err) },
+      });
+      if (!isHttpUrl(url)) {
+        throw err;
+      }
+      // http(s) URL — safe to fall back to window.open.
+    }
   }
   window.open(url, '_blank', 'noopener,noreferrer');
 };