[🐴] Error recovery (#4036)

* Handle block state when sending messages

* Handle different pending failures

* Use existing profile data to handle blocks

* Better cleanup, leave room for more

* Attempt recover upon next send

* Reset pending failure

* Capture unexpected error

* Gracefully handle network errors and recovery

* Re-align error components and types

* Include history fetching in recoverable states
zio/stable
Eric Bailey 2024-05-16 14:01:39 -05:00 committed by GitHub
parent dff6bd7c65
commit 4bceabc21c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 216 additions and 113 deletions

View File

@ -202,7 +202,7 @@ let MessageItemMetadata = ({
)} )}
</TimeElapsed> </TimeElapsed>
{item.type === 'pending-message' && item.retry && ( {item.type === 'pending-message' && item.failed && (
<> <>
{' '} {' '}
&middot;{' '} &middot;{' '}
@ -214,15 +214,20 @@ let MessageItemMetadata = ({
}, },
]}> ]}>
{_(msg`Failed to send`)} {_(msg`Failed to send`)}
</Text>{' '} </Text>
&middot;{' '} {item.retry && (
<InlineLinkText <>
label={_(msg`Click to retry failed message`)} {' '}
to="#" &middot;{' '}
onPress={handleRetry} <InlineLinkText
style={[a.text_xs]}> label={_(msg`Click to retry failed message`)}
{_(msg`Retry`)} to="#"
</InlineLinkText> onPress={handleRetry}
style={[a.text_xs]}>
{_(msg`Retry`)}
</InlineLinkText>
</>
)}
</> </>
)} )}
</Text> </Text>

View File

@ -5,27 +5,25 @@ import {useLingui} from '@lingui/react'
import {ConvoItem, ConvoItemError} from '#/state/messages/convo/types' import {ConvoItem, ConvoItemError} from '#/state/messages/convo/types'
import {atoms as a, useTheme} from '#/alf' import {atoms as a, useTheme} from '#/alf'
import {Button, ButtonIcon, ButtonText} from '#/components/Button'
import {ArrowRotateCounterClockwise_Stroke2_Corner0_Rounded as Refresh} from '#/components/icons/ArrowRotateCounterClockwise'
import {CircleInfo_Stroke2_Corner0_Rounded as CircleInfo} from '#/components/icons/CircleInfo' import {CircleInfo_Stroke2_Corner0_Rounded as CircleInfo} from '#/components/icons/CircleInfo'
import {InlineLinkText} from '#/components/Link'
import {Text} from '#/components/Typography' import {Text} from '#/components/Typography'
export function MessageListError({ export function MessageListError({item}: {item: ConvoItem & {type: 'error'}}) {
item,
}: {
item: ConvoItem & {type: 'error-recoverable'}
}) {
const t = useTheme() const t = useTheme()
const {_} = useLingui() const {_} = useLingui()
const message = React.useMemo(() => { const {description, help, cta} = React.useMemo(() => {
return { return {
[ConvoItemError.Network]: _( [ConvoItemError.FirehoseFailed]: {
msg`There was an issue connecting to the chat.`, description: _(msg`This chat was disconnected`),
), help: _(msg`Press to attempt reconnection`),
[ConvoItemError.FirehoseFailed]: _( cta: _(msg`Reconnect`),
msg`This chat was disconnected due to a network error.`, },
), [ConvoItemError.HistoryFailed]: {
[ConvoItemError.HistoryFailed]: _(msg`Failed to load past messages.`), description: _(msg`Failed to load past messages`),
help: _(msg`Press to retry`),
cta: _(msg`Retry`),
},
}[item.code] }[item.code]
}, [_, item.code]) }, [_, item.code])
@ -36,37 +34,31 @@ export function MessageListError({
a.flex_row, a.flex_row,
a.align_center, a.align_center,
a.justify_between, a.justify_between,
a.gap_lg, a.gap_sm,
a.py_md, a.pb_lg,
a.px_lg,
a.rounded_md,
t.atoms.bg_contrast_25,
{maxWidth: 400}, {maxWidth: 400},
]}> ]}>
<View style={[a.flex_row, a.align_start, a.justify_between, a.gap_sm]}> <CircleInfo
<CircleInfo size="sm"
size="sm" fill={t.palette.negative_400}
fill={t.palette.negative_400} style={[{top: 3}]}
style={[{top: 3}]} />
/>
<View style={[a.flex_1, {maxWidth: 200}]}>
<Text style={[a.leading_snug]}>{message}</Text>
</View>
</View>
<Button <Text style={[a.leading_snug, a.flex_1, t.atoms.text_contrast_medium]}>
label={_(msg`Press to retry`)} {description} &middot;{' '}
size="small" {item.retry && (
variant="ghost" <InlineLinkText
color="secondary" to="#"
onPress={e => { label={help}
e.preventDefault() onPress={e => {
item.retry() e.preventDefault()
return false item.retry?.()
}}> return false
<ButtonText>{_(msg`Retry`)}</ButtonText> }}>
<ButtonIcon icon={Refresh} position="right" /> {cta}
</Button> </InlineLinkText>
)}
</Text>
</View> </View>
</View> </View>
) )

