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.
zio/stable
dan 2024-08-13 18:51:49 +01:00 committed by GitHub
parent 7e11b862e9
commit 57be2ea15b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 117 additions and 14 deletions

View File

@ -2,18 +2,83 @@ import React, {useRef, useState} from 'react'
import {AppState, AppStateStatus} from 'react-native' import {AppState, AppStateStatus} from 'react-native'
import AsyncStorage from '@react-native-async-storage/async-storage' import AsyncStorage from '@react-native-async-storage/async-storage'
import {createAsyncStoragePersister} from '@tanstack/query-async-storage-persister' import {createAsyncStoragePersister} from '@tanstack/query-async-storage-persister'
import {focusManager, QueryClient} from '@tanstack/react-query' import {focusManager, onlineManager, QueryClient} from '@tanstack/react-query'
import { import {
PersistQueryClientProvider, PersistQueryClientProvider,
PersistQueryClientProviderProps, PersistQueryClientProviderProps,
} from '@tanstack/react-query-persist-client' } from '@tanstack/react-query-persist-client'
import {isNative} from '#/platform/detection' import {isNative} from '#/platform/detection'
import {listenNetworkConfirmed, listenNetworkLost} from '#/state/events'
// any query keys in this array will be persisted to AsyncStorage // any query keys in this array will be persisted to AsyncStorage
export const labelersDetailedInfoQueryKeyRoot = 'labelers-detailed-info' export const labelersDetailedInfoQueryKeyRoot = 'labelers-detailed-info'
const STORED_CACHE_QUERY_KEY_ROOTS = [labelersDetailedInfoQueryKeyRoot] 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 => { focusManager.setEventListener(onFocus => {
if (isNative) { if (isNative) {
const subscription = AppState.addEventListener( const subscription = AppState.addEventListener(

View File

@ -22,6 +22,22 @@ export function listenSessionDropped(fn: () => void): UnlistenFn {
return () => emitter.off('session-dropped', fn) 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() { export function emitPostCreated() {
emitter.emit('post-created') emitter.emit('post-created')
} }

View File

@ -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 // order the feeds/lists in the order they were pinned
const result: SavedFeedSourceInfo[] = [] const result: SavedFeedSourceInfo[] = []

View File

@ -1184,7 +1184,7 @@ describe('session', () => {
expect(state.currentAgentState.did).toBe('bob-did') expect(state.currentAgentState.did).toBe('bob-did')
}) })
it('does soft logout on network error', () => { it('ignores network errors', () => {
let state = getInitialState([]) let state = getInitialState([])
const agent1 = new BskyAgent({service: 'https://alice.com'}) const agent1 = new BskyAgent({service: 'https://alice.com'})
@ -1217,11 +1217,9 @@ describe('session', () => {
}, },
]) ])
expect(state.accounts.length).toBe(1) 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].accessJwt).toBe('alice-access-jwt-1')
expect(state.accounts[0].refreshJwt).toBe('alice-refresh-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(` expect(printState(state)).toMatchInlineSnapshot(`
{ {
"accounts": [ "accounts": [
@ -1242,9 +1240,9 @@ describe('session', () => {
], ],
"currentAgentState": { "currentAgentState": {
"agent": { "agent": {
"service": "https://public.api.bsky.app/", "service": "https://alice.com/",
}, },
"did": undefined, "did": "alice-did",
}, },
"needsPersist": true, "needsPersist": true,
} }

View File

@ -12,6 +12,7 @@ import {tryFetchGates} from '#/lib/statsig/statsig'
import {getAge} from '#/lib/strings/time' import {getAge} from '#/lib/strings/time'
import {logger} from '#/logger' import {logger} from '#/logger'
import {snoozeEmailConfirmationPrompt} from '#/state/shell/reminders' import {snoozeEmailConfirmationPrompt} from '#/state/shell/reminders'
import {emitNetworkConfirmed, emitNetworkLost} from '../events'
import {addSessionErrorLog} from './logging' import {addSessionErrorLog} from './logging'
import { import {
configureModerationForAccount, configureModerationForAccount,
@ -227,6 +228,7 @@ export function sessionAccountToSession(
} }
// Not exported. Use factories above to create it. // Not exported. Use factories above to create it.
let realFetch = globalThis.fetch
class BskyAppAgent extends BskyAgent { class BskyAppAgent extends BskyAgent {
persistSessionHandler: ((event: AtpSessionEvent) => void) | undefined = persistSessionHandler: ((event: AtpSessionEvent) => void) | undefined =
undefined undefined
@ -234,6 +236,23 @@ class BskyAppAgent extends BskyAgent {
constructor({service}: {service: string}) { constructor({service}: {service: string}) {
super({ super({
service, 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) => { persistSession: (event: AtpSessionEvent) => {
if (this.persistSessionHandler) { if (this.persistSessionHandler) {
this.persistSessionHandler(event) this.persistSessionHandler(event)
@ -257,7 +276,15 @@ class BskyAppAgent extends BskyAgent {
// Now the agent is ready. // Now the agent is ready.
const account = agentToSessionAccountOrThrow(this) const account = agentToSessionAccountOrThrow(this)
let lastSession = this.sessionManager.session
this.persistSessionHandler = event => { 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) onSessionChange(this, account.did, event)
if (event !== 'create' && event !== 'update') { if (event !== 'create' && event !== 'update') {
addSessionErrorLog(account.did, event) addSessionErrorLog(account.did, event)

View File

@ -79,12 +79,8 @@ let reducer = (state: State, action: Action): State => {
return state return state
} }
if (sessionEvent === 'network-error') { if (sessionEvent === 'network-error') {
// Don't change stored accounts but kick to the choose account screen. // Assume it's transient.
return { return state
accounts: state.accounts,
currentAgentState: createPublicAgentState(),
needsPersist: true,
}
} }
const existingAccount = state.accounts.find(a => a.did === accountDid) const existingAccount = state.accounts.find(a => a.did === accountDid)
if ( if (