Improvements to persisted state migration (#2098)

* Fix session email/emailConfirmed types, update usage for safer access

* Handle fallback better, better errors

* Better handling, add test

* Add test for default data

* Remove fallback, not needed, update logs

src/state/persisted/__tests__/legacy.test.ts

@@ -0,0 +1,13 @@
+import {expect, test} from '@jest/globals'
+
+import {transform} from '#/state/persisted/legacy'
+import {defaults, schema} from '#/state/persisted/schema'
+
+test('defaults', () => {
+  expect(() => schema.parse(defaults)).not.toThrow()
+})
+
+test('transform', () => {
+  const data = transform({})
+  expect(() => schema.parse(data)).not.toThrow()
+})

src/state/persisted/index.ts

@@ -26,7 +26,10 @@ export async function init() {
   try {
     await migrate() // migrate old store
     const stored = await store.read() // check for new store
-    if (!stored) await store.write(defaults) // opt: init new store
+    if (!stored) {
+      logger.info('persisted state: initializing default storage')
+      await store.write(defaults) // opt: init new store
+    }
     _state = stored || defaults // return new store
     logger.log('persisted state: initialized')
   } catch (e) {

src/state/persisted/legacy.ts

@@ -1,7 +1,7 @@
 import AsyncStorage from '@react-native-async-storage/async-storage'
 
 import {logger} from '#/logger'
-import {defaults, Schema} from '#/state/persisted/schema'
+import {defaults, Schema, schema} from '#/state/persisted/schema'
 import {write, read} from '#/state/persisted/store'
 
 /**
@@ -66,7 +66,6 @@ type LegacySchema = {
 
 const DEPRECATED_ROOT_STATE_STORAGE_KEY = 'root'
 
-// TODO remove, assume that partial data may be here during our refactor
 export function transform(legacy: Partial<LegacySchema>): Schema {
   return {
     colorMode: legacy.shell?.colorMode || defaults.colorMode,
@@ -116,7 +115,7 @@ export function transform(legacy: Partial<LegacySchema>): Schema {
  * local storage AND old storage exists.
  */
 export async function migrate() {
-  logger.info('persisted state: migrate')
+  logger.info('persisted state: check need to migrate')
 
   try {
     const rawLegacyData = await AsyncStorage.getItem(
@@ -138,6 +137,7 @@ export async function migrate() {
           ),
         })
         logger.info(`persisted state: debug new data`, {
+          hasNewData: Boolean(newData),
           hasExistingLoggedInAccount: Boolean(newData?.session?.currentAccount),
           numberOfExistingAccounts: newData?.session?.accounts?.length,
           existingAccountMatchesLegacy: Boolean(
@@ -145,27 +145,32 @@ export async function migrate() {
               legacy?.session?.data?.did,
           ),
         })
-      } else {
-        logger.info(`persisted state: no legacy to debug, fresh install`)
       }
-    } catch (e) {
+    } catch (e: any) {
-      logger.error(`persisted state: legacy debugging failed`, {error: e})
+      logger.error(e, {message: `persisted state: legacy debugging failed`})
     }
 
     if (!alreadyMigrated && rawLegacyData) {
       logger.info('persisted state: migrating legacy storage')
+
       const legacyData = JSON.parse(rawLegacyData)
       const newData = transform(legacyData)
-      await write(newData)
+      const validate = schema.safeParse(newData)
-      // track successful migrations
+
-      logger.log('persisted state: migrated legacy storage')
+      if (validate.success) {
+        await write(newData)
+        logger.log('persisted state: migrated legacy storage')
+      } else {
+        logger.error('persisted state: legacy data failed validation', {
+          error: validate.error,
+        })
+      }
     } else {
-      // track successful migrations
       logger.log('persisted state: no migration needed')
     }
-  } catch (e) {
+  } catch (e: any) {
-    logger.error('persisted state: error migrating legacy storage', {
+    logger.error(e, {
-      error: String(e),
+      message: 'persisted state: error migrating legacy storage',
     })
   }
 }

src/state/persisted/schema.ts

@@ -2,17 +2,14 @@ import {z} from 'zod'
 import {deviceLocales} from '#/platform/detection'
 
 // only data needed for rendering account page
-// TODO agent.resumeSession requires the following fields
 const accountSchema = z.object({
   service: z.string(),
   did: z.string(),
   handle: z.string(),
-  email: z.string(),
+  email: z.string().optional(),
-  emailConfirmed: z.boolean(),
+  emailConfirmed: z.boolean().optional(),
   refreshJwt: z.string().optional(), // optional because it can expire
   accessJwt: z.string().optional(), // optional because it can expire
-  // displayName: z.string().optional(),
-  // aviUrl: z.string().optional(),
 })
 export type PersistedAccount = z.infer<typeof accountSchema>
 

src/state/session/index.tsx

@@ -245,7 +245,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
         service: agent.service.toString(),
         did: agent.session.did,
         handle: agent.session.handle,
-        email: agent.session.email!, // TODO this is always defined?
+        email: agent.session.email,
         emailConfirmed: agent.session.emailConfirmed || false,
         refreshJwt: agent.session.refreshJwt,
         accessJwt: agent.session.accessJwt,
@@ -342,7 +342,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
         service: agent.service.toString(),
         did: agent.session.did,
         handle: agent.session.handle,
-        email: agent.session.email!, // TODO this is always defined?
+        email: agent.session.email,
         emailConfirmed: agent.session.emailConfirmed || false,
         refreshJwt: agent.session.refreshJwt,
         accessJwt: agent.session.accessJwt,

src/view/com/modals/ChangeEmail.tsx

@@ -118,8 +118,8 @@ export function Component() {
           ) : stage === Stages.ConfirmCode ? (
             <Trans>
               An email has been sent to your previous address,{' '}
-              {currentAccount?.email || ''}. It includes a confirmation code
+              {currentAccount?.email || '(no email)'}. It includes a
-              which you can enter below.
+              confirmation code which you can enter below.
             </Trans>
           ) : (
             <Trans>

src/view/com/modals/VerifyEmail.tsx

@@ -108,8 +108,8 @@ export function Component({showReminder}: {showReminder?: boolean}) {
             </Trans>
           ) : stage === Stages.ConfirmCode ? (
             <Trans>
-              An email has been sent to {currentAccount?.email || ''}. It
+              An email has been sent to {currentAccount?.email || '(no email)'}.
-              includes a confirmation code which you can enter below.
+              It includes a confirmation code which you can enter below.
             </Trans>
           ) : (
             ''
@@ -125,7 +125,7 @@ export function Component({showReminder}: {showReminder?: boolean}) {
                 size={16}
               />
               <Text type="xl-medium" style={[pal.text, s.flex1, {minWidth: 0}]}>
-                {currentAccount?.email || ''}
+                {currentAccount?.email || '(no email)'}
               </Text>
             </View>
             <Pressable

src/view/screens/Settings.tsx

@@ -299,7 +299,7 @@ export function SettingsScreen({}: Props) {
                 </>
               )}
               <Text type="lg" style={pal.text}>
-                {currentAccount.email}{' '}
+                {currentAccount.email || '(no email)'}{' '}
               </Text>
               <Link onPress={() => openModal({name: 'change-email'})}>
                 <Text type="lg" style={pal.link}>

src/view/shell/desktop/RightNav.tsx

@@ -58,8 +58,8 @@ export function DesktopRightNav() {
                 type="md"
                 style={pal.link}
                 href={FEEDBACK_FORM_URL({
-                  email: currentAccount!.email,
+                  email: currentAccount?.email,
-                  handle: currentAccount!.handle,
+                  handle: currentAccount?.handle,
                 })}
                 text={_(msg`Feedback`)}
               />