-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(ipc): guard isTauri() on __TAURI_INTERNALS__.invoke (OPENHUMAN-REACT-S) #1556
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
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 |
|---|---|---|
|
|
@@ -98,6 +98,17 @@ if (typeof Element !== 'undefined' && !Element.prototype.scrollIntoView) { | |
| Element.prototype.scrollIntoView = function () {}; | ||
| } | ||
|
|
||
| // The hardened `isTauri()` (in `utils/tauriCommands/common.ts`) checks both | ||
| // `coreIsTauri()` and `window.__TAURI_INTERNALS__.invoke`. Many existing test | ||
| // files mock `@tauri-apps/api/core::isTauri` to `true` to exercise the | ||
| // Tauri branch; without a matching IPC handle on `window` they would now | ||
| // regress to the non-Tauri path. Seed a no-op handle once globally so the | ||
| // IPC-readiness check passes by default. Tests that *want* the CEF gap | ||
| // behaviour can `delete window.__TAURI_INTERNALS__` in a `beforeEach`. | ||
| ( | ||
|
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. [major] The seed is set once globally at module load but not restored in Suggestion — re-seed in the existing afterEach(() => {
clearRequestLog();
cleanup();
// Re-seed the IPC handle after any test that may have deleted it.
(window as unknown as { __TAURI_INTERNALS__: { invoke: () => Promise<unknown> } })
.__TAURI_INTERNALS__ = { invoke: vi.fn(() => Promise.resolve()) };
});
Contributor
Author
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. Re-seeded |
||
| window as unknown as { __TAURI_INTERNALS__: { invoke: () => Promise<unknown> } } | ||
| ).__TAURI_INTERNALS__ = { invoke: vi.fn(() => Promise.resolve()) }; | ||
|
|
||
| // Mock Tauri APIs (not available in test env) | ||
| vi.mock('@tauri-apps/api/core', () => ({ invoke: vi.fn(), isTauri: vi.fn(() => false) })); | ||
|
|
||
|
|
@@ -223,6 +234,13 @@ if (!process.env.DEBUG_TESTS) { | |
| afterEach(() => { | ||
| clearRequestLog(); | ||
| cleanup(); | ||
| // Re-seed the IPC handle after any test that may have deleted it | ||
| // (e.g. tests exercising the CEF-gap branch of `isTauri()`). Without | ||
| // this, sibling tests in the same jsdom worker would silently regress | ||
| // to the non-Tauri path. Per graycyrus review on PR #1556. | ||
| ( | ||
| window as unknown as { __TAURI_INTERNALS__: { invoke: () => Promise<unknown> } } | ||
| ).__TAURI_INTERNALS__ = { invoke: vi.fn(() => Promise.resolve()) }; | ||
| }); | ||
| afterAll(async () => { | ||
| await stopMockServer(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| /** | ||
| * Unit tests for `isTauri()` — the canonical Tauri-runtime guard used across | ||
| * `app/src/`. Beyond delegating to `@tauri-apps/api/core::isTauri()`, this | ||
| * wrapper also confirms that the IPC transport (`window.__TAURI_INTERNALS__ | ||
| * .invoke`) is wired before reporting `true`. | ||
| * | ||
| * Why it matters: under CEF, `globalThis.isTauri` (which the underlying | ||
| * `coreIsTauri()` checks) is injected by the webview bootstrap BEFORE the | ||
| * `postMessage` IPC bridge is connected. An `invoke()` landing in that gap | ||
| * throws `TypeError: Cannot read properties of undefined (reading | ||
| * 'postMessage')` deep inside Tauri's `sendIpcMessage`, which surfaces as | ||
| * the OPENHUMAN-REACT-S Sentry issue (#1472 follow-up). All call sites that | ||
| * gate on `isTauri()` should now route through the non-Tauri branch during | ||
| * the gap instead of bursting into IPC. | ||
| */ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { isTauri } from './common'; | ||
|
|
||
| const coreIsTauriMock = vi.fn(); | ||
|
|
||
| vi.mock('@tauri-apps/api/core', () => ({ isTauri: () => coreIsTauriMock() })); | ||
|
|
||
| describe('isTauri (tauriCommands/common)', () => { | ||
| // We mutate `window` to simulate Tauri-runtime bootstrap state across cases. | ||
| // Stash + restore so other tests in the suite (which share the jsdom global) | ||
| // see a pristine window. | ||
| let originalInternals: unknown; | ||
|
|
||
| beforeEach(() => { | ||
| coreIsTauriMock.mockReset(); | ||
| originalInternals = (window as unknown as { __TAURI_INTERNALS__?: unknown }) | ||
| .__TAURI_INTERNALS__; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| if (originalInternals === undefined) { | ||
| delete (window as unknown as { __TAURI_INTERNALS__?: unknown }).__TAURI_INTERNALS__; | ||
| } else { | ||
| (window as unknown as { __TAURI_INTERNALS__?: unknown }).__TAURI_INTERNALS__ = | ||
| originalInternals; | ||
| } | ||
| }); | ||
|
|
||
| it('returns false when not running in Tauri at all (browser/Vitest)', () => { | ||
| coreIsTauriMock.mockReturnValue(false); | ||
| delete (window as unknown as { __TAURI_INTERNALS__?: unknown }).__TAURI_INTERNALS__; | ||
|
|
||
| expect(isTauri()).toBe(false); | ||
| }); | ||
|
|
||
| it('returns true when both the runtime flag and the IPC `invoke` handle are present', () => { | ||
| coreIsTauriMock.mockReturnValue(true); | ||
| (window as unknown as { __TAURI_INTERNALS__?: { invoke: unknown } }).__TAURI_INTERNALS__ = { | ||
| invoke: () => Promise.resolve(), | ||
| }; | ||
|
|
||
| expect(isTauri()).toBe(true); | ||
| }); | ||
|
|
||
| // The OPENHUMAN-REACT-S regression: Tauri sets `globalThis.isTauri = true` | ||
| // (so the official check returns true) before CEF wires the IPC postMessage | ||
| // bridge. During that gap any unguarded `invoke(...)` blows up inside | ||
| // `sendIpcMessage` with the "Cannot read properties of undefined (reading | ||
| // 'postMessage')" TypeError. Our guard must short-circuit to `false` so | ||
| // call sites skip the IPC path instead of trusting the runtime flag alone. | ||
| it('returns false during the CEF gap when runtime flag is set but __TAURI_INTERNALS__ is missing', () => { | ||
| coreIsTauriMock.mockReturnValue(true); | ||
| delete (window as unknown as { __TAURI_INTERNALS__?: unknown }).__TAURI_INTERNALS__; | ||
|
|
||
| expect(isTauri()).toBe(false); | ||
| }); | ||
|
|
||
| it('returns false during the partial-bootstrap gap when __TAURI_INTERNALS__ exists but `invoke` is not yet wired', () => { | ||
| coreIsTauriMock.mockReturnValue(true); | ||
| // Some CEF bootstrap stages set the object literal before the IPC handle | ||
| // is attached. Treat that as "not ready". | ||
| (window as unknown as { __TAURI_INTERNALS__?: Record<string, unknown> }).__TAURI_INTERNALS__ = | ||
| {}; | ||
|
|
||
| expect(isTauri()).toBe(false); | ||
| }); | ||
|
|
||
| it('returns false when __TAURI_INTERNALS__.invoke is present but not a function', () => { | ||
| coreIsTauriMock.mockReturnValue(true); | ||
| (window as unknown as { __TAURI_INTERNALS__?: { invoke: unknown } }).__TAURI_INTERNALS__ = { | ||
| invoke: 'not-a-function', | ||
| }; | ||
|
|
||
| expect(isTauri()).toBe(false); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,41 @@ | |
| * Common utilities and types for Tauri Commands. | ||
| */ | ||
| import { isTauri as coreIsTauri } from '@tauri-apps/api/core'; | ||
| import debug from 'debug'; | ||
|
|
||
| // Check if we're running in Tauri | ||
| const log = debug('tauri:ipc-guard'); | ||
|
|
||
| /** | ||
| * True when the Tauri runtime is present AND the underlying IPC transport is | ||
| * wired. The official `coreIsTauri()` check (which reads `globalThis.isTauri`) | ||
| * is set early by Tauri's webview bootstrap, but on CEF `__TAURI_INTERNALS__` | ||
| * (and the `postMessage` bridge it dispatches through) is injected *after* | ||
| * `on_after_created` fires. An `invoke()` landing in that gap throws | ||
| * `TypeError: Cannot read properties of undefined (reading 'postMessage')` | ||
| * deep inside Tauri's `sendIpcMessage` — see OPENHUMAN-REACT-S / #1472. | ||
| * | ||
| * Callers that gate on `isTauri()` BEFORE invoking should therefore use this | ||
| * function; it returns `false` during the bootstrap gap so the call site | ||
| * takes the non-Tauri branch (skip / fallback) instead of synchronously | ||
| * throwing into a `new Promise` body where the rejection escapes the local | ||
| * try/catch and lands as an unhandled Sentry event. | ||
| */ | ||
| export const isTauri = (): boolean => { | ||
| // Tauri v2: prefer the official runtime check over window globals. | ||
| return coreIsTauri(); | ||
| if (!coreIsTauri()) return false; | ||
| if (typeof window === 'undefined') return false; | ||
| // Narrow `window` access through a single optional chain so the check is | ||
| // resilient to either `__TAURI_INTERNALS__` being absent or `.invoke` | ||
| // being missing while the rest of the object is partially populated. | ||
| const internals = (window as unknown as { __TAURI_INTERNALS__?: { invoke?: unknown } }) | ||
| .__TAURI_INTERNALS__; | ||
| if (typeof internals?.invoke !== 'function') { | ||
| // Bridge-missing branch: distinct from `!coreIsTauri()` (= not in Tauri | ||
| // at all). Logging here makes the CEF bootstrap gap observable in dev | ||
| // and is a no-op in production (debug namespace disabled by default). | ||
| log('isTauri() -> false: IPC bridge not wired (CEF bootstrap gap or non-Tauri)'); | ||
| return false; | ||
| } | ||
| return true; | ||
| }; | ||
|
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. [minor] No debug logging when the guard short-circuits due to missing IPC bridge. Per CLAUDE.md's debug logging requirement, new/changed flows should log branches and state transitions. This wrapper is the single choke point for catching the CEF gap at runtime — but when it returns import debug from 'debug';
const log = debug('tauri:ipc-guard');
export const isTauri = (): boolean => {
if (!coreIsTauri()) return false;
if (typeof window === 'undefined') return false;
const internals = (window as unknown as { __TAURI_INTERNALS__?: { invoke?: unknown } })
.__TAURI_INTERNALS__;
if (typeof internals?.invoke !== 'function') {
log('isTauri() → false: IPC bridge not yet wired (CEF gap or non-Tauri)');
return false;
}
return true;
};
Contributor
Author
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. Added |
||
|
|
||
| export interface CommandResponse<T> { | ||
|
|
||
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.
[minor] Stale comment about "pre-Tauri detection".
The existing comment near the
currentWindowLabelblock says something like "Detect it before we touch any Tauri APIs" — buttauriRuntimeAvailable()now IS the hardenedisTauri()which itself readswindow.__TAURI_INTERNALS__. Worth a quick update to reflect reality so a future engineer doesn't wonder why we're touching internals in the "pre-Tauri" detection block. Behavior is correct; just the comment is misleading.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.
Rewrote the stale comment block above the
urlWindowParamIIFE inapp/src/main.tsxto reflect thattauriRuntimeAvailable()is the hardenedisTauri()which itself readswindow.__TAURI_INTERNALS__. Landed in 169f1fc.