fix: recover wedged webview boot instead of dying on injected script failure - #3436
Conversation
…failure
When the Meteor module registry is incomplete ('Cannot find module
/app/utils/rocketchat.info'), the injected script used to throw an uncaught
rejection and give up, leaving the workspace stuck on the loading throbber
until a full app restart — a plain reload never recovers this state.
- On require failure after retries, trigger reloadServer() (force reload
with service worker + cache storage clearing)
- Cap automatic recoveries at 2 per session via a sessionStorage counter
(reset on successful boot); the pre-existing window.require recovery
path previously reloaded without any limit and now shares the same guard
- Catch rejections from start() and its setTimeout retries so failures are
logged instead of surfacing as unhandled rejections in Bugsnag
|
Warning Review limit reached
Next review available in: 3 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughChangesBoot recovery and watchdog diagnostics
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Sequence Diagram(s)sequenceDiagram
participant MainStartup
participant ServerWebview
participant Startup
participant ServerInfo
participant BootWatchdog
participant DiagnosticReport
MainStartup->>BootWatchdog: setupBootWatchdog()
ServerWebview->>BootWatchdog: attachBootWatchdog()
Startup->>ServerInfo: load server-info module
alt module loading fails
Startup->>BootWatchdog: record recovery event
Startup->>Startup: attemptBootRecovery()
else server info loads
ServerInfo->>BootWatchdog: setVersion(serverVersion)
BootWatchdog->>ServerWebview: clear boot deadline
end
BootWatchdog->>DiagnosticReport: append failure diagnostics when required
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/injected.ts`:
- Around line 59-66: Update the recovery flow around the sessionStorage write so
automatic recovery is attempted only when incrementing and storing the counter
succeeds; when setItem throws, skip reloadServer() rather than proceeding with
an untracked attempt. Preserve the existing bounded-attempt behavior when
storage is available.
- Around line 145-150: The server-info handling in injected.ts returns early
when serverInfo.version is missing, which leaves the webview stuck after
tryRequire() resolves an incomplete export. Update this branch to call
attemptBootRecovery() before returning so recovery is retried, and keep
resetBootRecovery() only on the path where serverInfo.version is present and
valid.
🪄 Autofix
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 Plus
Run ID: 29f062a7-f6af-450f-a300-15dbfc00f55e
📒 Files selected for processing (1)
src/injected.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from@rocket.chat/fuselagefor UI work unless the design requires something Fuselage does not provide.
CheckTheme.d.tsfor valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local.d.tsfiles instead of assuming.
Use React functional components with hooks.
Redux actions follow FSA shape.
Use camelCase for file names and PascalCase for components.
Prefer clear names over unnecessary comments.
Prefer editing existing files over creating new abstractions unless the new abstraction removes real complexity or matches an existing pattern.
**/*.{ts,tsx}: Use TypeScript for all new code unless explicitly told otherwise.
Use Fuselage components for all UI work; create custom components only when Fuselage lacks the required functionality.
Import Fuselage components from@rocket.chat/fuselage.
Use only valid color tokens documented byTheme.d.ts.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs such asprocess.getuid(),getgid(),geteuid(), andgetegid().
Use TypeScript strict mode.
Redux actions must follow the Flux Standard Action pattern.
Use camelCase for file names and PascalCase for component names.
Avoid unnecessary comments; prefer self-documenting code through clear naming.
Do not commit or push without explicit user permission.
Verify library APIs, props, tokens, and types against official documentation and.d.tsfiles instead of assuming they are valid.
Files:
src/injected.ts
🔇 Additional comments (1)
src/injected.ts (1)
96-123: LGTM!Also applies to: 126-143, 764-766
…views Every wedge occurrence now produces a self-contained forensic report in <logs>/boot-watchdog.jsonl instead of requiring a live autopsy. Enabled in development, opt-in elsewhere via ROCKETCHAT_BOOT_WATCHDOG=true. - Track each server webview boot cycle: a committed main-frame navigation (did-navigate) arms a 90s deadline cleared by the WEBVIEW_SERVER_VERSION_UPDATED signal; did-start-loading is recorded but does not reset the cycle (it fires for subframe/resource loads too) - On deadline exceeded, render-process-gone, unresponsive, or injected recovery exhaustion, append a JSONL report with an in-page probe (require/module/sessionStorage/service-worker state), running service workers, process metrics, event timeline, and the last 150 console messages - Wire the orphaned setVersion preload call into setServerInfo so WEBVIEW_SERVER_VERSION_UPDATED actually fires on webapp boot, as the reducer comment already assumed
Update: dev boot watchdog added (1e098ec)Every wedge occurrence now produces a self-contained forensic report — no more depending on catching it live with DevTools open. How it works
Validated by dogfooding: the watchdog's first-ever report diagnosed a false positive in itself ( Next: when the wedge reproduces naturally, the JSONL report should pin whether the broken state lives in the service worker, the module registry, or the renderer process — which decides if the escalation needs to be webview recreation. |
…tion Agent/dev workflow: read boot-watchdog.jsonl reports first, run live CDP autopsies on wedged webviews (cdp-eval.mjs helper included), walk the manual cure ladder to localize the broken layer, and map findings to the next fix on this PR.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/main.ts`:
- Line 163: Move the setupBootWatchdog() call to immediately after
createMainReduxStore() so the version listener is registered before
attachGuestWebContentsEvents(), the root window, and awaited
setupSpellChecking() can allow webview boot events. Remove the later invocation
to ensure the watchdog is initialized only once.
🪄 Autofix
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 Plus
Run ID: dded84fe-e543-4779-b5db-a27fef4558ed
📒 Files selected for processing (4)
src/main.tssrc/servers/bootWatchdog.tssrc/servers/preload/api.tssrc/ui/main/serverView/index.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from@rocket.chat/fuselagefor UI work unless the design requires something Fuselage does not provide.
CheckTheme.d.tsfor valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local.d.tsfiles instead of assuming.
Use React functional components with hooks.
Redux actions follow FSA shape.
Use camelCase for file names and PascalCase for components.
Prefer clear names over unnecessary comments.
Prefer editing existing files over creating new abstractions unless the new abstraction removes real complexity or matches an existing pattern.
**/*.{ts,tsx}: Use TypeScript for all new code unless explicitly told otherwise.
Use Fuselage components for all UI work; create custom components only when Fuselage lacks the required functionality.
Import Fuselage components from@rocket.chat/fuselage.
Use only valid color tokens documented byTheme.d.ts.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs such asprocess.getuid(),getgid(),geteuid(), andgetegid().
Use TypeScript strict mode.
Redux actions must follow the Flux Standard Action pattern.
Use camelCase for file names and PascalCase for component names.
Avoid unnecessary comments; prefer self-documenting code through clear naming.
Do not commit or push without explicit user permission.
Verify library APIs, props, tokens, and types against official documentation and.d.tsfiles instead of assuming they are valid.
Files:
src/ui/main/serverView/index.tssrc/servers/preload/api.tssrc/main.tssrc/servers/bootWatchdog.ts
🔇 Additional comments (3)
src/servers/bootWatchdog.ts (1)
1-327: LGTM!src/ui/main/serverView/index.ts (1)
24-24: LGTM!Also applies to: 268-268
src/servers/preload/api.ts (1)
45-45: LGTM!Also applies to: 87-87
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.claude/skills/boot-wedge-debug/cdp-eval.mjs:
- Around line 17-19: Update the target selection logic around targets.find to
reject ambiguous webview matches: collect all webviews whose URLs match urlPart
and continue only when exactly one matches, otherwise fail with a clear error;
alternatively, normalize and require an exact origin match before selecting the
target.
- Around line 14-16: Update target discovery in cdp-eval.mjs to enforce an abort
timeout around the /json fetch, validate response.ok before parsing, and handle
fetch or JSON failures as controlled non-zero exits. Ensure discovery cannot
block before the existing CDP evaluation timeout begins, while preserving normal
target selection for valid responses.
- Around line 28-52: Update the CDP evaluation promise around the WebSocket
lifecycle so `ws.close()` runs in a `finally` block for success, timeout, and
error paths. Add an `ws.onclose` handler that rejects if the promise is still
pending, and catch synchronous `ws.send` and `JSON.parse` failures to reject
promptly while clearing the timer and preventing duplicate settlement.
In @.claude/skills/boot-wedge-debug/SKILL.md:
- Around line 27-33: Update the dev-only boot watchdog documentation in SKILL.md
to identify the boot-watchdog.jsonl location for every supported platform,
including Windows and Linux, or document the runtime <logs> directory as the
portable location. Preserve the existing macOS path and ensure developers can
locate the file from this workflow.
🪄 Autofix
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 Plus
Run ID: 0035bcd0-ab51-4ffa-af04-1076237b4e95
📒 Files selected for processing (2)
.claude/skills/boot-wedge-debug/SKILL.md.claude/skills/boot-wedge-debug/cdp-eval.mjs
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (macos-latest, mac)
🧰 Additional context used
🪛 markdownlint-cli2 (0.23.2)
.claude/skills/boot-wedge-debug/SKILL.md
[warning] 31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 SkillSpector (2.5.1)
.claude/skills/boot-wedge-debug/SKILL.md
[warning] 65: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 103: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[error] 81: [SC2] External Script Fetching: Remote code is downloaded and executed. This bypasses code review and could introduce malicious code.
Remediation: Avoid downloading and executing remote scripts. Use trusted packages from PyPI/npm. If remote fetch is required, verify checksums and use HTTPS.
(Supply Chain (SC2))
🔇 Additional comments (4)
.claude/skills/boot-wedge-debug/SKILL.md (2)
1-4: LGTM!Also applies to: 6-23, 35-60, 68-87, 89-102, 104-120
65-65: 🔒 Security & PrivacyOther (CWE-829): Inclusion of Functionality from Untrusted Control Sphere
Reachability: Internal
Prevent
npxfrom resolving unpinned packages.When the local
electronortypescriptbinary is missing, these commands can download and execute a package selected by the registry. A compromised or newly published package would run with developer permissions. Usenpx --no-install, the repository package-manager executable, or an exact version. Verify that the install contract always provides the local binaries.Also applies to: 103-103
.claude/skills/boot-wedge-debug/cdp-eval.mjs (2)
14-16: 🎯 Functional CorrectnessVerify the Node runtime for the built-in APIs.
This utility runs with
nodeand uses globalfetchand globalWebSocket. If the repository's declared Node target does not provide either API, the documented command fails before it can inspect a webview. Confirm the declared toolchain and run this file with that pinned runtime, or add a supported client implementation.Also applies to: 28-28
1-12: LGTM!Also applies to: 20-27, 53-53
- injected.ts: abort automatic recovery when the sessionStorage counter cannot be persisted (an unpersisted counter would allow an unlimited reload loop), and trigger recovery when the server-info module resolves without a version instead of silently leaving the view wedged - main.ts: register the boot watchdog right after the store is created so WEBVIEW_SERVER_VERSION_UPDATED cannot be dispatched before the listener exists (would cause false boot-deadline-exceeded reports) - cdp-eval.mjs: reject ambiguous webview matches, add fetch timeout and response.ok check, settle the WebSocket promise exactly once and close the socket in a finally block - SKILL.md: document the watchdog log path for macOS, Windows and Linux
|
Applied all CodeRabbit findings in 21699c8 — all three Majors were valid:
Full suite green (155 suites, 1662 tests) and build clean. |
macOS installer download |
1 participant
Symptom
Intermittently, a workspace tab gets stuck on the loading throbber forever. Its webview console shows:
A plain reload (⌘R =
webContents.reload()) never recovers it; only fully restarting the app does. The wedge is intermittent and so far not deterministically reproducible (kill-during-boot and instance-race attempts all booted healthy).What was happening
injected.tsrequires/app/utils/rocketchat.infowith 5 retries (~31s). When the Meteor module registry is incomplete (wedged webview state), the retries exhaust and the error escapes as an uncaught rejection — the entire injected script dies with no recovery path, and everything after that point (setServerInfo, Notification override, unread badge listeners, presence, Outlook) never initializes.Meanwhile the app already ships the right remedy —
WEBVIEW_FORCE_RELOAD_WITH_CACHE_CLEAR(clears service workers + cache storage + cache, thenreloadIgnoringCache) — but it was only wired to one failure mode (window.requirenever appearing).Changes (
src/injected.tsonly)reloadServer()→ force reload with service worker + cache clearingsessionStoragecounter (survives reloads, is not wiped by the force-reload storage list, resets on successful boot). The pre-existingwindow.requirerecovery path previously reloaded without any limit and now shares the same guardstart()and itssetTimeoutretries now.catchand log instead of surfacing unhandled rejections to BugsnagLive validation
Observed once in dev with the fix applied: the wedge reproduced on a real workspace and the new path fired
Triggering force reload with cache clear to recover (attempt 1 of 2)instead of dying silently. In that single observation the cache-clearing reload did not cure the wedge — see below.Known limitation / next steps (PR will be updated as the investigation progresses)
<webview>(fresh renderer process) — the per-server equivalent of the app restart that reliably cures it — after recovery attempts exhaust.Validation
npx tsc --noEmitcleanyarn lintcleanyarn test: 155 suites, 1662 passed, 2 skippedyarn buildcleanSummary by CodeRabbit