fix: keep first Jitsi call on its jwt navigation on cold cache (SUP-1072) - #3416
Conversation
Sorry, something went wrong.
On a cold cache the Jitsi page commits and consumes the jwt while its bundles are still downloading, so did-finish-load can miss the 15s loading timeout. Auto-recovery then reloaded the current URL — already jwt-stripped by Jitsi's history.replaceState — aborting the in-flight load (ERR_ABORTED -3) and landing the user on the prejoin page. - clear the loading timeout once the webview page commits (dom-ready); the timeout now only covers navigations that never commit - recovery attempt 1 re-navigates to the original validated call URL instead of blind-reloading the current (possibly tokenless) URL - ignore self-inflicted ERR_ABORTED (-3) in did-fail-load - make timeout/recovery logs production-visible (console.warn); they were dev-gated, which is why customer logs showed no recovery trace - guard attempt 2's about:blank hop from resetting the bounded 3-attempt escalation ladder
WalkthroughThe video call window now validates recovery URLs, supports an about:blank transition during recovery attempt 2, preserves recovery counters across placeholder navigation, upgrades recovery logs to warnings, and ignores self-inflicted aborted loads. ChangesVideo call recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: type: bug 🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agentsVerify 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/videoCallWindow/video-call-window.ts`: - Around line 278-317: Update the attempt 2 branch in attemptAutoRecovery to handle a missing state.url when a webview exists: add an else path that advances recoveryAttempt and invokes attemptAutoRecovery, matching the URL-validation catch behavior so the chain reaches the next recovery attempt.
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: 62010f05-73aa-48ad-9bcf-b191cf930865
📥 CommitsReviewing files that changed from the base of the PR and between 1c1d426 and 628878f.
📒 Files selected for processing (1)- src/videoCallWindow/video-call-window.ts
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from @rocket.chat/fuselage for UI work unless the design requires something Fuselage does not provide.
Check Theme.d.ts for valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local .d.ts files 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 by Theme.d.ts.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs such as process.getuid(), getgid(), geteuid(), and getegid().
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.ts files instead of assuming they are valid.
Files:
- src/videoCallWindow/video-call-window.ts
src/videoCallWindow/video-call-window.ts (1)64-66: LGTM!
Also applies to: 241-243, 425-427, 441-456, 538-544
Sorry, something went wrong.
| console.warn( | ||
| `Video call window: Auto-recovery attempt ${currentAttempt}/${MAX_RECOVERY_ATTEMPTS} - ${strategy}` | ||
| ); | ||
|
|
||
| recoveryTimeout = setTimeout(() => { | ||
| const webview = state.webview as any; | ||
|
|
||
| switch (currentAttempt) { | ||
| case 1: | ||
| if (webview) { | ||
| if (webview && state.url) { | ||
| try { | ||
| webview.src = validateVideoCallUrl(state.url); | ||
| } catch (error) { | ||
| console.error( | ||
| 'Video call window: URL validation failed during recovery:', | ||
| error | ||
| ); | ||
| console.error( | ||
| 'Video call window: Skipping webview reload recovery step, proceeding to next recovery attempt' | ||
| ); | ||
| if (recoveryTimeout) { | ||
| clearTimeout(recoveryTimeout); | ||
| recoveryTimeout = null; | ||
| } | ||
| state.recoveryAttempt = currentAttempt; | ||
| attemptAutoRecovery(); | ||
| return; | ||
| } | ||
| } else if (webview) { | ||
| webview.reload(); | ||
| } | ||
| break; | ||
| case 2: | ||
| if (webview && state.url) { | ||
| try { | ||
| const validatedUrl = validateVideoCallUrl(state.url); | ||
| isAboutBlankRecoveryHop = true; | ||
| webview.src = 'about:blank'; | ||
| setTimeout(() => { | ||
| isAboutBlankRecoveryHop = false; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Ensure the recovery chain continues if the URL is missing during attempt 2.
If state.url is missing (which is possible per the upstream IPC contract), the if (webview && state.url) block evaluates to false and does nothing. This breaks the recovery chain entirely: no navigation occurs, did-start-loading never fires, and no further timeouts are scheduled, leaving the UI permanently stuck in a loading state without ever reaching attempt 3.
Add an else block to explicitly proceed to the next attempt, matching the behavior in the URL validation catch block.
🐛 Proposed fix case 2:
if (webview && state.url) {
try {
const validatedUrl = validateVideoCallUrl(state.url);
isAboutBlankRecoveryHop = true;
webview.src = 'about:blank';
setTimeout(() => {
isAboutBlankRecoveryHop = false;
if (webview) {
webview.src = validatedUrl;
}
}, 500);
} catch (error) {
console.error(
'Video call window: URL validation failed during recovery:',
error
);
console.error(
'Video call window: Skipping URL refresh recovery step, proceeding to next recovery attempt'
);
if (recoveryTimeout) {
clearTimeout(recoveryTimeout);
recoveryTimeout = null;
}
state.recoveryAttempt = currentAttempt;
attemptAutoRecovery();
return;
}
+ } else {
+ console.warn(
+ 'Video call window: Missing URL during recovery attempt 2, proceeding to next recovery attempt'
+ );
+ state.recoveryAttempt = currentAttempt;
+ attemptAutoRecovery();
+ return;
}
break;‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.warn( | |
| `Video call window: Auto-recovery attempt ${currentAttempt}/${MAX_RECOVERY_ATTEMPTS} - ${strategy}` | |
| ); | |
| recoveryTimeout = setTimeout(() => { | |
| const webview = state.webview as any; | |
| switch (currentAttempt) { | |
| case 1: | |
| if (webview) { | |
| if (webview && state.url) { | |
| try { | |
| webview.src = validateVideoCallUrl(state.url); | |
| } catch (error) { | |
| console.error( | |
| 'Video call window: URL validation failed during recovery:', | |
| error | |
| ); | |
| console.error( | |
| 'Video call window: Skipping webview reload recovery step, proceeding to next recovery attempt' | |
| ); | |
| if (recoveryTimeout) { | |
| clearTimeout(recoveryTimeout); | |
| recoveryTimeout = null; | |
| } | |
| state.recoveryAttempt = currentAttempt; | |
| attemptAutoRecovery(); | |
| return; | |
| } | |
| } else if (webview) { | |
| webview.reload(); | |
| } | |
| break; | |
| case 2: | |
| if (webview && state.url) { | |
| try { | |
| const validatedUrl = validateVideoCallUrl(state.url); | |
| isAboutBlankRecoveryHop = true; | |
| webview.src = 'about:blank'; | |
| setTimeout(() => { | |
| isAboutBlankRecoveryHop = false; | |
| console.warn( | |
| `Video call window: Auto-recovery attempt ${currentAttempt}/${MAX_RECOVERY_ATTEMPTS} - ${strategy}` | |
| ); | |
| recoveryTimeout = setTimeout(() => { | |
| const webview = state.webview as any; | |
| switch (currentAttempt) { | |
| case 1: | |
| if (webview && state.url) { | |
| try { | |
| webview.src = validateVideoCallUrl(state.url); | |
| } catch (error) { | |
| console.error( | |
| 'Video call window: URL validation failed during recovery:', | |
| error | |
| ); | |
| console.error( | |
| 'Video call window: Skipping webview reload recovery step, proceeding to next recovery attempt' | |
| ); | |
| if (recoveryTimeout) { | |
| clearTimeout(recoveryTimeout); | |
| recoveryTimeout = null; | |
| } | |
| state.recoveryAttempt = currentAttempt; | |
| attemptAutoRecovery(); | |
| return; | |
| } | |
| } else if (webview) { | |
| webview.reload(); | |
| } | |
| break; | |
| case 2: | |
| if (webview && state.url) { | |
| try { | |
| const validatedUrl = validateVideoCallUrl(state.url); | |
| isAboutBlankRecoveryHop = true; | |
| webview.src = 'about:blank'; | |
| setTimeout(() => { | |
| isAboutBlankRecoveryHop = false; | |
| if (webview) { | |
| webview.src = validatedUrl; | |
| } | |
| }, 500); | |
| } catch (error) { | |
| console.error( | |
| 'Video call window: URL validation failed during recovery:', | |
| error | |
| ); | |
| console.error( | |
| 'Video call window: Skipping URL refresh recovery step, proceeding to next recovery attempt' | |
| ); | |
| if (recoveryTimeout) { | |
| clearTimeout(recoveryTimeout); | |
| recoveryTimeout = null; | |
| } | |
| state.recoveryAttempt = currentAttempt; | |
| attemptAutoRecovery(); | |
| return; | |
| } | |
| } else { | |
| console.warn( | |
| 'Video call window: Missing URL during recovery attempt 2, proceeding to next recovery attempt' | |
| ); | |
| state.recoveryAttempt = currentAttempt; | |
| attemptAutoRecovery(); | |
| return; | |
| } | |
| break; |
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/videoCallWindow/video-call-window.ts` around lines 278 - 317, Update the attempt 2 branch in attemptAutoRecovery to handle a missing state.url when a webview exists: add an else path that advances recoveryAttempt and invokes attemptAutoRecovery, matching the URL-validation catch behavior so the chain reaches the next recovery attempt.
Sorry, something went wrong.
Linux installer download |
Sorry, something went wrong.
Windows installer download |
Sorry, something went wrong.
macOS installer download |
Sorry, something went wrong.
What
Fixes the cold-cache first-call flow in the video call window: the first Jitsi call after a fresh install/update (or cleared cache) no longer drops to the Jitsi prejoin page — it stays on the jwt-carrying navigation and auto-joins.
Jira: SUP-1072
Why
On a cold cache, the Jitsi page commits and consumes the jwt=...&prejoinPageEnabled=false URL while its bundles are still downloading, so did-finish-load can arrive after the 15s loading timeout. The auto-recovery path — designed for navigations that never load — did not cover this committed-but-still-downloading case: it reloaded the current URL, which Jitsi had already jwt-stripped via history.replaceState, aborting the in-flight load (did-fail-load with ERR_ABORTED -3) and finishing on the tokenless URL, i.e. the prejoin page. All recovery logging was dev-gated, so production logs showed no trace of the trigger.
How
All changes in src/videoCallWindow/video-call-window.ts:
Validation
Notes
Summary by CodeRabbit