about summary refs log tree commit diff
path: root/src
diff options
context:
space:
mode:
authordan <dan.abramov@gmail.com>2024-05-03 17:57:09 +0100
committerGitHub <noreply@github.com>2024-05-03 17:57:09 +0100
commit4a2d4253e54f1bf3a375c6c6ffdbd5a9b6bcc24a (patch)
tree870e9113c57708dd3076237d61d838e2889de7f7 /src
parent85b34418ef31247f8d88bdb08248a149192c5b46 (diff)
downloadvoidsky-4a2d4253e54f1bf3a375c6c6ffdbd5a9b6bcc24a.tar.zst
[Session] Align state and global agent switchpoints (#3845)
* Adopt synced accounts unconditionally

* Remove try/catch around resuming session

* Move to login form on resume failure

* Restructure code flow for easier reading

---------

Co-authored-by: Eric Bailey <git@esb.lol>
Diffstat (limited to 'src')
-rw-r--r--src/lib/hooks/useAccountSwitcher.ts12
-rw-r--r--src/screens/Login/ChooseAccountForm.tsx50
-rw-r--r--src/state/session/index.tsx58
3 files changed, 50 insertions, 70 deletions
diff --git a/src/lib/hooks/useAccountSwitcher.ts b/src/lib/hooks/useAccountSwitcher.ts
index 558fcf74b..ad529f912 100644
--- a/src/lib/hooks/useAccountSwitcher.ts
+++ b/src/lib/hooks/useAccountSwitcher.ts
@@ -15,7 +15,7 @@ export function useAccountSwitcher() {
   const [pendingDid, setPendingDid] = useState<string | null>(null)
   const {_} = useLingui()
   const {track} = useAnalytics()
-  const {initSession, clearCurrentAccount} = useSessionApi()
+  const {initSession} = useSessionApi()
   const {requestSwitchToAccount} = useLoggedOutViewControls()
 
   const onPressSwitchAccount = useCallback(
@@ -53,19 +53,11 @@ export function useAccountSwitcher() {
         logger.error(`switch account: selectAccount failed`, {
           message: e.message,
         })
-        clearCurrentAccount() // back user out to login
       } finally {
         setPendingDid(null)
       }
     },
-    [
-      _,
-      track,
-      clearCurrentAccount,
-      initSession,
-      requestSwitchToAccount,
-      pendingDid,
-    ],
+    [_, track, initSession, requestSwitchToAccount, pendingDid],
   )
 
   return {onPressSwitchAccount, pendingDid}
diff --git a/src/screens/Login/ChooseAccountForm.tsx b/src/screens/Login/ChooseAccountForm.tsx
index 098ddeb1d..b02b8e162 100644
--- a/src/screens/Login/ChooseAccountForm.tsx
+++ b/src/screens/Login/ChooseAccountForm.tsx
@@ -39,31 +39,33 @@ export const ChooseAccountForm = ({
         // The session API isn't resilient to race conditions so let's just ignore this.
         return
       }
-      if (account.accessJwt) {
-        if (account.did === currentAccount?.did) {
-          setShowLoggedOut(false)
-          Toast.show(_(msg`Already signed in as @${account.handle}`))
-        } else {
-          try {
-            setPendingDid(account.did)
-            await initSession(account)
-            logEvent('account:loggedIn', {
-              logContext: 'ChooseAccountForm',
-              withPassword: false,
-            })
-            track('Sign In', {resumedSession: true})
-            Toast.show(_(msg`Signed in as @${account.handle}`))
-          } catch (e: any) {
-            logger.error('choose account: initSession failed', {
-              message: e.message,
-            })
-            onSelectAccount(account)
-          } finally {
-            setPendingDid(null)
-          }
-        }
-      } else {
+      if (!account.accessJwt) {
+        // Move to login form.
         onSelectAccount(account)
+        return
+      }
+      if (account.did === currentAccount?.did) {
+        setShowLoggedOut(false)
+        Toast.show(_(msg`Already signed in as @${account.handle}`))
+        return
+      }
+      try {
+        setPendingDid(account.did)
+        await initSession(account)
+        logEvent('account:loggedIn', {
+          logContext: 'ChooseAccountForm',
+          withPassword: false,
+        })
+        track('Sign In', {resumedSession: true})
+        Toast.show(_(msg`Signed in as @${account.handle}`))
+      } catch (e: any) {
+        logger.error('choose account: initSession failed', {
+          message: e.message,
+        })
+        // Move to login form.
+        onSelectAccount(account)
+      } finally {
+        setPendingDid(null)
       }
     },
     [
diff --git a/src/state/session/index.tsx b/src/state/session/index.tsx
index 4894ad696..276e3b97b 100644
--- a/src/state/session/index.tsx
+++ b/src/state/session/index.tsx
@@ -337,35 +337,22 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
       if (isSessionExpired(account)) {
         logger.debug(`session: attempting to resume using previous session`)
 
-        try {
-          const freshAccount = await resumeSessionWithFreshAccount()
-          __globalAgent = agent
-          await fetchingGates
-          setState(s => {
-            return {
-              accounts: [
-                freshAccount,
-                ...s.accounts.filter(a => a.did !== freshAccount.did),
-              ],
-              currentAgentState: {
-                did: freshAccount.did,
-                agent: agent,
-              },
-              needsPersist: true,
-            }
-          })
-        } catch (e) {
-          /*
-           * Note: `agent.persistSession` is also called when this fails, and
-           * we handle that failure via `createPersistSessionHandler`
-           */
-          logger.info(`session: resumeSessionWithFreshAccount failed`, {
-            message: e,
-          })
-
-          __globalAgent = PUBLIC_BSKY_AGENT
-          // TODO: This needs a setState.
-        }
+        const freshAccount = await resumeSessionWithFreshAccount()
+        __globalAgent = agent
+        await fetchingGates
+        setState(s => {
+          return {
+            accounts: [
+              freshAccount,
+              ...s.accounts.filter(a => a.did !== freshAccount.did),
+            ],
+            currentAgentState: {
+              did: freshAccount.did,
+              agent: agent,
+            },
+            needsPersist: true,
+          }
+        })
       } else {
         logger.debug(`session: attempting to reuse previous session`)
 
@@ -480,6 +467,11 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
       const persistedSession = persisted.get('session')
 
       logger.debug(`session: persisted onUpdate`, {})
+      setState(s => ({
+        accounts: persistedSession.accounts,
+        currentAgentState: s.currentAgentState,
+        needsPersist: false, // Synced from another tab. Don't persist to avoid cycles.
+      }))
 
       const selectedAccount = persistedSession.accounts.find(
         a => a.did === persistedSession.currentAccount?.did,
@@ -531,15 +523,9 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
             did: undefined,
             agent: PUBLIC_BSKY_AGENT,
           },
-          needsPersist: true, // TODO: This seems bad in this codepath. Existing behavior.
+          needsPersist: false, // Synced from another tab. Don't persist to avoid cycles.
         }))
       }
-
-      setState(s => ({
-        accounts: persistedSession.accounts,
-        currentAgentState: s.currentAgentState,
-        needsPersist: false, // Synced from another tab. Don't persist to avoid cycles.
-      }))
     })
   }, [state, setState, initSession])