about summary refs log tree commit diff
diff options
context:
space:
mode:
authordan <dan.abramov@gmail.com>2024-08-13 18:51:49 +0100
committerGitHub <noreply@github.com>2024-08-13 18:51:49 +0100
commit57be2ea15b5bea019abf95a590640d688b7a8633 (patch)
tree6bde2e03aa3ab4e49a24a1ed8729ab771328b413
parent7e11b862e931b5351bd3463d984ab11ee9b46522 (diff)
downloadvoidsky-57be2ea15b5bea019abf95a590640d688b7a8633.tar.zst
Don't kick to login screen on network error (#4911)
* Don't kick the user on network errors

* Track online status for RQ

* Use health endpoint

* Update test with new behavior

* Only poll while offline

* Handle races between the check and network events

* Reduce the poll kickoff interval

* Don't cache partially fetched pinned feeds

This isn't a new issue but it's more prominent with the offline handling. We're currently silently caching pinned infos that failed to fetch. This avoids showing a big spinner on failure but it also kills all feeds which is very confusing. If the request to get feed gens fails, let's fail the whole query.

Then it can be retried.
-rw-r--r--src/lib/react-query.tsx67
-rw-r--r--src/state/events.ts16
-rw-r--r--src/state/queries/feed.ts3
-rw-r--r--src/state/session/__tests__/session-test.ts10
-rw-r--r--src/state/session/agent.ts27
-rw-r--r--src/state/session/reducer.ts8
6 files changed, 117 insertions, 14 deletions
diff --git a/src/lib/react-query.tsx b/src/lib/react-query.tsx
index be507216a..5abfccd7f 100644
--- a/src/lib/react-query.tsx
+++ b/src/lib/react-query.tsx
@@ -2,18 +2,83 @@ import React, {useRef, useState} from 'react'
 import {AppState, AppStateStatus} from 'react-native'
 import AsyncStorage from '@react-native-async-storage/async-storage'
 import {createAsyncStoragePersister} from '@tanstack/query-async-storage-persister'
-import {focusManager, QueryClient} from '@tanstack/react-query'
+import {focusManager, onlineManager, QueryClient} from '@tanstack/react-query'
 import {
   PersistQueryClientProvider,
   PersistQueryClientProviderProps,
 } from '@tanstack/react-query-persist-client'
 
 import {isNative} from '#/platform/detection'
+import {listenNetworkConfirmed, listenNetworkLost} from '#/state/events'
 
 // any query keys in this array will be persisted to AsyncStorage
 export const labelersDetailedInfoQueryKeyRoot = 'labelers-detailed-info'
 const STORED_CACHE_QUERY_KEY_ROOTS = [labelersDetailedInfoQueryKeyRoot]
 
