Conversation
WalkthroughThe logout function in the authentication module was refactored to execute multiple asynchronous cleanup operations concurrently in the background, each with individual error handling. The main logout steps now proceed without waiting for these background tasks. A 500ms delay was introduced before updating the UI to ensure a smoother transition. Changes
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Execute logout tasks in background by refactoring the logout function in features/authentication/use-logout.ts to run cleanup operations without awaiting completionThe logout function in features/authentication/use-logout.ts changes from a blocking approach that awaits all cleanup operations to a non-blocking approach that executes cleanup tasks in the background. The function removes 📍Where to StartStart with the logout function in features/authentication/use-logout.ts. Macroscope summarized ce301fc. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
features/authentication/use-logout.ts (2)
64-93: Consider using consistent error handling pattern.While functionally correct, this IIFE pattern with internal try-catch differs from the
.catch()chaining used in other background operations. Consider refactoring for consistency.- // Unlink identities from device - void (async () => { - try { - const currentUser = getCurrentUserQueryData() - if (!currentUser) { - return - } - const currentDevice = await ensureUserDeviceQueryData({ - userId: currentUser.id, - }) - const currentUserIdentities = await ensureUserIdentitiesQueryData({ - userId: currentUser.id, - }) - await Promise.all( - currentUserIdentities.map((identity) => - unlinkIdentityFromDeviceMutation({ - identityId: identity.id, - deviceId: currentDevice.id, - }), - ), - ) - } catch (error) { - captureError( - new GenericError({ - error, - additionalMessage: "Error unlinking identities from device", - }), - ) - } - })() + // Unlink identities from device + void (async () => { + const currentUser = getCurrentUserQueryData() + if (!currentUser) { + return + } + const currentDevice = await ensureUserDeviceQueryData({ + userId: currentUser.id, + }) + const currentUserIdentities = await ensureUserIdentitiesQueryData({ + userId: currentUser.id, + }) + await Promise.all( + currentUserIdentities.map((identity) => + unlinkIdentityFromDeviceMutation({ + identityId: identity.id, + deviceId: currentDevice.id, + }), + ), + ) + })().catch((error: unknown) => { + captureError( + new GenericError({ + error, + additionalMessage: "Error unlinking identities from device", + }), + ) + })
157-161: Consider making the transition delay configurable.The 500ms delay works well for smooth transitions, but consider making it configurable or adaptive based on device performance for optimal UX across different devices.
+ // Configurable delay for smooth transition + const LOGOUT_TRANSITION_DELAY_MS = 500 + // Give a small delay before hiding the overlay to ensure smooth transition setTimeout(() => { useAppStore.getState().actions.setIsShowingFullScreenOverlay(false) useAppStore.getState().actions.setIsLoggingOut(false) - }, 500) + }, LOGOUT_TRANSITION_DELAY_MS)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
features/authentication/use-logout.ts(1 hunks)
🔇 Additional comments (6)
features/authentication/use-logout.ts (6)
37-52: LGTM! Good use of fire-and-forget pattern.The refactoring to execute unsubscribe operations in the background is well-implemented with proper error handling.
54-62: Pattern consistency maintained.
95-107: Consistent implementation.
149-155: Excellent UX optimization!Setting auth status and clearing stores immediately provides instant user feedback while background cleanup continues. This achieves the goal of a faster perceived logout experience.
163-169: Good defensive error handling!Adding UI state reset in the catch block ensures the app doesn't get stuck in a loading state if logout fails.
32-170: Verify behavior when user logs in quickly after logout.With background tasks continuing after logout, there's a potential for race conditions if a user logs out and immediately logs back in. The background cleanup tasks from the previous session might interfere with the new session.
Consider testing the following scenario:
- User initiates logout
- User immediately logs back in (within ~1-2 seconds)
- Verify that background tasks from the previous session don't interfere with the new session
This is particularly important for operations like:
- Push notification registration/unregistration
- XMTP client logout/login
- Device identity linking/unlinking
2 participants
I refactored the
logoutfunction withinfeatures/authentication/use-logout.tsto execute cleanup operations in the background, without waiting for their completion.Here's what I changed:
awaitkeyword from all asynchronous cleanup operations, including calls tounsubscribeFromAllConversationsNotifications,stopStreaming,unlinkIdentityFromDeviceMutation,unregisterPushNotifications,unregisterBackgroundSyncTask,logoutXmtpClient, andclearTurnkeySessions. This allows these tasks to run in parallel without blocking the main thread.customPromiseAllSettledutility, as individual promises are now fired off independently.voidoperator before each background promise (e.g.,void Promise.all(...),void stopStreaming(...)). This explicitly signals that the promise's return value is intentionally not awaited or handled directly, preventing linting warnings about unhandled promises while clearly stating the "fire-and-forget" intent..catch()blocks for each background operation. This ensures that any errors occurring during these non-blocking tasks are still captured and logged viacaptureError, maintaining error visibility without blocking the logout flow.clearImageCachecall from thefinallyblock into thetryblock and made it a background operation usingvoid.useAuthenticationStore.getState().actions.setStatus("signedOut")) and resetting various stores (useMultiInboxStore,useNotificationsStore,clearReacyQueryQueriesAndCache) now occur immediately after the background tasks are initiated, providing a faster perceived logout experience.setTimeoutwith a 500ms delay before hiding the full-screen overlay and settingisLoggingOuttofalse. This ensures a smoother visual transition for the user, preventing an abrupt disappearance of the loading state.The outcome is a significantly faster and more responsive logout process, as the user is immediately signed out while resource cleanup continues efficiently in the background. Errors from background tasks are still logged, ensuring operational visibility.
Summary by CodeRabbit