[Persisted] Fix the race condition causing clobbered writes between tabs (#4873)

* Broadcast the update in the same tick

The motivation for the original code is unclear. I was not able to reproduce the described behavior and have not seen it mentioned on the web. I'll assume that this was a misunderstanding.

* Remove defensive programming

The only places in this code that we can expect to throw are schema.parse(), JSON.parse(), JSON.stringify(), and localStorage.getItem/setItem/removeItem. Let's push try/catch'es where we expect them to be necessary.

* Don't write or clobber defaults

Writing defaults to local storage is unnecessary. We would write them as a part of next update anyway. So I'm removing that to reduce the number of moving pieces.

However, we do need to be wary of _state being set to defaults. Because _state gets mutated on write. We don't want to mutate the defaults object. To avoid having to think about this, let's copy on write. We don't write to this object very often.

* Refactor: extract tryParse

* Refactor: move string parsing into tryParse

* Extract tryStringify, split logging by platform

Shared data parsing/stringification errors are always logged. Storage errors are only logged on native because we trust the web APIs to work.

* Add a layer of caching to readFromStorage to web

We're going to be doing a read on every write so let's add a fast path that avoids parsing and validating.

* Fix the race condition causing clobbered writes between tabs

1 files changed, 33 insertions, 41 deletions

diff --git a/src/state/persisted/index.ts b/src/state/persisted/index.ts
index 639e4e47f..95f814850 100644
--- a/src/state/persisted/index.ts
+++ b/src/state/persisted/index.ts
@@ -1,7 +1,12 @@
 import AsyncStorage from '@react-native-async-storage/async-storage'
 
 import {logger} from '#/logger'
-import {defaults, Schema, schema} from '#/state/persisted/schema'
+import {
+  defaults,
+  Schema,
+  tryParse,
+  tryStringify,
+} from '#/state/persisted/schema'
 import {PersistedApi} from './types'
 
 export type {PersistedAccount, Schema} from '#/state/persisted/schema'
@@ -12,16 +17,9 @@ const BSKY_STORAGE = 'BSKY_STORAGE'
 let _state: Schema = defaults
 
 export async function init() {
-  try {
-    const stored = await readFromStorage()
-    if (!stored) {
-      await writeToStorage(defaults)
-    }
-    _state = stored || defaults
-  } catch (e) {
-    logger.error('persisted state: failed to load root state from storage', {
-      message: e,
-    })
+  const stored = await readFromStorage()
+  if (stored) {
+    _state = stored
   }
 }
 init satisfies PersistedApi['init']
@@ -35,14 +33,11 @@ export async function write<K extends keyof Schema>(
   key: K,
   value: Schema[K],
 ): Promise<void> {
-  try {
-    _state[key] = value
-    await writeToStorage(_state)
-  } catch (e) {
-    logger.error(`persisted state: failed writing root state to storage`, {
-      message: e,
-    })
+  _state = {
+    ..._state,
+    [key]: value,
   }
+  await writeToStorage(_state)
 }
 write satisfies PersistedApi['write']
 
@@ -61,31 +56,28 @@ export async function clearStorage() {
 clearStorage satisfies PersistedApi['clearStorage']
 
 async function writeToStorage(value: Schema) {
-  schema.parse(value)
-  await AsyncStorage.setItem(BSKY_STORAGE, JSON.stringify(value))
+  const rawData = tryStringify(value)
+  if (rawData) {
+    try {
+      await AsyncStorage.setItem(BSKY_STORAGE, rawData)
+    } catch (e) {
+      logger.error(`persisted state: failed writing root state to storage`, {
+        message: e,
+      })
+    }
+  }
 }
 
 async function readFromStorage(): Promise<Schema | undefined> {
-  const rawData = await AsyncStorage.getItem(BSKY_STORAGE)
-  const objData = rawData ? JSON.parse(rawData) : undefined
-
-  // new user
-  if (!objData) return undefined
-
-  // existing user, validate
-  const parsed = schema.safeParse(objData)
-
-  if (parsed.success) {
-    return objData
-  } else {
-    const errors =
-      parsed.error?.errors?.map(e => ({
-        code: e.code,
-        // @ts-ignore exists on some types
-        expected: e?.expected,
-        path: e.path?.join('.'),
-      })) || []
-    logger.error(`persisted store: data failed validation on read`, {errors})
-    return undefined
+  let rawData: string | null = null
+  try {
+    rawData = await AsyncStorage.getItem(BSKY_STORAGE)
+  } catch (e) {
+    logger.error(`persisted state: failed reading root state from storage`, {
+      message: e,
+    })
+  }
+  if (rawData) {
+    return tryParse(rawData)
   }
 }