+async function checkIsOnline(): Promise<boolean> {
+  try {
+    const controller = new AbortController()
+    setTimeout(() => {
+      controller.abort()
+    }, 15e3)
+    const res = await fetch('https://public.api.bsky.app/xrpc/_health', {
+      cache: 'no-store',
+      signal: controller.signal,
+    })
+    const json = await res.json()
+    if (json.version) {
+      return true
+    } else {
+      return false
+    }
+  } catch (e) {
+    return false
+  }
+}
+
+let receivedNetworkLost = false
+let receivedNetworkConfirmed = false
+let isNetworkStateUnclear = false
+
+listenNetworkLost(() => {
+  receivedNetworkLost = true
+  onlineManager.setOnline(false)
+})
+
+listenNetworkConfirmed(() => {
+  receivedNetworkConfirmed = true
+  onlineManager.setOnline(true)
+})
+
+let checkPromise: Promise<void> | undefined
+function checkIsOnlineIfNeeded() {
+  if (checkPromise) {
+    return
+  }
+  receivedNetworkLost = false
+  receivedNetworkConfirmed = false
+  checkPromise = checkIsOnline().then(nextIsOnline => {
+    checkPromise = undefined
+    if (nextIsOnline && receivedNetworkLost) {
+      isNetworkStateUnclear = true
+    }
+    if (!nextIsOnline && receivedNetworkConfirmed) {
+      isNetworkStateUnclear = true
+    }
+    if (!isNetworkStateUnclear) {
+      onlineManager.setOnline(nextIsOnline)
+    }
+  })
+}
+
+setInterval(() => {
+  if (AppState.currentState === 'active') {
+    if (!onlineManager.isOnline() || isNetworkStateUnclear) {
+      checkIsOnlineIfNeeded()
+    }
+  }
+}, 2000)
+
 focusManager.setEventListener(onFocus => {
   if (isNative) {
     const subscription = AppState.addEventListener(
diff --git a/src/state/events.ts b/src/state/events.ts
index 1384abded..dcd36464e 100644
--- a/src/state/events.ts
+++ b/src/state/events.ts
@@ -22,6 +22,22 @@ export function listenSessionDropped(fn: () => void): UnlistenFn {
   return () => emitter.off('session-dropped', fn)
 }
 
+export function emitNetworkConfirmed() {
+  emitter.emit('network-confirmed')
+}
+export function listenNetworkConfirmed(fn: () => void): UnlistenFn {
+  emitter.on('network-confirmed', fn)
+  return () => emitter.off('network-confirmed', fn)
+}
+
+export function emitNetworkLost() {
+  emitter.emit('network-lost')
+}
+export function listenNetworkLost(fn: () => void): UnlistenFn {
+  emitter.on('network-lost', fn)
+  return () => emitter.off('network-lost', fn)
+}
+
 export function emitPostCreated() {
   emitter.emit('post-created')
 }
diff --git a/src/state/queries/feed.ts b/src/state/queries/feed.ts
index 2b6751e89..e5ce19a9a 100644
--- a/src/state/queries/feed.ts
+++ b/src/state/queries/feed.ts
@@ -454,7 +454,8 @@ export function usePinnedFeedsInfos() {
           }),
       )
 
-      await Promise.allSettled([feedsPromise, ...listsPromises])
+      await feedsPromise // Fail the whole query if it fails.
+      await Promise.allSettled(listsPromises) // Ignore individual failing ones.
 
       // order the feeds/lists in the order they were pinned
       const result: SavedFeedSourceInfo[] = []
diff --git a/src/state/session/__tests__/session-test.ts b/src/state/session/__tests__/session-test.ts
index 731b66b0e..cb4c6a35b 100644
--- a/src/state/session/__tests__/session-test.ts
+++ b/src/state/session/__tests__/session-test.ts
@@ -1184,7 +1184,7 @@ describe('session', () => {
     expect(state.currentAgentState.did).toBe('bob-did')
   })
 
-  it('does soft logout on network error', () => {
+  it('ignores network errors', () => {
     let state = getInitialState([])
 
     const agent1 = new BskyAgent({service: 'https://alice.com'})
@@ -1217,11 +1217,9 @@ describe('session', () => {
       },
     ])
     expect(state.accounts.length).toBe(1)
-    // Network error should reset current user but not reset the tokens.
-    // TODO: We might want to remove or change this behavior?
     expect(state.accounts[0].accessJwt).toBe('alice-access-jwt-1')
     expect(state.accounts[0].refreshJwt).toBe('alice-refresh-jwt-1')
-    expect(state.currentAgentState.did).toBe(undefined)
+    expect(state.currentAgentState.did).toBe('alice-did')
     expect(printState(state)).toMatchInlineSnapshot(`
       {
         "accounts": [
@@ -1242,9 +1240,9 @@ describe('session', () => {
         ],
         "currentAgentState": {
           "agent": {
-            "service": "https://public.api.bsky.app/",
+            "service": "https://alice.com/",
           },
-          "did": undefined,
+          "did": "alice-did",
         },
         "needsPersist": true,
       }
diff --git a/src/state/session/agent.ts b/src/state/session/agent.ts
index ea6af677c..8a48cf95e 100644
--- a/src/state/session/agent.ts
+++ b/src/state/session/agent.ts
@@ -12,6 +12,7 @@ import {tryFetchGates} from '#/lib/statsig/statsig'
 import {getAge} from '#/lib/strings/time'
 import {logger} from '#/logger'
 import {snoozeEmailConfirmationPrompt} from '#/state/shell/reminders'
+import {emitNetworkConfirmed, emitNetworkLost} from '../events'
 import {addSessionErrorLog} from './logging'
 import {
   configureModerationForAccount,
@@ -227,6 +228,7 @@ export function sessionAccountToSession(
 }
 
 // Not exported. Use factories above to create it.
+let realFetch = globalThis.fetch
 class BskyAppAgent extends BskyAgent {
   persistSessionHandler: ((event: AtpSessionEvent) => void) | undefined =
     undefined
@@ -234,6 +236,23 @@ class BskyAppAgent extends BskyAgent {
   constructor({service}: {service: string}) {
     super({
       service,
+      async fetch(...args) {
+        let success = false
+        try {
+          const result = await realFetch(...args)
+          success = true
+          return result
+        } catch (e) {
+          success = false
+          throw e
+        } finally {
+          if (success) {
+            emitNetworkConfirmed()
+          } else {
+            emitNetworkLost()
+          }
+        }
+      },
       persistSession: (event: AtpSessionEvent) => {
         if (this.persistSessionHandler) {
           this.persistSessionHandler(event)
@@ -257,7 +276,15 @@ class BskyAppAgent extends BskyAgent {
 
     // Now the agent is ready.
     const account = agentToSessionAccountOrThrow(this)
+    let lastSession = this.sessionManager.session
     this.persistSessionHandler = event => {
+      if (this.sessionManager.session) {
+        lastSession = this.sessionManager.session
+      } else if (event === 'network-error') {
+        // Put it back, we'll try again later.
+        this.sessionManager.session = lastSession
+      }
+
       onSessionChange(this, account.did, event)
       if (event !== 'create' && event !== 'update') {
         addSessionErrorLog(account.did, event)
diff --git a/src/state/session/reducer.ts b/src/state/session/reducer.ts
index 0a537b42c..b49198514 100644
--- a/src/state/session/reducer.ts
+++ b/src/state/session/reducer.ts
@@ -79,12 +79,8 @@ let reducer = (state: State, action: Action): State => {
         return state
       }
       if (sessionEvent === 'network-error') {
-        // Don't change stored accounts but kick to the choose account screen.
-        return {
-          accounts: state.accounts,
-          currentAgentState: createPublicAgentState(),
-          needsPersist: true,
-        }
+        // Assume it's transient.
+        return state
       }
       const existingAccount = state.accounts.find(a => a.did === accountDid)
       if (