fix[tauri]: ability to actually log out of mobile app#2145
fix[tauri]: ability to actually log out of mobile app#2145peterchinman merged 7 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request adds Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/core/auth/logout.ts`:
- Around line 39-48: The code contains a stray period after the opening brace in
the conditional that causes a parse error; remove the extraneous '.' so the if
statement reads if (isNativeMobilePlatform()) { ... } and keep the inner logic
unchanged (the fetch call with credentials/mode/redirect, catch, and
navigate('/login') for isNativeMobilePlatform(), and the window.location.href =
SERVER_HOSTS['auth-logout'] fallback) to restore valid syntax around
isNativeMobilePlatform(), fetch, navigate and SERVER_HOSTS['auth-logout'].
- Around line 40-45: Add a short inline comment above the fire-and-forget logout
fetch to explain why we're using mode: 'no-cors' and swallowing errors (i.e., we
don't want network failures to block client-side navigation), referencing the
fetch to SERVER_HOSTS['auth-logout'] and the subsequent navigate('/login') call;
keep it concise and mention that the request is intentionally non-blocking and
errors are ignored to ensure immediate redirect to login.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f1e03c28-8e3e-4071-b5e5-8be40388c839
⛔ Files ignored due to path filters (1)
js/app/tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
js/app/packages/app/component/auth/LoginOptions.tsxjs/app/packages/core/auth/logout.ts
| await fetch(SERVER_HOSTS['auth-logout'], { | ||
| credentials: 'include', | ||
| mode: 'no-cors', | ||
| redirect: 'manual', | ||
| }).catch(() => {}); | ||
| navigate('/login'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a brief comment explaining the fire-and-forget pattern.
The no-cors mode with swallowed errors is intentional for logout, but a brief comment would help future maintainers understand why errors are ignored.
📝 Suggested documentation
if (isNativeMobilePlatform()) {
+ // Fire-and-forget: clear server session; navigate regardless of success
await fetch(SERVER_HOSTS['auth-logout'], {
credentials: 'include',
mode: 'no-cors',
redirect: 'manual',
}).catch(() => {});
navigate('/login');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/core/auth/logout.ts` around lines 40 - 45, Add a short inline
comment above the fire-and-forget logout fetch to explain why we're using mode:
'no-cors' and swallowing errors (i.e., we don't want network failures to block
client-side navigation), referencing the fetch to SERVER_HOSTS['auth-logout']
and the subsequent navigate('/login') call; keep it concise and mention that the
request is intentionally non-blocking and errors are ignored to ensure immediate
redirect to login.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/core/auth/logout.ts`:
- Line 36: The call to undefined identifiers track and TrackingEvents in
logout.ts is causing the build failure; either remove the redundant
track(TrackingEvents.AUTH.LOGOUT) call (since analytics.track('sign_out') is
already called) or replace it by invoking the existing analytics instance from
useAnalytics() (e.g., use the analytics returned by useAnalytics() to send the
AUTH.LOGOUT event) so you no longer reference the undefined track/TrackingEvents
symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 92d20849-82c8-43d7-8fc3-60fb4279b6e0
📒 Files selected for processing (1)
js/app/packages/core/auth/logout.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/core/auth/logout.ts`:
- Around line 36-42: In logout.ts inside the isNativeMobilePlatform() branch,
remove the awaiting of the fetch to SERVER_HOSTS['auth-logout'] so the network
call becomes fire-and-forget; call fetch(...) without await (keeping the
.catch(() => {}) to swallow errors) and immediately call navigate('/login') so
navigation is not blocked by a stalled request in the logout function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4f0f8e45-af43-42a0-8ff2-f6b48a027c6c
📒 Files selected for processing (1)
js/app/packages/core/auth/logout.ts
| if (isNativeMobilePlatform()) { | ||
| await fetch(SERVER_HOSTS['auth-logout'], { | ||
| credentials: 'include', | ||
| mode: 'no-cors', | ||
| redirect: 'manual', | ||
| }).catch(() => {}); | ||
| navigate('/login'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that native logout currently gates navigation on awaited fetch.
# Expected: a match showing `await fetch(...)` before `navigate('/login')`.
rg -nPU -C2 "if \(isNativeMobilePlatform\(\)\) \{\n\s*await fetch\(SERVER_HOSTS\['auth-logout'\]," js/app/packages/core/auth/logout.ts
rg -nPU -C2 "await fetch\(SERVER_HOSTS\['auth-logout'\],[\s\S]*?\)\.catch\(\(\) => \{\}\);\n\s*navigate\('/login'\);" js/app/packages/core/auth/logout.tsRepository: macro-inc/macro
Length of output: 566
Make logout navigation fire-and-forget instead of awaiting the network request.
The native mobile logout currently gates the /login navigation on an awaited fetch to the auth-logout endpoint. If that request stalls, the logout UX is delayed even though the error is silently caught and ignored. Remove the await to unblock navigation and provide immediate feedback to the user.
Suggested change
if (isNativeMobilePlatform()) {
- await fetch(SERVER_HOSTS['auth-logout'], {
+ void fetch(SERVER_HOSTS['auth-logout'], {
credentials: 'include',
mode: 'no-cors',
redirect: 'manual',
}).catch(() => {});
navigate('/login');📝 Committable suggestion
‼️ 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.
| if (isNativeMobilePlatform()) { | |
| await fetch(SERVER_HOSTS['auth-logout'], { | |
| credentials: 'include', | |
| mode: 'no-cors', | |
| redirect: 'manual', | |
| }).catch(() => {}); | |
| navigate('/login'); | |
| if (isNativeMobilePlatform()) { | |
| void fetch(SERVER_HOSTS['auth-logout'], { | |
| credentials: 'include', | |
| mode: 'no-cors', | |
| redirect: 'manual', | |
| }).catch(() => {}); | |
| navigate('/login'); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/core/auth/logout.ts` around lines 36 - 42, In logout.ts
inside the isNativeMobilePlatform() branch, remove the awaiting of the fetch to
SERVER_HOSTS['auth-logout'] so the network call becomes fire-and-forget; call
fetch(...) without await (keeping the .catch(() => {}) to swallow errors) and
immediately call navigate('/login') so navigation is not blocked by a stalled
request in the logout function.
1 participant
No description provided.