Add a mutation queue to fix race conditions in toggles (#1933)

* Prototype a queue

* Track both current and pending actions

* Skip unnecessary actions

* Commit last confirmed state to shadow

* Thread state through actions over time

* Fix the logic to skip redundant mutations

* Track status

* Extract an abstraction

* Fix standalone mutations

* Add types

* Move to another file

* Return stable function

* Clean up

* Use queue for muting

* Use queue for blocking

* Convert other follow buttons

* Don't export non-queue mutations

* Properly handle canceled tasks

* Fix copy paste
zio/stable
dan 2023-11-16 22:01:01 +00:00 committed by GitHub
parent 54faa7e176
commit 8475312422
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 453 additions and 188 deletions

View File

@ -0,0 +1,98 @@
import {useState, useRef, useEffect, useCallback} from 'react'
type Task<TServerState> = {
isOn: boolean
resolve: (serverState: TServerState) => void
reject: (e: unknown) => void
}
type TaskQueue<TServerState> = {
activeTask: Task<TServerState> | null
queuedTask: Task<TServerState> | null
}
function AbortError() {
const e = new Error()
e.name = 'AbortError'
return e
}
export function useToggleMutationQueue<TServerState>({
initialState,
runMutation,
onSuccess,
}: {
initialState: TServerState
runMutation: (
prevState: TServerState,
nextIsOn: boolean,
) => Promise<TServerState>
onSuccess: (finalState: TServerState) => void
}) {
// We use the queue as a mutable object.
// This is safe becuase it is not used for rendering.
const [queue] = useState<TaskQueue<TServerState>>({
activeTask: null,
queuedTask: null,
})
async function processQueue() {
if (queue.activeTask) {
// There is another active processQueue call iterating over tasks.
// It will handle any newly added tasks, so we should exit early.
return
}
// To avoid relying on the rendered state, capture it once at the start.
// From that point on, and until the queue is drained, we'll use the real server state.
let confirmedState: TServerState = initialState
try {
while (queue.queuedTask) {
const prevTask = queue.activeTask
const nextTask = queue.queuedTask
queue.activeTask = nextTask
queue.queuedTask = null
if (prevTask?.isOn === nextTask.isOn) {
// Skip multiple requests to update to the same value in a row.
prevTask.reject(new (AbortError as any)())
continue
}
try {
// The state received from the server feeds into the next task.
// This lets us queue deletions of not-yet-created resources.
confirmedState = await runMutation(confirmedState, nextTask.isOn)
nextTask.resolve(confirmedState)
} catch (e) {
nextTask.reject(e)
}
}
} finally {
onSuccess(confirmedState)
queue.activeTask = null
queue.queuedTask = null
}
}
function queueToggle(isOn: boolean): Promise<TServerState> {
return new Promise((resolve, reject) => {
// This is a toggle, so the next queued value can safely replace the queued one.
if (queue.queuedTask) {
queue.queuedTask.reject(new (AbortError as any)())
}
queue.queuedTask = {isOn, resolve, reject}
processQueue()
})
}
const queueToggleRef = useRef(queueToggle)
useEffect(() => {
queueToggleRef.current = queueToggle
})
const queueToggleStable = useCallback(
(isOn: boolean): Promise<TServerState> => {
const queueToggleLatest = queueToggleRef.current
return queueToggleLatest(isOn)
},
[],
)
return queueToggleStable
}

View File

@ -1,3 +1,4 @@
import {useCallback} from 'react'
import {
AtUri,
AppBskyActorDefs,
@ -11,6 +12,8 @@ import {useSession} from '../session'
import {updateProfileShadow} from '../cache/profile-shadow'
import {uploadBlob} from '#/lib/api'
import {until} from '#/lib/async/until'
import {Shadow} from '#/state/cache/types'
import {useToggleMutationQueue} from '#/lib/hooks/useToggleMutationQueue'
import {RQKEY as RQKEY_MY_MUTED} from './my-muted-accounts'
import {RQKEY as RQKEY_MY_BLOCKED} from './my-blocked-accounts'
@ -99,104 +102,294 @@ export function useProfileUpdateMutation() {
})
}
export function useProfileFollowMutation() {
export function useProfileFollowMutationQueue(
profile: Shadow<AppBskyActorDefs.ProfileViewDetailed>,
) {
const did = profile.did
const initialFollowingUri = profile.viewer?.following
const followMutation = useProfileFollowMutation()
const unfollowMutation = useProfileUnfollowMutation()
const queueToggle = useToggleMutationQueue({
initialState: initialFollowingUri,
runMutation: async (prevFollowingUri, shouldFollow) => {
if (shouldFollow) {
const {uri} = await followMutation.mutateAsync({
did,
skipOptimistic: true,
})
return uri
} else {
if (prevFollowingUri) {
await unfollowMutation.mutateAsync({
did,
followUri: prevFollowingUri,
skipOptimistic: true,
})
}
return undefined
}
},
onSuccess(finalFollowingUri) {
// finalize
updateProfileShadow(did, {
followingUri: finalFollowingUri,
})
},
})
const queueFollow = useCallback(() => {
// optimistically update
updateProfileShadow(did, {
followingUri: 'pending',
})
return queueToggle(true)
}, [did, queueToggle])
const queueUnfollow = useCallback(() => {
// optimistically update
updateProfileShadow(did, {
followingUri: undefined,
})
return queueToggle(false)
}, [did, queueToggle])
return [queueFollow, queueUnfollow]
}
function useProfileFollowMutation() {
const {agent} = useSession()
return useMutation<{uri: string; cid: string}, Error, {did: string}>({
return useMutation<
{uri: string; cid: string},
Error,
{did: string; skipOptimistic?: boolean}
>({
mutationFn: async ({did}) => {
return await agent.follow(did)
},
onMutate(variables) {
// optimstically update
if (!variables.skipOptimistic) {
// optimistically update
updateProfileShadow(variables.did, {
followingUri: 'pending',
})
}
},
onSuccess(data, variables) {
if (!variables.skipOptimistic) {
// finalize
updateProfileShadow(variables.did, {
followingUri: data.uri,
})
}
},
onError(error, variables) {
if (!variables.skipOptimistic) {
// revert the optimistic update
updateProfileShadow(variables.did, {
followingUri: undefined,
})
}
},
})
}
export function useProfileUnfollowMutation() {
function useProfileUnfollowMutation() {
const {agent} = useSession()
return useMutation<void, Error, {did: string; followUri: string}>({
return useMutation<
void,
Error,
{did: string; followUri: string; skipOptimistic?: boolean}
>({
mutationFn: async ({followUri}) => {
return await agent.deleteFollow(followUri)
},
onMutate(variables) {
// optimstically update
if (!variables.skipOptimistic) {
// optimistically update
updateProfileShadow(variables.did, {
followingUri: undefined,
})
}
},
onError(error, variables) {
if (!variables.skipOptimistic) {
// revert the optimistic update
updateProfileShadow(variables.did, {
followingUri: variables.followUri,
})
}
},
})
}
export function useProfileMuteMutation() {
export function useProfileMuteMutationQueue(
profile: Shadow<AppBskyActorDefs.ProfileViewDetailed>,
) {
const did = profile.did
const initialMuted = profile.viewer?.muted
const muteMutation = useProfileMuteMutation()
const unmuteMutation = useProfileUnmuteMutation()
const queueToggle = useToggleMutationQueue({
initialState: initialMuted,
runMutation: async (_prevMuted, shouldMute) => {
if (shouldMute) {
await muteMutation.mutateAsync({
did,
skipOptimistic: true,
})
return true
} else {
await unmuteMutation.mutateAsync({
did,
skipOptimistic: true,
})
return false
}
},
onSuccess(finalMuted) {
// finalize
updateProfileShadow(did, {muted: finalMuted})
},
})
const queueMute = useCallback(() => {
// optimistically update
updateProfileShadow(did, {
muted: true,
})
return queueToggle(true)
}, [did, queueToggle])
const queueUnmute = useCallback(() => {
// optimistically update
updateProfileShadow(did, {
muted: false,
})
return queueToggle(false)
}, [did, queueToggle])
return [queueMute, queueUnmute]
}
function useProfileMuteMutation() {
const {agent} = useSession()
const queryClient = useQueryClient()
return useMutation<void, Error, {did: string}>({
return useMutation<void, Error, {did: string; skipOptimistic?: boolean}>({
mutationFn: async ({did}) => {
await agent.mute(did)
},
onMutate(variables) {
// optimstically update
if (!variables.skipOptimistic) {
// optimistically update
updateProfileShadow(variables.did, {
muted: true,
})
}
},
onSuccess() {
queryClient.invalidateQueries({queryKey: RQKEY_MY_MUTED()})
},
onError(error, variables) {
if (!variables.skipOptimistic) {
// revert the optimistic update
updateProfileShadow(variables.did, {
muted: false,
})
}
},
})
}
export function useProfileUnmuteMutation() {
function useProfileUnmuteMutation() {
const {agent} = useSession()
return useMutation<void, Error, {did: string}>({
return useMutation<void, Error, {did: string; skipOptimistic?: boolean}>({
mutationFn: async ({did}) => {
await agent.unmute(did)
},
onMutate(variables) {
// optimstically update
if (!variables.skipOptimistic) {
// optimistically update
updateProfileShadow(variables.did, {
muted: false,
})
}
},
onError(error, variables) {
if (!variables.skipOptimistic) {
// revert the optimistic update
updateProfileShadow(variables.did, {
muted: true,
})
}
},
})
}
export function useProfileBlockMutation() {
export function useProfileBlockMutationQueue(
profile: Shadow<AppBskyActorDefs.ProfileViewDetailed>,
) {
const did = profile.did
const initialBlockingUri = profile.viewer?.blocking
const blockMutation = useProfileBlockMutation()
const unblockMutation = useProfileUnblockMutation()
const queueToggle = useToggleMutationQueue({
initialState: initialBlockingUri,
runMutation: async (prevBlockUri, shouldFollow) => {
if (shouldFollow) {
const {uri} = await blockMutation.mutateAsync({
did,
skipOptimistic: true,
})
return uri
} else {
if (prevBlockUri) {
await unblockMutation.mutateAsync({
did,
blockUri: prevBlockUri,
skipOptimistic: true,
})
}
return undefined
}
},
onSuccess(finalBlockingUri) {
// finalize
updateProfileShadow(did, {
blockingUri: finalBlockingUri,
})
},
})
const queueBlock = useCallback(() => {
// optimistically update
updateProfileShadow(did, {
blockingUri: 'pending',
})
return queueToggle(true)
}, [did, queueToggle])
const queueUnblock = useCallback(() => {
// optimistically update
updateProfileShadow(did, {
blockingUri: undefined,
})
return queueToggle(false)
}, [did, queueToggle])
return [queueBlock, queueUnblock]
}
function useProfileBlockMutation() {
const {agent, currentAccount} = useSession()
const queryClient = useQueryClient()
return useMutation<{uri: string; cid: string}, Error, {did: string}>({
return useMutation<
{uri: string; cid: string},
Error,
{did: string; skipOptimistic?: boolean}
>({
mutationFn: async ({did}) => {
if (!currentAccount) {
throw new Error('Not signed in')
@ -207,30 +400,40 @@ export function useProfileBlockMutation() {
)
},
onMutate(variables) {
// optimstically update
if (!variables.skipOptimistic) {
// optimistically update
updateProfileShadow(variables.did, {
blockingUri: 'pending',
})
}
},
onSuccess(data, variables) {
if (!variables.skipOptimistic) {
// finalize
updateProfileShadow(variables.did, {
blockingUri: data.uri,
})
}
queryClient.invalidateQueries({queryKey: RQKEY_MY_BLOCKED()})
},
onError(error, variables) {
if (!variables.skipOptimistic) {
// revert the optimistic update
updateProfileShadow(variables.did, {
blockingUri: undefined,
})
}
},
})
}
export function useProfileUnblockMutation() {
function useProfileUnblockMutation() {
const {agent, currentAccount} = useSession()
return useMutation<void, Error, {did: string; blockUri: string}>({
return useMutation<
void,
Error,
{did: string; blockUri: string; skipOptimistic?: boolean}
>({
mutationFn: async ({blockUri}) => {
if (!currentAccount) {
throw new Error('Not signed in')
@ -242,16 +445,20 @@ export function useProfileUnblockMutation() {
})
},
onMutate(variables) {
// optimstically update
if (!variables.skipOptimistic) {
// optimistically update
updateProfileShadow(variables.did, {
blockingUri: undefined,
})
}
},
onError(error, variables) {
if (!variables.skipOptimistic) {
// revert the optimistic update
updateProfileShadow(variables.did, {
blockingUri: variables.blockUri,
})
}
},
})
}

View File

@ -13,10 +13,7 @@ import {useWebMediaQueries} from 'lib/hooks/useWebMediaQueries'
import {useAnalytics} from 'lib/analytics/analytics'
import {Trans} from '@lingui/macro'
import {Shadow, useProfileShadow} from '#/state/cache/profile-shadow'
import {
useProfileFollowMutation,
useProfileUnfollowMutation,
} from '#/state/queries/profile'
import {useProfileFollowMutationQueue} from '#/state/queries/profile'
import {logger} from '#/logger'
type Props = {
@ -77,35 +74,32 @@ export function ProfileCard({
const pal = usePalette('default')
const [addingMoreSuggestions, setAddingMoreSuggestions] =
React.useState(false)
const {mutateAsync: follow} = useProfileFollowMutation()
const {mutateAsync: unfollow} = useProfileUnfollowMutation()
const [queueFollow, queueUnfollow] = useProfileFollowMutationQueue(profile)
const onToggleFollow = React.useCallback(async () => {
try {
if (
profile.viewer?.following &&
profile.viewer?.following !== 'pending'
) {
await unfollow({did: profile.did, followUri: profile.viewer.following})
} else if (
!profile.viewer?.following &&
profile.viewer?.following !== 'pending'
) {
if (profile.viewer?.following) {
await queueUnfollow()
} else {
setAddingMoreSuggestions(true)
await follow({did: profile.did})
await queueFollow()
await onFollowStateChange({did: profile.did, following: true})
setAddingMoreSuggestions(false)
track('Onboarding:SuggestedFollowFollowed')
}
} catch (e) {
logger.error('RecommendedFollows: failed to toggle following', {error: e})
} catch (e: any) {
if (e?.name !== 'AbortError') {
logger.error('RecommendedFollows: failed to toggle following', {
error: e,
})
}
} finally {
setAddingMoreSuggestions(false)
}
}, [
profile,
follow,
unfollow,
queueFollow,
queueUnfollow,
setAddingMoreSuggestions,
track,
onFollowStateChange,
@ -142,7 +136,6 @@ export function ProfileCard({
labelStyle={styles.followButton}
onPress={onToggleFollow}
label={profile.viewer?.following ? 'Unfollow' : 'Follow'}
withLoading={true}
/>
</View>
{profile.description ? (

View File

@ -3,10 +3,7 @@ import {StyleProp, TextStyle, View} from 'react-native'
import {AppBskyActorDefs} from '@atproto/api'
import {Button, ButtonType} from '../util/forms/Button'
import * as Toast from '../util/Toast'
import {
useProfileFollowMutation,
useProfileUnfollowMutation,
} from '#/state/queries/profile'
import {useProfileFollowMutationQueue} from '#/state/queries/profile'
import {Shadow} from '#/state/cache/types'
export function FollowButton({
@ -20,33 +17,27 @@ export function FollowButton({
profile: Shadow<AppBskyActorDefs.ProfileViewBasic>
labelStyle?: StyleProp<TextStyle>
}) {
const followMutation = useProfileFollowMutation()
const unfollowMutation = useProfileUnfollowMutation()
const [queueFollow, queueUnfollow] = useProfileFollowMutationQueue(profile)
const onPressFollow = async () => {
if (profile.viewer?.following) {
return
}
try {
await followMutation.mutateAsync({did: profile.did})
await queueFollow()
} catch (e: any) {
if (e?.name !== 'AbortError') {
Toast.show(`An issue occurred, please try again.`)
}
}
}
const onPressUnfollow = async () => {
if (!profile.viewer?.following) {
return
}
try {
await unfollowMutation.mutateAsync({
did: profile.did,
followUri: profile.viewer?.following,
})
await queueUnfollow()
} catch (e: any) {
if (e?.name !== 'AbortError') {
Toast.show(`An issue occurred, please try again.`)
}
}
}
if (!profile.viewer) {
return <View />
@ -59,7 +50,6 @@ export function FollowButton({
labelStyle={labelStyle}
onPress={onPressUnfollow}
label="Unfollow"
withLoading={true}
/>
)
} else {
@ -69,7 +59,6 @@ export function FollowButton({
labelStyle={labelStyle}
onPress={onPressFollow}
label="Follow"
withLoading={true}
/>
)
}

View File

@ -32,12 +32,9 @@ import {ProfileHeaderSuggestedFollows} from './ProfileHeaderSuggestedFollows'
import {useModalControls} from '#/state/modals'
import {useLightboxControls, ProfileImageLightbox} from '#/state/lightbox'
import {
useProfileFollowMutation,
useProfileUnfollowMutation,
useProfileMuteMutation,
useProfileUnmuteMutation,
useProfileBlockMutation,
useProfileUnblockMutation,
useProfileMuteMutationQueue,
useProfileBlockMutationQueue,
useProfileFollowMutationQueue,
} from '#/state/queries/profile'
import {usePalette} from 'lib/hooks/usePalette'
import {useAnalytics} from 'lib/analytics/analytics'
@ -130,12 +127,9 @@ function ProfileHeaderLoaded({
: undefined,
[profile],
)
const followMutation = useProfileFollowMutation()
const unfollowMutation = useProfileUnfollowMutation()
const muteMutation = useProfileMuteMutation()
const unmuteMutation = useProfileUnmuteMutation()
const blockMutation = useProfileBlockMutation()
const unblockMutation = useProfileUnblockMutation()
const [queueFollow, queueUnfollow] = useProfileFollowMutationQueue(profile)
const [queueMute, queueUnmute] = useProfileMuteMutationQueue(profile)
const [queueBlock, queueUnblock] = useProfileBlockMutationQueue(profile)
const onPressBack = React.useCallback(() => {
if (navigation.canGoBack()) {
@ -154,44 +148,39 @@ function ProfileHeaderLoaded({
}
}, [openLightbox, profile, moderation])
const onPressFollow = React.useCallback(async () => {
if (profile.viewer?.following) {
return
}
const onPressFollow = async () => {
try {
track('ProfileHeader:FollowButtonClicked')
await followMutation.mutateAsync({did: profile.did})
await queueFollow()
Toast.show(
`Following ${sanitizeDisplayName(
profile.displayName || profile.handle,
)}`,
)
} catch (e: any) {
if (e?.name !== 'AbortError') {
logger.error('Failed to follow', {error: String(e)})
Toast.show(`There was an issue! ${e.toString()}`)
}
}, [followMutation, profile, track])
const onPressUnfollow = React.useCallback(async () => {
if (!profile.viewer?.following) {
return
}
}
const onPressUnfollow = async () => {
try {
track('ProfileHeader:UnfollowButtonClicked')
await unfollowMutation.mutateAsync({
did: profile.did,
followUri: profile.viewer?.following,
})
await queueUnfollow()
Toast.show(
`No longer following ${sanitizeDisplayName(
profile.displayName || profile.handle,
)}`,
)
} catch (e: any) {
if (e?.name !== 'AbortError') {
logger.error('Failed to unfollow', {error: String(e)})
Toast.show(`There was an issue! ${e.toString()}`)
}
}, [unfollowMutation, profile, track])
}
}
const onPressEditProfile = React.useCallback(() => {
track('ProfileHeader:EditProfileButtonClicked')
@ -218,24 +207,28 @@ function ProfileHeaderLoaded({
const onPressMuteAccount = React.useCallback(async () => {
track('ProfileHeader:MuteAccountButtonClicked')
try {
await muteMutation.mutateAsync({did: profile.did})
await queueMute()
Toast.show('Account muted')
} catch (e: any) {
if (e?.name !== 'AbortError') {
logger.error('Failed to mute account', {error: e})
Toast.show(`There was an issue! ${e.toString()}`)
}
}, [track, muteMutation, profile])
}
}, [track, queueMute])
const onPressUnmuteAccount = React.useCallback(async () => {
track('ProfileHeader:UnmuteAccountButtonClicked')
try {
await unmuteMutation.mutateAsync({did: profile.did})
await queueUnmute()
Toast.show('Account unmuted')
} catch (e: any) {
if (e?.name !== 'AbortError') {
logger.error('Failed to unmute account', {error: e})
Toast.show(`There was an issue! ${e.toString()}`)
}
}, [track, unmuteMutation, profile])
}
}, [track, queueUnmute])
const onPressBlockAccount = React.useCallback(async () => {
track('ProfileHeader:BlockAccountButtonClicked')
@ -245,19 +238,18 @@ function ProfileHeaderLoaded({
message:
'Blocked accounts cannot reply in your threads, mention you, or otherwise interact with you.',
onPressConfirm: async () => {
if (profile.viewer?.blocking) {
return
}
try {
await blockMutation.mutateAsync({did: profile.did})
await queueBlock()
Toast.show('Account blocked')
} catch (e: any) {
if (e?.name !== 'AbortError') {
logger.error('Failed to block account', {error: e})
Toast.show(`There was an issue! ${e.toString()}`)
}
}
},
})
}, [track, blockMutation, profile, openModal])
}, [track, queueBlock, openModal])
const onPressUnblockAccount = React.useCallback(async () => {
track('ProfileHeader:UnblockAccountButtonClicked')
@ -267,22 +259,18 @@ function ProfileHeaderLoaded({
message:
'The account will be able to interact with you after unblocking.',
onPressConfirm: async () => {
if (!profile.viewer?.blocking) {
return
}
try {
await unblockMutation.mutateAsync({
did: profile.did,
blockUri: profile.viewer.blocking,
})
await queueUnblock()
Toast.show('Account unblocked')
} catch (e: any) {
if (e?.name !== 'AbortError') {
logger.error('Failed to unblock account', {error: e})
Toast.show(`There was an issue! ${e.toString()}`)
}
}
},
})
}, [track, unblockMutation, profile, openModal])
}, [track, queueUnblock, openModal])
const onPressReportAccount = React.useCallback(() => {
track('ProfileHeader:ReportAccountButtonClicked')

View File

@ -26,10 +26,7 @@ import {isWeb} from 'platform/detection'
import {useModerationOpts} from '#/state/queries/preferences'
import {useSuggestedFollowsByActorQuery} from '#/state/queries/suggested-follows'
import {useProfileShadow} from '#/state/cache/profile-shadow'
import {
useProfileFollowMutation,
useProfileUnfollowMutation,
} from '#/state/queries/profile'
import {useProfileFollowMutationQueue} from '#/state/queries/profile'
const OUTER_PADDING = 10
const INNER_PADDING = 14
@ -208,34 +205,28 @@ function SuggestedFollow({
const pal = usePalette('default')
const moderationOpts = useModerationOpts()
const profile = useProfileShadow(profileUnshadowed, dataUpdatedAt)
const followMutation = useProfileFollowMutation()
const unfollowMutation = useProfileUnfollowMutation()
const [queueFollow, queueUnfollow] = useProfileFollowMutationQueue(profile)
const onPressFollow = React.useCallback(async () => {
if (profile.viewer?.following) {
return
}
try {
track('ProfileHeader:SuggestedFollowFollowed')
await followMutation.mutateAsync({did: profile.did})
await queueFollow()
} catch (e: any) {
if (e?.name !== 'AbortError') {
Toast.show('An issue occurred, please try again.')
}
}, [followMutation, profile, track])
}
}, [queueFollow, track])
const onPressUnfollow = React.useCallback(async () => {
if (!profile.viewer?.following) {
return
}
try {
await unfollowMutation.mutateAsync({
did: profile.did,
followUri: profile.viewer?.following,
})
await queueUnfollow()
} catch (e: any) {
if (e?.name !== 'AbortError') {
Toast.show('An issue occurred, please try again.')
}
}, [unfollowMutation, profile])
}
}, [queueUnfollow])
if (!moderationOpts) {
return null
@ -284,7 +275,6 @@ function SuggestedFollow({
type="inverted"
labelStyle={{textAlign: 'center'}}
onPress={following ? onPressUnfollow : onPressFollow}
withLoading
/>
</View>
</Link>