[SDK] Address stack review feedback (#11389)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Samuel Newman
2026-08-13 22:26:23 +03:00
committed by GitHub
parent 3677c98873
commit b89167f07c
8 changed files with 69 additions and 22 deletions
+6 -1
View File
@@ -40,7 +40,12 @@ export function createLexClient(
* Requests use PLAIN `fetch`, not `networkAwareFetch`: the host is untrusted * Requests use PLAIN `fetch`, not `networkAwareFetch`: the host is untrusted
* input, and a typo'd or dead service must not be reported as the app losing * input, and a typo'd or dead service must not be reported as the app losing
* network reachability. * network reachability.
*
* `appLabelers: null` suppresses the global `Client.appLabelers` static: these
* are `com.atproto.server` calls to a host the user typed, which have no use
* for moderation labels, and the header would disclose the app's configured
* moderation authorities to an arbitrary third-party server.
*/ */
export function createServiceClient(service: string): Client { export function createServiceClient(service: string): Client {
return createLexClient({service}) return createLexClient({service}, {appLabelers: null})
} }
+9
View File
@@ -27,6 +27,15 @@ const TRUSTED_REGEX = new RegExp(
)})|/|#)`, )})|/|#)`,
) )
export function canParseUrl(url: string | URL, base?: string | URL): boolean {
try {
new URL(url, base)
return true
} catch {
return false
}
}
export function isValidDomain(str: string): boolean { export function isValidDomain(str: string): boolean {
return !!TLDs.find(tld => { return !!TLDs.find(tld => {
let i = str.lastIndexOf(tld) let i = str.lastIndexOf(tld)
+3 -1
View File
@@ -169,7 +169,9 @@ export const LoginForm = ({
) )
} else { } else {
logger.warn('Failed to login', {error: errMsg}) logger.warn('Failed to login', {error: errMsg})
setError(cleanError(errMsg)) /* the error object, not its stringification: cleanError only
* extracts the clean server message from a live LexError */
setError(cleanError(err))
} }
} }
} }
+3 -2
View File
@@ -354,7 +354,6 @@ export function useSubmitSignup() {
onboardingDispatch({type: 'start'}) onboardingDispatch({type: 'start'})
} catch (err) { } catch (err) {
const e = err as Error const e = err as Error
let errMsg = e.toString()
if ( if (
matchXrpcError(e, com.atproto.server.createAccount) === matchXrpcError(e, com.atproto.server.createAccount) ===
'InvalidInviteCode' 'InvalidInviteCode'
@@ -368,7 +367,9 @@ export function useSubmitSignup() {
return return
} }
const error = cleanError(errMsg) /* the error object, not its stringification: cleanError only extracts
* the clean server message from a live LexError */
const error = cleanError(e)
const isHandleError = error.toLowerCase().includes('handle') const isHandleError = error.toLowerCase().includes('handle')
dispatch({type: 'setIsLoading', value: false}) dispatch({type: 'setIsLoading', value: false})
+7 -7
View File
@@ -206,21 +206,21 @@ export function useProfileUpdateMutation() {
appviewClient, appviewClient,
profile.did, profile.did,
checkCommitted || checkCommitted ||
(profile => { (fresh => {
if (typeof newUserAvatar !== 'undefined') { if (typeof newUserAvatar !== 'undefined') {
if (newUserAvatar === null && profile.avatar) { if (newUserAvatar === null && fresh.avatar) {
// url hasn't cleared yet // url hasn't cleared yet
return false return false
} else if (profile.avatar === profile.avatar) { } else if (fresh.avatar === profile.avatar) {
// url hasn't changed yet // url hasn't changed yet
return false return false
} }
} }
if (typeof newUserBanner !== 'undefined') { if (typeof newUserBanner !== 'undefined') {
if (newUserBanner === null && profile.banner) { if (newUserBanner === null && fresh.banner) {
// url hasn't cleared yet // url hasn't cleared yet
return false return false
} else if (profile.banner === profile.banner) { } else if (fresh.banner === profile.banner) {
// url hasn't changed yet // url hasn't changed yet
return false return false
} }
@@ -229,8 +229,8 @@ export function useProfileUpdateMutation() {
return true return true
} }
return ( return (
profile.displayName === updates.displayName && fresh.displayName === updates.displayName &&
profile.description === updates.description fresh.description === updates.description
) )
}), }),
) )
+25 -1
View File
@@ -157,7 +157,19 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
sessionEvent: AtpSessionEvent, sessionEvent: AtpSessionEvent,
sessionData?: SessionData, sessionData?: SessionData,
) => { ) => {
if (sessionEvent === 'update' && sessionData) { /*
* Only the live bundle may reset the expiry-rescue bookkeeping: a stale
* bundle's late update would otherwise clear the failed-generation set
* that bounds the rescue loop. (Its dispatch below is separately dropped
* by the reducer's identity guard.)
*/
if (
sessionEvent === 'update' &&
sessionData &&
(store.getState().currentBundleState.bundle as unknown as
| SessionBundle
| PublicSessionBundle) === bundle
) {
failedExpiryTokensRef.current.get(accountDid)?.clear() failedExpiryTokensRef.current.get(accountDid)?.clear()
} }
@@ -505,6 +517,18 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
if (after === before) { if (after === before) {
throw new Error('Failed to refresh session') throw new Error('Failed to refresh session')
} }
/*
* The user may have logged out or switched accounts while the refresh was
* in flight. Reporting success then would run the caller's success path
* (dialogs closing, success toasts, SignupQueued advancing) against an
* account that is no longer active, so a stale bundle rejects instead.
*/
if (
(store.getState().currentBundleState.bundle as unknown) !==
(bundle as unknown)
) {
throw new Error('The session changed while it was being refreshed')
}
/* /*
* The session's `onUpdated` hook dispatches the new tokens into the store, * The session's `onUpdated` hook dispatches the new tokens into the store,
* but that lands a render away; this snapshot exposes them immediately. * but that lands a render away; this snapshot exposes them immediately.
+11 -3
View File
@@ -6,6 +6,7 @@ import {
} from '@atproto/lex-password-session' } from '@atproto/lex-password-session'
import {PUBLIC_BSKY_SERVICE} from '#/lib/constants' import {PUBLIC_BSKY_SERVICE} from '#/lib/constants'
import {canParseUrl} from '#/lib/strings/url-helpers'
import {logger} from '#/logger' import {logger} from '#/logger'
import {prefetchAgeAssuranceServerData} from '#/ageAssurance/data' import {prefetchAgeAssuranceServerData} from '#/ageAssurance/data'
import {features} from '#/analytics' import {features} from '#/analytics'
@@ -95,9 +96,16 @@ export function buildBundle(
session: PasswordSession, session: PasswordSession,
storedPdsUrl?: string, storedPdsUrl?: string,
): SessionBundle { ): SessionBundle {
const agent = storedPdsUrl /*
? routeSessionToPds(session, storedPdsUrl) * The stored url is persisted data and may be malformed (legacy writes,
: session * corruption). `routeSessionToPds` feeds it to `new URL()` on every request,
* so an invalid value would throw from every client call; discard it here
* and let the session route against its own service instead.
*/
const agent =
storedPdsUrl && canParseUrl(storedPdsUrl)
? routeSessionToPds(session, storedPdsUrl)
: session
return { return {
session, session,
appviewClient: buildAppviewClient(agent), appviewClient: buildAppviewClient(agent),
+5 -7
View File
@@ -1,5 +1,7 @@
import {getPdsEndpoint, isValidDidDoc} from '@atproto/common-web' import {
import {type SessionData} from '@atproto/lex-password-session' extractPdsEndpoint,
type SessionData,
} from '@atproto/lex-password-session'
import {jwtDecode} from 'jwt-decode' import {jwtDecode} from 'jwt-decode'
import {BSKY_SERVICE} from '#/lib/constants' import {BSKY_SERVICE} from '#/lib/constants'
@@ -40,11 +42,7 @@ export function sessionDataToSessionAccount(
return undefined return undefined
} }
const normalizedService = new URL(service).toString() const normalizedService = new URL(service).toString()
const didDocPdsUrl = const pdsUrl = extractPdsEndpoint(session.didDoc) ?? storedPdsUrl
session.didDoc && isValidDidDoc(session.didDoc)
? getPdsEndpoint(session.didDoc)
: undefined
const pdsUrl = didDocPdsUrl ?? storedPdsUrl
return { return {
service: normalizedService, service: normalizedService,
did: session.did, did: session.did,