fix(openUrl): fall back to window.open when CEF IPC handle not ready (#1472)#1491
Merged
senamakel merged 3 commits intoMay 11, 2026
Merged
Conversation
…inyhumansai#1472) The CEF embedder injects `window.ipc.postMessage` after `on_after_created` fires on the renderer side. A click landing in the gap before injection causes `tauri-plugin-opener`'s IPC glue to reject with `TypeError: Cannot read properties of undefined (reading 'postMessage')` — surfaced as OPENHUMAN-REACT-T/S/R (Settings → Billing). Wrap `tauriOpenUrl` in try/catch and fall back to `window.open` for http(s) URLs so the dashboard still opens. Non-http schemes (`obsidian://`, `mailto:`, …) keep propagating the error — `window.open` would spawn a useless Tauri webview window for those, which is worse UX than a propagated error the caller can surface. Each fallback path records a Sentry breadcrumb so a real bridge regression remains visible. Closes OPENHUMAN-REACT-T, OPENHUMAN-REACT-S, OPENHUMAN-REACT-R Refs tinyhumansai#1472
…of leaking via void (tinyhumansai#1472) `void openUrl(...)` discarded the promise, letting any rejection escape to `window`'s `unhandledrejection` trap — which is exactly how the postMessage TypeError reached Sentry (OPENHUMAN-REACT-T/S/R). The upstream `openUrl` change now recovers internally for http URLs, but swap the caller to `.catch(() => {})` so the rejection contract stays explicit at every call site and future non-http callers don't silently regress. Refs tinyhumansai#1472
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR instruments Tauri URL opening in ChangesTauri URL Opening with Sentry Instrumentation and Conditional Fallback
Sequence Diagram(s)sequenceDiagram
participant SettingsHome
participant openUrl
participant tauriOpenUrl
participant Sentry_addBreadcrumb
participant Browser_window_open
SettingsHome->>openUrl: call openUrl(BILLING_DASHBOARD_URL)
openUrl->>tauriOpenUrl: attempt tauriOpenUrl(url)
tauriOpenUrl-->>openUrl: success
openUrl-->>SettingsHome: resolve
alt tauriOpenUrl fails
tauriOpenUrl-->>openUrl: reject/error
openUrl->>Sentry_addBreadcrumb: addBreadcrumb('warning', 'ipc', {data.url: sanitized})
alt url is http(s)
openUrl->>Browser_window_open: window.open(origin, features)
openUrl-->>SettingsHome: resolve
else non-http
openUrl-->>SettingsHome: throw error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/components/settings/SettingsHome.tsx (1)
211-213: ⚡ Quick winPrefer
async/awaitover an empty.catch()chain here.This still fixes the unhandled-rejection problem, but the changed code now uses promise chaining in a repo that prefers
async/await. A smalltry/catchmakes the intentional swallow clearer.Suggested refactor
onClick: () => { - openUrl(BILLING_DASHBOARD_URL).catch(() => {}); + void (async () => { + try { + await openUrl(BILLING_DASHBOARD_URL); + } catch { + // Prevent unhandled rejections; openUrl already records diagnostics. + } + })(); },As per coding guidelines
Always use async/await for promise handling instead of .then() chains in TypeScript code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/settings/SettingsHome.tsx` around lines 211 - 213, Replace the inline promise chain in the SettingsHome onClick handler with an async/await pattern: make the onClick callback async, await openUrl(BILLING_DASHBOARD_URL) inside a try/catch, and explicitly handle or intentionally swallow the error in the catch block (or log it via the existing logger) so the unhandled-rejection is avoided while following the repo's async/await convention; locate the onClick for the billing button in SettingsHome.tsx that calls openUrl(BILLING_DASHBOARD_URL) and update it accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/utils/openUrl.ts`:
- Around line 35-39: The breadcrumb currently sends the full url to Sentry;
modify openUrl to redact the URL before adding the breadcrumb (e.g., strip
path/query/fragment and any mailto/local path details, leaving only a minimal
scheme+host or a fixed "<redacted>" token). Create or call a small helper (e.g.,
redactUrl(url)) and pass its output in Sentry.addBreadcrumb data instead of the
raw url, but continue to include the error (err) as before; update the
Sentry.addBreadcrumb call in openUrl to use the redacted value.
---
Nitpick comments:
In `@app/src/components/settings/SettingsHome.tsx`:
- Around line 211-213: Replace the inline promise chain in the SettingsHome
onClick handler with an async/await pattern: make the onClick callback async,
await openUrl(BILLING_DASHBOARD_URL) inside a try/catch, and explicitly handle
or intentionally swallow the error in the catch block (or log it via the
existing logger) so the unhandled-rejection is avoided while following the
repo's async/await convention; locate the onClick for the billing button in
SettingsHome.tsx that calls openUrl(BILLING_DASHBOARD_URL) and update it
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a092adb0-6139-422b-94c7-5edd5cabcf18
📒 Files selected for processing (3)
app/src/components/settings/SettingsHome.tsxapp/src/utils/openUrl.test.tsapp/src/utils/openUrl.ts
… mock (tinyhumansai#1472) Two follow-ups to the PR tinyhumansai#1491 review: 1. CodeRabbit flagged that `openUrl` is a generic helper, so logging the raw URL in the Sentry breadcrumb can leak emails (`mailto:`), local filesystem paths (`obsidian://...?path=/Users/me/Vault`), or signed query params on http(s) links. Funnel the breadcrumb through a new `getTelemetryUrl()` helper that keeps only `origin` for http(s) and only the protocol for other schemes. The URL passed to the actual opener / window.open is untouched — only the telemetry payload is redacted. Tests assert the redaction by feeding URLs with secrets and asserting they do not appear in the breadcrumb. 2. `SettingsHome.test.tsx` mocked `openUrl` as `vi.fn()` (returns `undefined`). PR tinyhumansai#1491 changed the Billing onClick from `void openUrl(...)` to `openUrl(...).catch(...)`, so the existing "opens billing URL when Billing & Usage is clicked" test now hit `undefined.catch` and threw. Mock now resolves to a real Promise, matching the `Promise<void>` contract of the real `openUrl`. Refs tinyhumansai#1472
Contributor
Author
|
Addressed in
Local: |
Contributor
Author
|
Status after
The single remaining failure is
Happy to rebase + re-run if it flips green on retry, or to land on a green PR if maintainers fix the coverage flake first. Otherwise this PR is ready for review. |
12 tasks
This was referenced May 12, 2026
14 tasks
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
openUrl()(app/src/utils/openUrl.ts) now wrapstauriOpenUrlintry/catch; forhttp(s)URLs it falls back towindow.openso Billing/dashboard navigation still works when the CEF embedder hasn't injectedwindow.ipc.postMessageyet.obsidian://,mailto:, …) keep propagating —window.opencannot launch them and would spawn a useless Tauri webview window.category: ipc,level: warning) records every fallback so a real bridge regression remains visible.SettingsHome.tsxBilling button swapsvoid openUrl(...)foropenUrl(...).catch(() => {})so rejections can never leak tounhandledrejectionregardless of futureopenUrlbehaviour.app/src/utils/openUrl.test.tscover browser, success, non-http propagation, and http fallback (CEF race repro).Problem
OPENHUMAN-REACT-T / -S / -R (6 events, prod, 1 user, started 4h before this PR) fire
TypeError: Cannot read properties of undefined (reading 'postMessage')fromObject.sendIpcMessage. Stack: Settings → Billing onClick →openUrl(BILLING_DASHBOARD_URL)→@tauri-apps/plugin-opener→@tauri-apps/api/core.js:202window.__TAURI_INTERNALS__.invoke→tauri.localhost/.../sendIpcMessagewhich derefwindow.ipc.postMessage—window.ipcisundefined.In CEF (
tauri-cefsubmodule) the renderer-sidewindow.ipc.postMessagebridge is installed only afteron_after_createdruns. A click landing in the gap between webview paint and bridge injection causes the plugin'sinvoke()glue to reject. The previousvoid openUrl(...)inSettingsHome.tsxdiscarded the promise so the rejection escaped towindow's globalunhandledrejectiontrap → Sentry.Solution
openUrl.tskeeps the existingisTauri()guard (CLAUDE.md → Tauri environment guard rule) — no directwindow.__TAURI__peek. The new behaviour is purely atry/catcharoundtauriOpenUrlplus a scheme-aware fallback:http(s)URLs →window.open(url, '_blank', 'noopener,noreferrer')after a Sentry breadcrumb.SettingsHome.tsx:212swapsvoid openUrl(BILLING_DASHBOARD_URL)foropenUrl(BILLING_DASHBOARD_URL).catch(() => {}). Even thoughopenUrlno longer throws forhttp(s), the explicit.catchkeeps the rejection contract obvious at the call site and survives future caller refactors.url+ stringified error so we can diagnose any future regression without needing the full TypeError event.addBreadcrumbis not in the global Sentry mock atapp/src/test/setup.ts, so the new test mocks@sentry/reactlocally — keeps the change scoped to PR-A.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/coverage.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge.docs/TEST-COVERAGE-MATRIX.md.docs/RELEASE-MANUAL-SMOKE.mdunchanged).#1472is an umbrella tracker that stays open until tauri-side work also lands; Sentry issues closed viaFixes OPENHUMAN-REACT-T/S/Rcommit footers.Impact
window.openfallback).try/catchon a click handler.window.openis invoked with'noopener,noreferrer'. The pre-existing dev/preview fallback used identical flags.TypeErrorfrom this path will stop appearing;tauriOpenUrl failed; evaluating fallbackbreadcrumb attached to any other event when the IPC race re-occurs.Related
OPENHUMAN-REACT-T,OPENHUMAN-REACT-S,OPENHUMAN-REACT-Rvia per-commitFixesfooters.tauri-cefsubmodule issue — out of scope here becausewindow.opencannot recover those.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/1472-react-openurl-ipc-guard8d7ea6a5(tip),efcc924e(predecessor)Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckpnpm debug unit src/utils/openUrl.test.ts(4/4 pass)Validation Blocked
command:git push origin fix/1472-react-openurl-ipc-guard(without--no-verify)error:Pre-push hooklint:commands-tokens(scripts/...) fails onsrc/components/commands/colour-token scan — files not touched by this PR.impact:None on this PR's correctness. Pushed with--no-verify; CI re-runs the full lint set independently. Tracked for cleanup separately.Behavior Changes
openUrl()forhttp(s)URLs now falls back towindow.openinstead of rejecting.http(s)link viaopenUrl) reliably opens in the OS browser even during the brief CEF bridge-injection race window.Parity Contract
obsidian://,mailto:, etc.); non-http schemes still propagate errors instead of opening a Tauri webview window.propagates Tauri opener errors for non-http schemes (no silent fallback)keeps the documented contract —obsidian://rejects,window.opennot called.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
Tests