fix: Add error handling for OAuth desktop redirect failures #297
Conversation
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
- Add 5-second timeout to detect failed deep link redirects - Display error UI with manual fallback options when redirect fails - Provide copyable deep link and manual 'Open Maple App' button - Add 'Back to Login' button for user recovery - Fixes issue where users were stuck on success page indefinitely Fixes #294 Co-authored-by: Anthony <AnthonyRonning@users.noreply.github.com>
WalkthroughThe PR enhances OAuth callback error handling in the desktop Tauri flow by adding detection for redirect failures. A 5-second timer flags when redirection doesn't complete, triggering fallback UI that allows users to manually open the Maple app via a stored deep link URL. Changes
Sequence DiagramsequenceDiagram
participant User
participant WebCallback as Web Callback Page
participant Tauri as Tauri App
participant Timer as 5s Timeout
User->>WebCallback: Completes OAuth
WebCallback->>WebCallback: handleSuccessfulAuth triggered
rect rgb(200, 220, 255)
note over WebCallback: New: Initialize timer & redirect attempt
WebCallback->>WebCallback: Store deepLinkUrl in state
WebCallback->>Tauri: Attempt redirect to native app
WebCallback->>Timer: Start 5-second timer
end
alt Redirect Succeeds
Tauri->>User: Opens in Maple app (before timer fires)
else Redirect Fails (timer fires)
Timer->>WebCallback: Timeout reached
WebCallback->>WebCallback: Set redirectFailed = true
rect rgb(255, 220, 200)
note over WebCallback: New: Fallback UI shown
WebCallback->>User: Display "Redirect Failed" with manual link
end
User->>User: Manually opens deep link
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-07-19T21:31:44.925ZApplied to files:
📚 Learning: 2025-07-19T21:31:44.925ZApplied to files:
🧬 Code graph analysis (1)frontend/src/routes/auth.$provider.callback.tsx (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Adds timeout-based error detection for OAuth desktop redirect failures. When the deep link redirect fails silently, users now see an error UI after 5 seconds with manual fallback options.
Key changes:
- Added 5-second timeout to detect when deep link redirect doesn't navigate away from page
- New error UI displays with manual "Open Maple App" button and copyable deep link
- Added "Back to Login" recovery option
- Stores deep link URL in state for manual fallback
Issues found:
- Critical: Timer cleanup missing - if redirect succeeds, the 5s timer still fires and calls state setters on unmounted component
- Critical: UI conditional logic bug -
localStorageflag is removed before timeout completes, causing UI to switch from Tauri success screen back to web flow prematurely
Confidence Score: 2/5
- This PR has critical logic bugs that will cause runtime errors and incorrect UI behavior
- The implementation addresses the core issue but introduces two critical bugs: (1) missing timer cleanup leads to calling state setters on unmounted components, and (2) the localStorage flag is cleared before checking it in the UI conditional, breaking the success screen display
- frontend/src/routes/auth.$provider.callback.tsx requires fixes for timer cleanup and state management logic
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| frontend/src/routes/auth.$provider.callback.tsx | 2/5 | Added timeout-based error handling for failed OAuth deep link redirects, but has critical logic bugs with state management and timer cleanup |
Sequence Diagram
sequenceDiagram
participant User
participant TauriApp as Tauri App
participant Browser as Web Browser
participant OAuthProvider
participant CallbackPage
User->>TauriApp: Start OAuth flow
TauriApp->>Browser: Open web OAuth
Browser->>OAuthProvider: Request authentication
User->>OAuthProvider: Complete login
OAuthProvider->>CallbackPage: Return with code
CallbackPage->>CallbackPage: Process callback
CallbackPage->>CallbackPage: Start 5s timer
alt Redirect succeeds
CallbackPage->>TauriApp: Deep link redirect
TauriApp->>TauriApp: Handle redirect
Note over CallbackPage: Timer still active
else Redirect fails
CallbackPage->>CallbackPage: Timer expires
CallbackPage->>User: Show error UI
User->>CallbackPage: Manual retry
CallbackPage->>TauriApp: Retry redirect
end
1 file reviewed, 2 comments
Comment on lines
+61
to
+64
| const redirectTimer = setTimeout(() => { | ||
| setRedirectFailed(true); | ||
| setIsProcessing(false); | ||
| }, 5000); |
Contributor
There was a problem hiding this comment.
logic: The redirectTimer is never cleaned up if the redirect succeeds and the page unloads. If the deep link works, the timer will still fire 5 seconds later (potentially in the background), calling state setters on an unmounted component.
Suggested change
| const redirectTimer = setTimeout(() => { | |
| setRedirectFailed(true); | |
| setIsProcessing(false); | |
| }, 5000); | |
| // Set a timeout to detect if the redirect failed | |
| // If the user is still on this page after 5 seconds, show error | |
| const redirectTimer = setTimeout(() => { | |
| if (document.visibilityState === 'visible') { | |
| setRedirectFailed(true); | |
| setIsProcessing(false); | |
| } | |
| }, 5000); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/auth.$provider.callback.tsx
Line: 61:64
Comment:
**logic:** The `redirectTimer` is never cleaned up if the redirect succeeds and the page unloads. If the deep link works, the timer will still fire 5 seconds later (potentially in the background), calling state setters on an unmounted component.
```suggestion
// Set a timeout to detect if the redirect failed
// If the user is still on this page after 5 seconds, show error
const redirectTimer = setTimeout(() => {
if (document.visibilityState === 'visible') {
setRedirectFailed(true);
setIsProcessing(false);
}
}, 5000);
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| // If this is a Tauri app auth flow (desktop or mobile), show a different UI | ||
| if (localStorage.getItem("redirect-to-native") === "true") { | ||
| if (localStorage.getItem("redirect-to-native") === "true" || redirectFailed) { |
Contributor
There was a problem hiding this comment.
logic: This condition checks localStorage on every render. Since localStorage.removeItem("redirect-to-native") is called on line 43, this will be false after the first execution, causing the UI to switch from the Tauri success screen back to the web flow screen before the 5-second timer completes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/auth.$provider.callback.tsx
Line: 185:185
Comment:
**logic:** This condition checks `localStorage` on every render. Since `localStorage.removeItem("redirect-to-native")` is called on line 43, this will be `false` after the first execution, causing the UI to switch from the Tauri success screen back to the web flow screen before the 5-second timer completes.
How can I resolve this? If you propose a fix, please make it concise.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #294
Summary
Adds error handling to the OAuth desktop redirect flow for Tauri apps. Previously, if the deep link redirect failed, users would be stuck on the success page indefinitely.
Changes
Testing
Generated with Claude Code
Summary by CodeRabbit