fix[client]: Exit WebXR when failed to connect or streaming error#342
Merged
yanziz-nvidia merged 1 commit intomainfrom Apr 2, 2026
Merged
fix[client]: Exit WebXR when failed to connect or streaming error#342yanziz-nvidia merged 1 commit intomainfrom
yanziz-nvidia merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughAdded an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deps/cloudxr/webxr_client/src/App.tsx (1)
596-617: 🧹 Nitpick | 🔵 TrivialConsider guarding against redundant
session.end()calls.If
handleDisconnectis invoked multiple times in quick succession (e.g., from rapidly-firing error callbacks), callingend()on an already-ending session will reject with anInvalidStateError, displaying a potentially confusing message to the user.A simple guard could prevent this:
🛡️ Optional: Add guard to prevent redundant session.end() calls
+ const isDisconnectingRef = useRef(false); const handleDisconnect = () => { console.log('Disconnect pressed'); // Cleanup countdown state on disconnect if (countdownTimerRef.current !== null) { clearInterval(countdownTimerRef.current); countdownTimerRef.current = null; } setIsCountingDown(false); setCountdownRemaining(0); setIsTeleopRunning(false); const xrState = store.getState(); const session = xrState.session; - if (session) { + if (session && !isDisconnectingRef.current) { + isDisconnectingRef.current = true; session.end().catch((err: unknown) => { setErrorMessage( `Failed to end XR session: ${err instanceof Error ? err.message : String(err)}` ); + }).finally(() => { + isDisconnectingRef.current = false; }); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deps/cloudxr/webxr_client/src/App.tsx` around lines 596 - 617, handleDisconnect currently calls session.end() without guarding against repeated calls; add a guard (eg. a ref or boolean like sessionEndingRef or isSessionEnding state) and check it before calling session.end(), set it true immediately before calling end(), and in the promise handlers (then/catch/finally) reset it (false) or clear the ref so subsequent disconnects are no-ops; update references to the XR session from store.getState().session and ensure the flag is used around that session.end() invocation in handleDisconnect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@deps/cloudxr/webxr_client/src/App.tsx`:
- Around line 596-617: handleDisconnect currently calls session.end() without
guarding against repeated calls; add a guard (eg. a ref or boolean like
sessionEndingRef or isSessionEnding state) and check it before calling
session.end(), set it true immediately before calling end(), and in the promise
handlers (then/catch/finally) reset it (false) or clear the ref so subsequent
disconnects are no-ops; update references to the XR session from
store.getState().session and ensure the flag is used around that session.end()
invocation in handleDisconnect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e0745617-bcbf-453c-a90c-e0713749e6ee
📒 Files selected for processing (2)
deps/cloudxr/webxr_client/helpers/react/CloudXRComponent.tsxdeps/cloudxr/webxr_client/src/App.tsx
nvddr
reviewed
Apr 1, 2026
nvddr
reviewed
Apr 1, 2026
nvddr
reviewed
Apr 1, 2026
blongs-nv
reviewed
Apr 1, 2026
blongs-nv
approved these changes
Apr 1, 2026
yanziz-nvidia
force-pushed
the
yanziz/exit-when-fail
branch
from
483e340 to
e63df28
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
deps/cloudxr/webxr_client/helpers/react/CloudXRComponent.tsx (1)
348-352: 🧹 Nitpick | 🔵 TrivialConsider early-exit pattern for improved readability.
The
if (referenceSpace)block (lines 184-352) is quite long. An early-exit pattern would reduce nesting and improve readability, consistent with the past review suggestion:♻️ Optional refactor: early-exit pattern
- if (referenceSpace) { + if (!referenceSpace) { + onStatusChange?.(false, 'Reference Space Unavailable'); + onError?.('Could not obtain an XR reference space for CloudXR'); + onExitImmersiveXRRef.current?.(); + return; + } + // Ensure that the session is not already created. if (cxrSessionRef.current) { console.error('CloudXR session already exists'); return; } // ... rest of session setup at reduced indent level ... - } else { - onStatusChange?.(false, 'Reference Space Unavailable'); - onError?.('Could not obtain an XR reference space for CloudXR'); - onExitImmersiveXRRef.current?.(); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deps/cloudxr/webxr_client/helpers/react/CloudXRComponent.tsx` around lines 348 - 352, Invert the large if (referenceSpace) block to use an early-exit: at the start of the handler where referenceSpace is checked in CloudXRComponent.tsx, add a guard if (!referenceSpace) { onStatusChange?.(false, 'Reference Space Unavailable'); onError?.('Could not obtain an XR reference space for CloudXR'); onExitImmersiveXRRef.current?.(); return; } and then remove the corresponding else branch so the remaining logic runs unindented; this keeps calls to onStatusChange, onError, and onExitImmersiveXRRef the same but simplifies control flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@deps/cloudxr/webxr_client/helpers/react/CloudXRComponent.tsx`:
- Around line 348-352: Invert the large if (referenceSpace) block to use an
early-exit: at the start of the handler where referenceSpace is checked in
CloudXRComponent.tsx, add a guard if (!referenceSpace) { onStatusChange?.(false,
'Reference Space Unavailable'); onError?.('Could not obtain an XR reference
space for CloudXR'); onExitImmersiveXRRef.current?.(); return; } and then remove
the corresponding else branch so the remaining logic runs unindented; this keeps
calls to onStatusChange, onError, and onExitImmersiveXRRef the same but
simplifies control flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: df1d61c8-ec50-42c0-8cd5-779b37dff203
📒 Files selected for processing (2)
deps/cloudxr/webxr_client/helpers/react/CloudXRComponent.tsxdeps/cloudxr/webxr_client/src/App.tsx
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
3 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 by CodeRabbit
New Features
Bug Fixes