View File

@ -46,7 +46,7 @@ function renderItem({item}: {item: ConvoItem}) {
return <MessageItem item={item} /> return <MessageItem item={item} />
} else if (item.type === 'deleted-message') { } else if (item.type === 'deleted-message') {
return <Text>Deleted message</Text> return <Text>Deleted message</Text>
} else if (item.type === 'error-recoverable') { } else if (item.type === 'error') {
return <MessageListError item={item} /> return <MessageListError item={item} />
} }

View File

@ -5,6 +5,8 @@ import {
ChatBskyConvoGetLog, ChatBskyConvoGetLog,
ChatBskyConvoSendMessage, ChatBskyConvoSendMessage,
} from '@atproto/api' } from '@atproto/api'
import {XRPCError} from '@atproto/xrpc'
import EventEmitter from 'eventemitter3'
import {nanoid} from 'nanoid/non-secure' import {nanoid} from 'nanoid/non-secure'
import {networkRetry} from '#/lib/async/retry' import {networkRetry} from '#/lib/async/retry'
@ -14,11 +16,14 @@ import {
ACTIVE_POLL_INTERVAL, ACTIVE_POLL_INTERVAL,
BACKGROUND_POLL_INTERVAL, BACKGROUND_POLL_INTERVAL,
INACTIVE_TIMEOUT, INACTIVE_TIMEOUT,
NETWORK_FAILURE_STATUSES,
} from '#/state/messages/convo/const' } from '#/state/messages/convo/const'
import { import {
ConvoDispatch, ConvoDispatch,
ConvoDispatchEvent, ConvoDispatchEvent,
ConvoError,
ConvoErrorCode, ConvoErrorCode,
ConvoEvent,
ConvoItem, ConvoItem,
ConvoItemError, ConvoItemError,
ConvoParams, ConvoParams,
@ -51,13 +56,7 @@ export class Convo {
private senderUserDid: string private senderUserDid: string
private status: ConvoStatus = ConvoStatus.Uninitialized private status: ConvoStatus = ConvoStatus.Uninitialized
private error: private error: ConvoError | undefined
| {
code: ConvoErrorCode
exception?: Error
retry: () => void
}
| undefined
private oldestRev: string | undefined | null = undefined private oldestRev: string | undefined | null = undefined
private isFetchingHistory = false private isFetchingHistory = false
private latestRev: string | undefined = undefined private latestRev: string | undefined = undefined
@ -75,13 +74,13 @@ export class Convo {
{id: string; message: ChatBskyConvoSendMessage.InputSchema['message']} {id: string; message: ChatBskyConvoSendMessage.InputSchema['message']}
> = new Map() > = new Map()
private deletedMessages: Set<string> = new Set() private deletedMessages: Set<string> = new Set()
private footerItems: Map<string, ConvoItem> = new Map()
private headerItems: Map<string, ConvoItem> = new Map()
private isProcessingPendingMessages = false private isProcessingPendingMessages = false
private lastActiveTimestamp: number | undefined private lastActiveTimestamp: number | undefined
private emitter = new EventEmitter<{event: [ConvoEvent]}>()
convoId: string convoId: string
convo: ChatBskyConvoDefs.ConvoView | undefined convo: ChatBskyConvoDefs.ConvoView | undefined
sender: AppBskyActorDefs.ProfileViewBasic | undefined sender: AppBskyActorDefs.ProfileViewBasic | undefined
@ -174,7 +173,7 @@ export class Convo {
status: ConvoStatus.Error, status: ConvoStatus.Error,
items: [], items: [],
convo: undefined, convo: undefined,
error: this.error, error: this.error!,
sender: undefined, sender: undefined,
recipients: undefined, recipients: undefined,
isFetchingHistory: false, isFetchingHistory: false,
@ -282,6 +281,7 @@ export class Convo {
if (this.convo) { if (this.convo) {
this.status = ConvoStatus.Ready this.status = ConvoStatus.Ready
this.refreshConvo() this.refreshConvo()
this.maybeRecoverFromNetworkError()
} else { } else {
this.status = ConvoStatus.Initializing this.status = ConvoStatus.Initializing
this.setup() this.setup()
@ -379,12 +379,30 @@ export class Convo {
this.newMessages = new Map() this.newMessages = new Map()
this.pendingMessages = new Map() this.pendingMessages = new Map()
this.deletedMessages = new Set() this.deletedMessages = new Set()
this.footerItems = new Map()
this.headerItems = new Map() this.pendingMessageFailure = null
this.fetchMessageHistoryError = undefined
this.firehoseError = undefined
this.dispatch({event: ConvoDispatchEvent.Init}) this.dispatch({event: ConvoDispatchEvent.Init})
} }
maybeRecoverFromNetworkError() {
if (this.firehoseError) {
this.firehoseError.retry()
this.firehoseError = undefined
this.commit()
} else {
this.batchRetryPendingMessages()
}
if (this.fetchMessageHistoryError) {
this.fetchMessageHistoryError.retry()
this.fetchMessageHistoryError = undefined
this.commit()
}
}
private async setup() { private async setup() {
try { try {
const {convo, sender, recipients} = await this.fetchConvo() const {convo, sender, recipients} = await this.fetchConvo()
@ -520,6 +538,11 @@ export class Convo {
} }
} }
private fetchMessageHistoryError:
| {
retry: () => void
}
| undefined
async fetchMessageHistory() { async fetchMessageHistory() {
logger.debug('Convo: fetch message history', {}, logger.DebugContext.convo) logger.debug('Convo: fetch message history', {}, logger.DebugContext.convo)
@ -537,7 +560,7 @@ export class Convo {
* If we've rendered a retry state for history fetching, exit. Upon retry, * If we've rendered a retry state for history fetching, exit. Upon retry,
* this will be removed and we'll try again. * this will be removed and we'll try again.
*/ */
if (this.headerItems.has(ConvoItemError.HistoryFailed)) return if (this.fetchMessageHistoryError) return
try { try {
this.isFetchingHistory = true this.isFetchingHistory = true
@ -586,15 +609,11 @@ export class Convo {
} catch (e: any) { } catch (e: any) {
logger.error('Convo: failed to fetch message history') logger.error('Convo: failed to fetch message history')
this.headerItems.set(ConvoItemError.HistoryFailed, { this.fetchMessageHistoryError = {
type: 'error-recoverable',
key: ConvoItemError.HistoryFailed,
code: ConvoItemError.HistoryFailed,
retry: () => { retry: () => {
this.headerItems.delete(ConvoItemError.HistoryFailed)
this.fetchMessageHistory() this.fetchMessageHistory()
}, },
}) }
} finally { } finally {
this.isFetchingHistory = false this.isFetchingHistory = false
this.commit() this.commit()
@ -628,22 +647,16 @@ export class Convo {
) )
} }
private firehoseError: MessagesEventBusError | undefined
onFirehoseConnect() { onFirehoseConnect() {
this.footerItems.delete(ConvoItemError.FirehoseFailed) this.firehoseError = undefined
this.batchRetryPendingMessages()
this.commit() this.commit()
} }
onFirehoseError(error?: MessagesEventBusError) { onFirehoseError(error?: MessagesEventBusError) {
this.footerItems.set(ConvoItemError.FirehoseFailed, { this.firehoseError = error
type: 'error-recoverable',
key: ConvoItemError.FirehoseFailed,
code: ConvoItemError.FirehoseFailed,
retry: () => {
this.footerItems.delete(ConvoItemError.FirehoseFailed)
this.commit()
error?.retry()
},
})
this.commit() this.commit()
} }
@ -724,7 +737,7 @@ export class Convo {
} }
} }
private pendingFailed = false private pendingMessageFailure: 'recoverable' | 'unrecoverable' | null = null
async sendMessage(message: ChatBskyConvoSendMessage.InputSchema['message']) { async sendMessage(message: ChatBskyConvoSendMessage.InputSchema['message']) {
// Ignore empty messages for now since they have no other purpose atm // Ignore empty messages for now since they have no other purpose atm
@ -734,13 +747,14 @@ export class Convo {
const tempId = nanoid() const tempId = nanoid()
this.pendingMessageFailure = null
this.pendingMessages.set(tempId, { this.pendingMessages.set(tempId, {
id: tempId, id: tempId,
message, message,
}) })
this.commit() this.commit()
if (!this.isProcessingPendingMessages && !this.pendingFailed) { if (!this.isProcessingPendingMessages && !this.pendingMessageFailure) {
this.processPendingMessages() this.processPendingMessages()
} }
} }
@ -765,7 +779,6 @@ export class Convo {
try { try {
this.isProcessingPendingMessages = true this.isProcessingPendingMessages = true
// throw new Error('UNCOMMENT TO TEST RETRY')
const {id, message} = pendingMessage const {id, message} = pendingMessage
const response = await networkRetry(2, () => { const response = await networkRetry(2, () => {
@ -794,23 +807,65 @@ export class Convo {
this.commit() this.commit()
} catch (e: any) { } catch (e: any) {
logger.error(e, {context: `Convo: failed to send message`}) logger.error(e, {context: `Convo: failed to send message`})
this.pendingFailed = true this.handleSendMessageFailure(e)
this.commit()
} finally { } finally {
this.isProcessingPendingMessages = false this.isProcessingPendingMessages = false
} }
} }
private handleSendMessageFailure(e: any) {
if (e instanceof XRPCError) {
if (NETWORK_FAILURE_STATUSES.includes(e.status)) {
this.pendingMessageFailure = 'recoverable'
} else {
switch (e.message) {
case 'block between recipient and sender':
this.pendingMessageFailure = 'unrecoverable'
this.emitter.emit('event', {
type: 'invalidate-block-state',
accountDids: [
this.sender!.did,
...this.recipients!.map(r => r.did),
],
})
break
default:
logger.warn(
`Convo handleSendMessageFailure could not handle error`,
{
status: e.status,
message: e.message,
},
)
break
}
}
} else {
logger.error(e, {
context: `Convo handleSendMessageFailure received unknown error`,
})
}
this.commit()
}
async batchRetryPendingMessages() { async batchRetryPendingMessages() {
if (this.pendingMessageFailure === null) return
const messageArray = Array.from(this.pendingMessages.values())
if (messageArray.length === 0) return
this.pendingMessageFailure = null
this.commit()
logger.debug( logger.debug(
`Convo: retrying ${this.pendingMessages.size} pending messages`, `Convo: batch retrying ${this.pendingMessages.size} pending messages`,
{}, {},
logger.DebugContext.convo, logger.DebugContext.convo,
) )
try { try {
// throw new Error('UNCOMMENT TO TEST RETRY') // throw new Error('UNCOMMENT TO TEST RETRY')
const messageArray = Array.from(this.pendingMessages.values())
const {data} = await networkRetry(2, () => { const {data} = await networkRetry(2, () => {
return this.agent.api.chat.bsky.convo.sendMessageBatch( return this.agent.api.chat.bsky.convo.sendMessageBatch(
{ {
@ -848,8 +903,7 @@ export class Convo {
) )
} catch (e: any) { } catch (e: any) {
logger.error(e, {context: `Convo: failed to batch retry messages`}) logger.error(e, {context: `Convo: failed to batch retry messages`})
this.pendingFailed = true this.handleSendMessageFailure(e)
this.commit()
} }
} }
@ -877,6 +931,14 @@ export class Convo {
} }
} }
on(handler: (event: ConvoEvent) => void) {
this.emitter.on('event', handler)
return () => {
this.emitter.off('event', handler)
}
}
/* /*
* Items in reverse order, since FlatList inverts * Items in reverse order, since FlatList inverts
*/ */
@ -901,9 +963,16 @@ export class Convo {
} }
}) })
this.headerItems.forEach(item => { if (this.fetchMessageHistoryError) {
items.unshift(item) items.unshift({
}) type: 'error',
code: ConvoItemError.HistoryFailed,
key: ConvoItemError.HistoryFailed,
retry: () => {
this.maybeRecoverFromNetworkError()
},
})
}
this.newMessages.forEach(m => { this.newMessages.forEach(m => {
if (ChatBskyConvoDefs.isMessageView(m)) { if (ChatBskyConvoDefs.isMessageView(m)) {
@ -940,19 +1009,26 @@ export class Convo {
sender: this.sender!, sender: this.sender!,
}, },
nextMessage: null, nextMessage: null,
retry: this.pendingFailed failed: this.pendingMessageFailure !== null,
? () => { retry:
this.pendingFailed = false this.pendingMessageFailure === 'recoverable'
this.commit() ? () => {
this.batchRetryPendingMessages() this.maybeRecoverFromNetworkError()
} }
: undefined, : undefined,
}) })
}) })
this.footerItems.forEach(item => { if (this.firehoseError) {
items.push(item) items.push({
}) type: 'error',
code: ConvoItemError.FirehoseFailed,
key: ConvoItemError.FirehoseFailed,
retry: () => {
this.firehoseError?.retry()
},
})
}
return items return items
.filter(item => { .filter(item => {

View File

@ -1,3 +1,7 @@
export const ACTIVE_POLL_INTERVAL = 1e3 export const ACTIVE_POLL_INTERVAL = 1e3
export const BACKGROUND_POLL_INTERVAL = 5e3 export const BACKGROUND_POLL_INTERVAL = 5e3
export const INACTIVE_TIMEOUT = 60e3 * 5 export const INACTIVE_TIMEOUT = 60e3 * 5
export const NETWORK_FAILURE_STATUSES = [
1, 408, 425, 429, 500, 502, 503, 504, 522, 524,
]

View File

@ -1,6 +1,7 @@
import React, {useContext, useState, useSyncExternalStore} from 'react' import React, {useContext, useState, useSyncExternalStore} from 'react'
import {AppState} from 'react-native' import {AppState} from 'react-native'
import {useFocusEffect, useIsFocused} from '@react-navigation/native' import {useFocusEffect, useIsFocused} from '@react-navigation/native'
import {useQueryClient} from '@tanstack/react-query'
import {Convo} from '#/state/messages/convo/agent' import {Convo} from '#/state/messages/convo/agent'
import { import {
@ -13,6 +14,8 @@ import {
import {isConvoActive} from '#/state/messages/convo/util' import {isConvoActive} from '#/state/messages/convo/util'
import {useMessagesEventBus} from '#/state/messages/events' import {useMessagesEventBus} from '#/state/messages/events'
import {useMarkAsReadMutation} from '#/state/queries/messages/conversation' import {useMarkAsReadMutation} from '#/state/queries/messages/conversation'
import {RQKEY as ListConvosQueryKey} from '#/state/queries/messages/list-converations'
import {RQKEY as createProfileQueryKey} from '#/state/queries/profile'
import {useAgent} from '#/state/session' import {useAgent} from '#/state/session'
export * from '#/state/messages/convo/util' export * from '#/state/messages/convo/util'
@ -52,6 +55,7 @@ export function ConvoProvider({
children, children,
convoId, convoId,
}: Pick<ConvoParams, 'convoId'> & {children: React.ReactNode}) { }: Pick<ConvoParams, 'convoId'> & {children: React.ReactNode}) {
const queryClient = useQueryClient()
const isScreenFocused = useIsFocused() const isScreenFocused = useIsFocused()
const {getAgent} = useAgent() const {getAgent} = useAgent()
const events = useMessagesEventBus() const events = useMessagesEventBus()
@ -78,6 +82,23 @@ export function ConvoProvider({
}, [convo, convoId, markAsRead]), }, [convo, convoId, markAsRead]),
) )
React.useEffect(() => {
return convo.on(event => {
switch (event.type) {
case 'invalidate-block-state': {
for (const did of event.accountDids) {
queryClient.invalidateQueries({
queryKey: createProfileQueryKey(did),
})
}
queryClient.invalidateQueries({
queryKey: ListConvosQueryKey,
})
}
}
})
}, [convo, queryClient])
React.useEffect(() => { React.useEffect(() => {
const handleAppStateChange = (nextAppState: string) => { const handleAppStateChange = (nextAppState: string) => {
if (isScreenFocused) { if (isScreenFocused) {

View File

@ -23,10 +23,6 @@ export enum ConvoStatus {
} }
export enum ConvoItemError { export enum ConvoItemError {
/**
* Generic error
*/
Network = 'network',
/** /**
* Error connecting to event firehose * Error connecting to event firehose
*/ */
@ -95,6 +91,7 @@ export type ConvoItem =
| ChatBskyConvoDefs.MessageView | ChatBskyConvoDefs.MessageView
| ChatBskyConvoDefs.DeletedMessageView | ChatBskyConvoDefs.DeletedMessageView
| null | null
failed: boolean
/** /**
* Retry sending the message. If present, the message is in a failed state. * Retry sending the message. If present, the message is in a failed state.
*/ */
@ -110,10 +107,13 @@ export type ConvoItem =
| null | null
} }
| { | {
type: 'error-recoverable' type: 'error'
key: string key: string
code: ConvoItemError code: ConvoItemError
retry: () => void /**
* If present, error is recoverable.
*/
retry?: () => void
} }
type DeleteMessage = (messageId: string) => Promise<void> type DeleteMessage = (messageId: string) => Promise<void>
@ -186,7 +186,7 @@ export type ConvoStateError = {
status: ConvoStatus.Error status: ConvoStatus.Error
items: [] items: []
convo: undefined convo: undefined
error: any error: ConvoError
sender: undefined sender: undefined
recipients: undefined recipients: undefined
isFetchingHistory: false isFetchingHistory: false
@ -201,3 +201,8 @@ export type ConvoState =
| ConvoStateBackgrounded | ConvoStateBackgrounded
| ConvoStateSuspended | ConvoStateSuspended
| ConvoStateError | ConvoStateError
export type ConvoEvent = {
type: 'invalidate-block-state'
accountDids: string[]
}