survive session-bundle rebuild in chat, feeds, prefs, and age assurance
Review fixes (PR #11182 round 2). Client identity is no longer stable across token refresh - on web, a cross-tab sync rebuilds the bundle and disposes the old clients (whose fetch now throws) - so anything that captured a client at construction time must re-point at the live one: - MessagesEventBus and Convo get updateClient(); providers sync it via effect. No remount, so poll cursors and optimistic pendingMessages survive. - FeedAPI implementations get setClient(); the post-feed queryFn and pollLatest re-point page-held apis before fetching (Merge/Home apis are stateful across pages, so they cannot be rebuilt per fetch). - usePreferencesQuery applies fetched labeler dids to the live appview client (applyLabelersToClient, factored from session/moderation), restoring the old BskyAgent.getPreferences header side effect. - ageAssurance redirect overlay/dialog polling is mount-only with render-synced refs; a client swap mid-poll no longer latches the unmounted flag and strands the overlay. - Convo message-failure classification defers to lex's shouldRetry() instead of treating all status-less errors as recoverable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -21,7 +21,6 @@ import {
|
||||
ACTIVE_POLL_INTERVAL,
|
||||
BACKGROUND_POLL_INTERVAL,
|
||||
INACTIVE_TIMEOUT,
|
||||
NETWORK_FAILURE_STATUSES,
|
||||
} from '#/state/messages/convo/const'
|
||||
import {
|
||||
type ConvoDispatch,
|
||||
@@ -176,6 +175,7 @@ export class Convo {
|
||||
this.setupPlaceholderData(params.placeholderData)
|
||||
}
|
||||
|
||||
this.updateClient = this.updateClient.bind(this)
|
||||
this.setConvo = this.setConvo.bind(this)
|
||||
this.subscribe = this.subscribe.bind(this)
|
||||
this.getSnapshot = this.getSnapshot.bind(this)
|
||||
@@ -194,6 +194,18 @@ export class Convo {
|
||||
this.updateLockStatus = this.updateLockStatus.bind(this)
|
||||
}
|
||||
|
||||
/**
|
||||
* Swap in a fresh chat client. On web, a cross-tab token sync rebuilds the
|
||||
* session bundle (new client identities, same DID) and disposes the old one,
|
||||
* whose fetch then throws. Every request reads `this.chatClient` per call, so
|
||||
* reassigning the field keeps the convo alive without a reset (which would
|
||||
* drop optimistic `pendingMessages`). Same-DID rebuild keeps `senderUserDid`
|
||||
* valid, so it is intentionally left untouched.
|
||||
*/
|
||||
updateClient(client: Client) {
|
||||
this.chatClient = client
|
||||
}
|
||||
|
||||
private commit() {
|
||||
this.snapshot = undefined
|
||||
this.subscribers.forEach(subscriber => subscriber())
|
||||
@@ -1198,14 +1210,13 @@ export class Convo {
|
||||
const status = getErrorStatus(e)
|
||||
if (isXrpcError(e)) {
|
||||
/*
|
||||
* A status-less xrpc error is a network/transport failure (lex throws
|
||||
* `XrpcInternalError`, which carries no HTTP status). The old bridge
|
||||
* represented the same case with a sentinel `status` of `1`, which is a
|
||||
* member of `NETWORK_FAILURE_STATUSES` - so a network failure was
|
||||
* `recoverable`. Preserve that by treating `undefined` status the same
|
||||
* as a network-failure status here.
|
||||
* Defer to lex's own retry classification: transient statuses
|
||||
* (408/425/429/5xx) and transport/fetch failures are recoverable, while
|
||||
* permanent statuses and internal errors are not. This is the lex-native
|
||||
* analogue of the old `NETWORK_FAILURE_STATUSES` check, and correctly
|
||||
* excludes status-less internal/validation errors that are not retryable.
|
||||
*/
|
||||
if (status === undefined || NETWORK_FAILURE_STATUSES.includes(status)) {
|
||||
if (e.shouldRetry()) {
|
||||
this.pendingMessageFailure = 'recoverable'
|
||||
} else {
|
||||
this.pendingMessageFailure = 'unrecoverable'
|
||||
|
||||
@@ -2,7 +2,3 @@ export const ACTIVE_POLL_INTERVAL = 4e3
|
||||
export const MESSAGE_SCREEN_POLL_INTERVAL = 30e3
|
||||
export const BACKGROUND_POLL_INTERVAL = 60e3
|
||||
export const INACTIVE_TIMEOUT = 60e3 * 5
|
||||
|
||||
export const NETWORK_FAILURE_STATUSES = [
|
||||
1, 408, 425, 429, 500, 502, 503, 504, 522, 524,
|
||||
]
|
||||
|
||||
@@ -101,6 +101,10 @@ export function ConvoProvider({
|
||||
const service = useSyncExternalStore(convo.subscribe, convo.getSnapshot)
|
||||
const {mutate: markAsRead} = useMarkAsReadMutation()
|
||||
|
||||
useEffect(() => {
|
||||
convo.updateClient(chatClient)
|
||||
}, [convo, chatClient])
|
||||
|
||||
const appState = useAppState()
|
||||
const isActive = appState === 'active'
|
||||
useFocusEffect(
|
||||
|
||||
@@ -42,6 +42,17 @@ export class MessagesEventBus {
|
||||
this.init()
|
||||
}
|
||||
|
||||
/**
|
||||
* Swap in a fresh chat client. On web, a cross-tab token sync rebuilds the
|
||||
* session bundle (new client identities, same DID) and disposes the old one,
|
||||
* whose fetch then throws. Every request reads `this.chatClient` per call, so
|
||||
* reassigning the field is enough to keep polling alive without tearing down
|
||||
* the bus and its in-memory poll cursor.
|
||||
*/
|
||||
updateClient(client: Client) {
|
||||
this.chatClient = client
|
||||
}
|
||||
|
||||
requestPollInterval(interval: number) {
|
||||
const id = nanoid()
|
||||
this.requestedPollIntervals.set(id, interval)
|
||||
|
||||
@@ -50,6 +50,10 @@ export function MessagesEventBusProviderInner({
|
||||
}),
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
bus.updateClient(chatClient)
|
||||
}, [bus, chatClient])
|
||||
|
||||
useEffect(() => {
|
||||
bus.resume()
|
||||
|
||||
|
||||
@@ -201,6 +201,15 @@ export function usePostFeedQuery(
|
||||
cursor: undefined,
|
||||
}
|
||||
|
||||
/*
|
||||
* A page-held api captured its client at construction. On web, a
|
||||
* cross-tab token sync rebuilds the session bundle and disposes the old
|
||||
* client (whose fetch then throws), so re-point the api at the current
|
||||
* client before fetching. Safe because the swap is same-DID (fresh
|
||||
* tokens only), so the stateful Merge/Home api's pagination is unaffected.
|
||||
*/
|
||||
api.setClient(client)
|
||||
|
||||
const res = await api.fetch({cursor, limit: fetchLimit})
|
||||
|
||||
/*
|
||||
@@ -422,7 +431,7 @@ export function usePostFeedQuery(
|
||||
return query
|
||||
}
|
||||
|
||||
export async function pollLatest(page: FeedPage | undefined) {
|
||||
export async function pollLatest(page: FeedPage | undefined, client: Client) {
|
||||
if (!page) {
|
||||
return false
|
||||
}
|
||||
@@ -431,6 +440,9 @@ export async function pollLatest(page: FeedPage | undefined) {
|
||||
}
|
||||
|
||||
logger.debug('usePostFeedQuery: pollLatest')
|
||||
// The page-held api may carry a disposed client after a session-bundle
|
||||
// rebuild - re-point it at the current client before peeking.
|
||||
page.api.setClient(client)
|
||||
const post = await page.api.peekLatest()
|
||||
if (post) {
|
||||
const slices = page.tuner.tune([post], {
|
||||
|
||||
@@ -38,8 +38,9 @@ import {
|
||||
type UsePreferencesQueryResponse,
|
||||
} from '#/state/queries/preferences/types'
|
||||
import {createQueryKey} from '#/state/queries/util'
|
||||
import {usePdsClient} from '#/state/session'
|
||||
import {useAppviewClient, usePdsClient} from '#/state/session'
|
||||
import {saveLabelers} from '#/state/session/agent-config'
|
||||
import {applyLabelersToClient} from '#/state/session/moderation'
|
||||
import {useAgeAssurance} from '#/ageAssurance'
|
||||
import {makeAgeRestrictedModerationPrefs} from '#/ageAssurance/util'
|
||||
import {useAnalytics} from '#/analytics'
|
||||
@@ -57,6 +58,7 @@ export const preferencesQueryKey = createQueryKey(
|
||||
|
||||
export function usePreferencesQuery() {
|
||||
const client = usePdsClient()
|
||||
const appviewClient = useAppviewClient()
|
||||
const aa = useAgeAssurance()
|
||||
|
||||
const query = useQuery({
|
||||
@@ -71,11 +73,19 @@ export function usePreferencesQuery() {
|
||||
} else {
|
||||
const res = await client.call(getPreferences)
|
||||
|
||||
const labelerDids = res.moderationPrefs.labelers.map(l => l.did)
|
||||
|
||||
// save to local storage to ensure there are labels on initial requests
|
||||
void saveLabelers(
|
||||
client.did,
|
||||
res.moderationPrefs.labelers.map(l => l.did),
|
||||
)
|
||||
void saveLabelers(client.did, labelerDids)
|
||||
|
||||
/*
|
||||
* Sync the subscribed labelers to the live appview client, mirroring the
|
||||
* old `BskyAgent.getPreferences` side effect (which called
|
||||
* `configureLabelersHeader`). Without this, subscribing/unsubscribing to
|
||||
* a labeler would not affect server-attached labels until the session
|
||||
* bundle is rebuilt.
|
||||
*/
|
||||
applyLabelersToClient(appviewClient, labelerDids)
|
||||
|
||||
const preferences: UsePreferencesQueryResponse = {
|
||||
...res,
|
||||
|
||||
@@ -24,6 +24,27 @@ function configureGlobalAppLabelers(dids: string[]) {
|
||||
Client.configure({appLabelers: dids as `did:${string}:${string}`[]})
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply an account's subscribed labeler DIDs to a live appview client. The lex
|
||||
* `Client` rebuilds the `atproto-accept-labelers` header per request, so this
|
||||
* takes effect on the very next request without a client rebuild.
|
||||
*
|
||||
* The Bluesky moderation labeler is always re-asserted as the base: sending ANY
|
||||
* `atproto-accept-labelers` header replaces the server-side default, and
|
||||
* `setLabelers` clears then re-adds, so the moderation DID must be included
|
||||
* explicitly to stay active.
|
||||
*/
|
||||
export function applyLabelersToClient(
|
||||
client: Client,
|
||||
subscribedDids: string[],
|
||||
) {
|
||||
const perAccount = subscribedDids.filter(did => did !== api.moderation.did)
|
||||
client.setLabelers([
|
||||
api.moderation.did,
|
||||
...perAccount,
|
||||
] as `did:${string}:${string}`[])
|
||||
}
|
||||
|
||||
export function configureModerationForGuest() {
|
||||
// This global mutation is *only* OK because this code is only relevant for testing.
|
||||
// Don't add any other global behavior here!
|
||||
@@ -51,18 +72,8 @@ export async function configureModerationForAccount(
|
||||
// The code below is actually relevant to production (and isn't global).
|
||||
const labelerDids = await readLabelers(account.did).catch(_ => {})
|
||||
if (labelerDids) {
|
||||
const perAccount = labelerDids.filter(did => did !== api.moderation.did)
|
||||
/*
|
||||
* Apply the per-account labelers to the appview client. It re-asserts the
|
||||
* Bluesky moderation labeler as its base because sending ANY
|
||||
* `atproto-accept-labelers` header replaces the server-side default -
|
||||
* `setLabelers` clears then re-adds, so the moderation DID must be included
|
||||
* explicitly to stay active.
|
||||
*/
|
||||
bundle.appviewClient.setLabelers([
|
||||
api.moderation.did,
|
||||
...perAccount,
|
||||
] as `did:${string}:${string}`[])
|
||||
// Apply the per-account labelers to the appview client.
|
||||
applyLabelersToClient(bundle.appviewClient, labelerDids)
|
||||
} else {
|
||||
// If there are no headers in the storage, we'll not send them on the initial requests.
|
||||
// If we wanted to fix this, we could block on the preferences query here.
|
||||
|
||||
Reference in New Issue
Block a user