Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion shared/chat/conversation/thread-load.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ export const loadConversationThreadMessages = (
reason: threadLoadReasonToRPCReason(reason),
waitingKey: loadingKey,
})
if (!isCurrentThreadLoad()) {
if (!isCurrentThreadLoad() || !results) {
return
}
updateInboxConversationMeta(conversationIDKey, {offline: results.offline})
Expand Down
4 changes: 4 additions & 0 deletions shared/chat/conversation/thread-rpc.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as T from '@/constants/types'
import {enumKeys} from '@/constants/utils'
import {isChatSessionReady} from '@/stores/config'

type WaitingKey = string | ReadonlyArray<string>

Expand Down Expand Up @@ -54,6 +55,9 @@ export const loadThreadNonblock = async (p: {
reason?: T.RPCChat.GetThreadReason
waitingKey?: WaitingKey
}) => {
if (!isChatSessionReady()) {
return
}
const incomingCallMap: T.RPCChat.IncomingCallMapType = {}
if (p.onCachedThread) {
incomingCallMap['chat.1.chatUi.chatThreadCached'] = params => p.onCachedThread?.(params.thread || '')
Expand Down
8 changes: 6 additions & 2 deletions shared/chat/inbox/layout-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

let mockIsPhone = false
let mockLoggedIn = true
let mockUsername = 'alice'
let mockUserSwitching = false
let mockUsername = 'testuser'
const mockLoggerInfo = jest.fn()
const mockLoggerWarn = jest.fn()

Expand All @@ -22,9 +23,11 @@ jest.mock('@/logger', () => ({
}))

jest.mock('@/stores/config', () => ({
isChatSessionReady: () => mockLoggedIn && !mockUserSwitching,
useConfigState: {
getState: () => ({
loggedIn: mockLoggedIn,
userSwitching: mockUserSwitching,
}),
},
}))
Expand Down Expand Up @@ -55,7 +58,8 @@ const layoutWithRows: T.RPCChat.UIInboxLayout = {
beforeEach(() => {
mockIsPhone = false
mockLoggedIn = true
mockUsername = 'alice'
mockUserSwitching = false
mockUsername = 'testuser'
mockLoggerInfo.mockClear()
mockLoggerWarn.mockClear()
useInboxLayoutState.getState().dispatch.resetState()
Expand Down
13 changes: 6 additions & 7 deletions shared/chat/inbox/layout-state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import * as Z from '@/util/zustand'
import isEqual from 'lodash/isEqual'
import logger from '@/logger'
import {isPhone} from '@/constants/platform'
import {useConfigState} from '@/stores/config'
import {useCurrentUserState} from '@/stores/current-user'
import {isChatSessionReady} from '@/stores/config'
import {ignorePromise} from '@/constants/utils'
import {registerInboxRefresh} from './inbox-refresh'
import {withChatSessionRetry} from './session-rpc'

type Store = T.Immutable<{
hasLoaded: boolean
Expand Down Expand Up @@ -65,18 +65,17 @@ const recycleLayoutRows = (

export const useInboxLayoutState = Z.createZustand<State>('chat-inbox-layout', (set, get) => {
const requestInboxLayout = async (reason: T.Chat.RefreshReason) => {
const {username} = useCurrentUserState.getState()
const {loggedIn} = useConfigState.getState()
if (!loggedIn || !username) {
if (!isChatSessionReady()) {
return
}

logger.info(`Inbox refresh due to ${reason}`)
const reselectMode =
get().hasLoaded || isPhone
? T.RPCChat.InboxLayoutReselectMode.default
: T.RPCChat.InboxLayoutReselectMode.force
await T.RPCChat.localRequestInboxLayoutRpcPromise({reselectMode})
await withChatSessionRetry(async () =>
T.RPCChat.localRequestInboxLayoutRpcPromise({reselectMode})
)
}

const dispatch: State['dispatch'] = {
Expand Down
38 changes: 37 additions & 1 deletion shared/chat/inbox/metadata.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as Meta from '@/constants/chat/meta'
import * as T from '@/constants/types'
import {resetAllStores} from '@/util/zustand'
import {useConfigState} from '@/stores/config'
import {useCurrentUserState} from '@/stores/current-user'
import {
ensureConversationMetaLoaded,
forceUnboxRowsForService,
Expand All @@ -23,7 +24,8 @@ const flushPromises = async () => {
}

beforeEach(() => {
useConfigState.setState({loggedIn: true})
useConfigState.setState({loggedIn: true, userSwitching: false})
useCurrentUserState.setState({username: 'testuser'})
})

afterEach(() => {
Expand Down Expand Up @@ -302,3 +304,37 @@ test('ensure does not run while logged out and can re-arm after login', async ()
await jest.advanceTimersByTimeAsync(0)
expect(rpc).toHaveBeenCalledTimes(1)
})

test('userSwitching skips inbox unbox', async () => {
jest.spyOn(T.RPCChat, 'localRequestInboxUnboxRpcPromise').mockResolvedValue(undefined)
useConfigState.setState({loggedIn: true, userSwitching: true})

unboxRows([convID])
await flushPromises()

expect(T.RPCChat.localRequestInboxUnboxRpcPromise).not.toHaveBeenCalled()
})

test('setUserSwitching abandons further unbox until switch completes', async () => {
const resolvers = new Array<() => void>()
jest.spyOn(T.RPCChat, 'localRequestInboxUnboxRpcPromise').mockImplementation(
async () =>
new Promise(resolve => {
resolvers.push(() => {
resolve(undefined)
})
})
)

unboxRows([convID])
await flushPromises()
expect(T.RPCChat.localRequestInboxUnboxRpcPromise).toHaveBeenCalledTimes(1)

useConfigState.getState().dispatch.setUserSwitching(true)
resolvers[0]?.()
await flushPromises()

unboxRows([convID])
await flushPromises()
expect(T.RPCChat.localRequestInboxUnboxRpcPromise).toHaveBeenCalledTimes(1)
})
13 changes: 8 additions & 5 deletions shared/chat/inbox/metadata.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import logger from '@/logger'
import {ignorePromise, timeoutPromise} from '@/constants/utils'
import {RPCError} from '@/util/errors'
import * as Z from '@/util/zustand'
import {useConfigState} from '@/stores/config'
import {useConfigState, isChatSessionReady} from '@/stores/config'
import {withChatSessionRetry} from './session-rpc'
import {useCurrentUserState} from '@/stores/current-user'
import {useUsersState} from '@/stores/users'

Expand Down Expand Up @@ -405,7 +406,7 @@ async function runMetaQueueWorker(generation: number) {

const requestInboxUnboxRows = (ids: ReadonlyArray<T.Chat.ConversationIDKey>, force: boolean) => {
const f = async () => {
if (!useConfigState.getState().loggedIn) {
if (!isChatSessionReady()) {
return
}

Expand All @@ -431,9 +432,11 @@ const requestInboxUnboxRows = (ids: ReadonlyArray<T.Chat.ConversationIDKey>, for
`unboxRows: unboxing len: ${conversationIDKeys.length} convs: ${conversationIDKeys.join(',')}`
)
try {
await T.RPCChat.localRequestInboxUnboxRpcPromise({
convIDs: conversationIDKeys.map(k => T.Chat.keyToConversationID(k)),
})
await withChatSessionRetry(async () =>
T.RPCChat.localRequestInboxUnboxRpcPromise({
convIDs: conversationIDKeys.map(k => T.Chat.keyToConversationID(k)),
})
)
} catch (error) {
if (error instanceof RPCError) {
logger.info(`unboxRows: failed ${error.desc}`)
Expand Down
77 changes: 77 additions & 0 deletions shared/chat/inbox/session-rpc.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/// <reference types="jest" />
import * as T from '@/constants/types'
import {resetAllStores} from '@/util/zustand'
import {useConfigState} from '@/stores/config'
import {useCurrentUserState} from '@/stores/current-user'
import {RPCError} from '@/util/errors'
import {withChatSessionRetry} from './session-rpc'

beforeEach(() => {
useConfigState.setState({loggedIn: true, userSwitching: false})
useCurrentUserState.setState({username: 'testuser'})
})

afterEach(() => {
resetAllStores()
jest.useRealTimers()
})

test('returns the first success', async () => {
const run = jest.fn().mockResolvedValue('ok')
await expect(withChatSessionRetry(run)).resolves.toBe('ok')
expect(run).toHaveBeenCalledTimes(1)
})

test('gives up when the username is empty', async () => {
useCurrentUserState.setState({username: ''})
const run = jest.fn().mockResolvedValue('ok')
await expect(withChatSessionRetry(run)).resolves.toBeUndefined()
expect(run).not.toHaveBeenCalled()
})

test('retries login-required while still logged in', async () => {
jest.useFakeTimers()
const run = jest
.fn()
.mockRejectedValueOnce(new RPCError('chat session not ready', T.RPCGen.StatusCode.scloginrequired))
.mockResolvedValueOnce('ok')

const pending = withChatSessionRetry(run)
await jest.advanceTimersByTimeAsync(250)
await expect(pending).resolves.toBe('ok')
expect(run).toHaveBeenCalledTimes(2)
})

test('does not retry other errors', async () => {
const run = jest.fn().mockRejectedValue(new RPCError('nope', T.RPCGen.StatusCode.scgeneric))
await expect(withChatSessionRetry(run)).rejects.toMatchObject({code: T.RPCGen.StatusCode.scgeneric})
expect(run).toHaveBeenCalledTimes(1)
})

test('gives up when the chat session is no longer ready', async () => {
jest.useFakeTimers()
const run = jest
.fn()
.mockRejectedValue(new RPCError('chat session not ready', T.RPCGen.StatusCode.scloginrequired))

const pending = withChatSessionRetry(run)
await Promise.resolve()
useConfigState.setState({loggedIn: true, userSwitching: true})
await jest.advanceTimersByTimeAsync(250)
await expect(pending).resolves.toBeUndefined()
expect(run).toHaveBeenCalledTimes(1)
})

test('gives up when the username changes during retry', async () => {
jest.useFakeTimers()
const run = jest
.fn()
.mockRejectedValue(new RPCError('chat session not ready', T.RPCGen.StatusCode.scloginrequired))

const pending = withChatSessionRetry(run)
await Promise.resolve()
useCurrentUserState.setState({username: 'otheruser'})
await jest.advanceTimersByTimeAsync(250)
await expect(pending).resolves.toBeUndefined()
expect(run).toHaveBeenCalledTimes(1)
})
34 changes: 34 additions & 0 deletions shared/chat/inbox/session-rpc.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import * as T from '@/constants/types'
import logger from '@/logger'
import {isChatSessionReady} from '@/stores/config'
import {useCurrentUserState} from '@/stores/current-user'
import {timeoutPromise} from '@/constants/utils'
import {RPCError} from '@/util/errors'

const retryDelaysMs = [250, 750]

export const withChatSessionRetry = async <R,>(run: () => Promise<R>): Promise<R | undefined> => {
const username = useCurrentUserState.getState().username
const sameSession = () =>
!!username && isChatSessionReady() && useCurrentUserState.getState().username === username
for (let attempt = 0; attempt <= retryDelaysMs.length; attempt++) {
if (!sameSession()) {
return
}
try {
return await run()
} catch (error) {
const delay = retryDelaysMs[attempt]
if (!(error instanceof RPCError) || error.code !== T.RPCGen.StatusCode.scloginrequired) {
throw error
}
if (delay === undefined || !sameSession()) {
logger.info('chat session not ready, giving up')
return
}
logger.info(`chat session not ready, retrying in ${delay}ms`)
await timeoutPromise(delay)
}
}
return
}
Loading