perf: clean up mobile pairing attempts (#4058)
This commit is contained in:
parent
9d352818fe
commit
2067bebf50
|
|
@ -1,9 +1,13 @@
|
|||
import { useCallback, useRef, useState } from 'react'
|
||||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { View, Text, StyleSheet, Pressable, ActivityIndicator, BackHandler } from 'react-native'
|
||||
import { useSafeAreaInsets } from 'react-native-safe-area-context'
|
||||
import { useFocusEffect, useLocalSearchParams, useRouter } from 'expo-router'
|
||||
import { ChevronLeft } from 'lucide-react-native'
|
||||
import { resolvePairConfirmRouteState } from '../src/transport/pair-confirm-state'
|
||||
import {
|
||||
startPairingConnectionAttempt,
|
||||
type PairingConnectionAttempt
|
||||
} from '../src/transport/pairing-connection-attempt'
|
||||
import { connect } from '../src/transport/rpc-client'
|
||||
import { saveHost, getNextHostName } from '../src/transport/host-store'
|
||||
import type { ConnectionLogEntry, RpcResponse } from '../src/transport/types'
|
||||
|
|
@ -30,6 +34,8 @@ export default function PairConfirmScreen() {
|
|||
// over the initial state setter) always sees the freshest list and we
|
||||
// batch fewer setState calls when entries arrive in bursts.
|
||||
const logsRef = useRef<ConnectionLogEntry[]>([])
|
||||
const mountedRef = useRef(true)
|
||||
const activePairingAttemptRef = useRef<PairingConnectionAttempt | null>(null)
|
||||
|
||||
const routeState = resolvePairConfirmRouteState(params.code)
|
||||
const offer = routeState.offer
|
||||
|
|
@ -54,35 +60,54 @@ export default function PairConfirmScreen() {
|
|||
}, [cancel])
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
mountedRef.current = false
|
||||
activePairingAttemptRef.current?.dispose()
|
||||
activePairingAttemptRef.current = null
|
||||
}
|
||||
}, [])
|
||||
|
||||
async function confirm() {
|
||||
if (!offer) return
|
||||
setStatus('connecting')
|
||||
logsRef.current = []
|
||||
setLogs([])
|
||||
let client: ReturnType<typeof connect> | null = null
|
||||
activePairingAttemptRef.current?.dispose()
|
||||
|
||||
// Why: split the try/catch around the network call vs the local save
|
||||
// so a Keychain or AsyncStorage failure doesn't masquerade as a
|
||||
// "Cannot connect" error.
|
||||
let response: RpcResponse
|
||||
let timedOut = false
|
||||
const overallTimer = setTimeout(() => {
|
||||
timedOut = true
|
||||
client?.close()
|
||||
}, PAIRING_OVERALL_TIMEOUT_MS)
|
||||
const attempt = startPairingConnectionAttempt({
|
||||
timeoutMs: PAIRING_OVERALL_TIMEOUT_MS,
|
||||
closeClient: () => client?.close()
|
||||
})
|
||||
activePairingAttemptRef.current = attempt
|
||||
try {
|
||||
client = connect(offer.endpoint, offer.deviceToken, offer.publicKeyB64, {
|
||||
onLog: (entry) => {
|
||||
if (!mountedRef.current || activePairingAttemptRef.current !== attempt) return
|
||||
logsRef.current = [...logsRef.current, entry]
|
||||
setLogs(logsRef.current)
|
||||
}
|
||||
})
|
||||
response = await client.sendRequest('status.get')
|
||||
clearTimeout(overallTimer)
|
||||
client.close()
|
||||
client = null
|
||||
const attemptIsCurrent = activePairingAttemptRef.current === attempt
|
||||
attempt.dispose()
|
||||
if (activePairingAttemptRef.current === attempt) {
|
||||
activePairingAttemptRef.current = null
|
||||
}
|
||||
if (!mountedRef.current || !attemptIsCurrent) return
|
||||
} catch (err) {
|
||||
clearTimeout(overallTimer)
|
||||
const timedOut = attempt.timedOut
|
||||
const attemptIsCurrent = activePairingAttemptRef.current === attempt
|
||||
attempt.dispose()
|
||||
if (activePairingAttemptRef.current === attempt) {
|
||||
activePairingAttemptRef.current = null
|
||||
}
|
||||
if (!mountedRef.current || !attemptIsCurrent) return
|
||||
console.warn('[pair-confirm] connect failed', err)
|
||||
setStatus('error')
|
||||
setErrorMessage(
|
||||
|
|
@ -90,11 +115,11 @@ export default function PairConfirmScreen() {
|
|||
? `Couldn't connect within ${PAIRING_OVERALL_TIMEOUT_MS / 1000}s — see log below for where it stalled`
|
||||
: 'Cannot connect — check that your computer is on the same network'
|
||||
)
|
||||
client?.close()
|
||||
return
|
||||
}
|
||||
|
||||
if (!response.ok) {
|
||||
if (!mountedRef.current) return
|
||||
setStatus('error')
|
||||
setErrorMessage(
|
||||
response.error.code === 'unauthorized'
|
||||
|
|
@ -115,8 +140,10 @@ export default function PairConfirmScreen() {
|
|||
publicKeyB64: offer.publicKeyB64,
|
||||
lastConnected: Date.now()
|
||||
})
|
||||
if (!mountedRef.current) return
|
||||
router.replace(`/h/${hostId}`)
|
||||
} catch (err) {
|
||||
if (!mountedRef.current) return
|
||||
console.warn('[pair-confirm] save failed', err)
|
||||
setStatus('error')
|
||||
setErrorMessage(
|
||||
|
|
|
|||
|
|
@ -1,10 +1,14 @@
|
|||
import { useState, useRef, useCallback } from 'react'
|
||||
import { useState, useRef, useCallback, useEffect } from 'react'
|
||||
import { View, Text, StyleSheet, Pressable, ActivityIndicator, Linking } from 'react-native'
|
||||
import { useSafeAreaInsets } from 'react-native-safe-area-context'
|
||||
import { CameraView, useCameraPermissions } from 'expo-camera'
|
||||
import { useRouter } from 'expo-router'
|
||||
import { ChevronLeft, Clipboard as ClipboardIcon, QrCode } from 'lucide-react-native'
|
||||
import { decodePairingUrl, parsePairingCode } from '../src/transport/pairing'
|
||||
import {
|
||||
startPairingConnectionAttempt,
|
||||
type PairingConnectionAttempt
|
||||
} from '../src/transport/pairing-connection-attempt'
|
||||
import { connect } from '../src/transport/rpc-client'
|
||||
import { saveHost, getNextHostName } from '../src/transport/host-store'
|
||||
import type { ConnectionLogEntry, PairingOffer, RpcResponse } from '../src/transport/types'
|
||||
|
|
@ -38,6 +42,16 @@ export default function PairScanScreen() {
|
|||
const [logs, setLogs] = useState<ConnectionLogEntry[]>([])
|
||||
const logsRef = useRef<ConnectionLogEntry[]>([])
|
||||
const processingRef = useRef(false)
|
||||
const mountedRef = useRef(true)
|
||||
const activePairingAttemptRef = useRef<PairingConnectionAttempt | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
mountedRef.current = false
|
||||
activePairingAttemptRef.current?.dispose()
|
||||
activePairingAttemptRef.current = null
|
||||
}
|
||||
}, [])
|
||||
|
||||
const handleBarCodeScanned = useCallback(
|
||||
({ data }: { data: string }) => {
|
||||
|
|
@ -78,30 +92,41 @@ export default function PairScanScreen() {
|
|||
logsRef.current = []
|
||||
setLogs([])
|
||||
let client: ReturnType<typeof connect> | null = null
|
||||
activePairingAttemptRef.current?.dispose()
|
||||
|
||||
// Why: split the try/catch around the network call vs the local save
|
||||
// so a Keychain or AsyncStorage failure doesn't masquerade as a
|
||||
// "Cannot connect — same network?" error. Pairing reached the
|
||||
// desktop fine; the failure is local persistence.
|
||||
let response: RpcResponse
|
||||
let timedOut = false
|
||||
const overallTimer = setTimeout(() => {
|
||||
timedOut = true
|
||||
client?.close()
|
||||
}, PAIRING_OVERALL_TIMEOUT_MS)
|
||||
const attempt = startPairingConnectionAttempt({
|
||||
timeoutMs: PAIRING_OVERALL_TIMEOUT_MS,
|
||||
closeClient: () => client?.close()
|
||||
})
|
||||
activePairingAttemptRef.current = attempt
|
||||
try {
|
||||
client = connect(offer.endpoint, offer.deviceToken, offer.publicKeyB64, {
|
||||
onLog: (entry) => {
|
||||
if (!mountedRef.current || activePairingAttemptRef.current !== attempt) return
|
||||
logsRef.current = [...logsRef.current, entry]
|
||||
setLogs(logsRef.current)
|
||||
}
|
||||
})
|
||||
response = await client.sendRequest('status.get')
|
||||
clearTimeout(overallTimer)
|
||||
client.close()
|
||||
client = null
|
||||
const attemptIsCurrent = activePairingAttemptRef.current === attempt
|
||||
attempt.dispose()
|
||||
if (activePairingAttemptRef.current === attempt) {
|
||||
activePairingAttemptRef.current = null
|
||||
}
|
||||
if (!mountedRef.current || !attemptIsCurrent) return
|
||||
} catch (err) {
|
||||
clearTimeout(overallTimer)
|
||||
const timedOut = attempt.timedOut
|
||||
const attemptIsCurrent = activePairingAttemptRef.current === attempt
|
||||
attempt.dispose()
|
||||
if (activePairingAttemptRef.current === attempt) {
|
||||
activePairingAttemptRef.current = null
|
||||
}
|
||||
if (!mountedRef.current || !attemptIsCurrent) return
|
||||
console.warn('[pair] connect failed', err)
|
||||
setStatus('error')
|
||||
setErrorMessage(
|
||||
|
|
@ -110,11 +135,11 @@ export default function PairScanScreen() {
|
|||
: 'Cannot connect — check that your computer is on the same network'
|
||||
)
|
||||
processingRef.current = false
|
||||
client?.close()
|
||||
return
|
||||
}
|
||||
|
||||
if (!response.ok) {
|
||||
if (!mountedRef.current) return
|
||||
if (response.error.code === 'unauthorized') {
|
||||
setStatus('error')
|
||||
setErrorMessage('Authentication failed — token may be expired')
|
||||
|
|
@ -138,8 +163,10 @@ export default function PairScanScreen() {
|
|||
publicKeyB64: offer.publicKeyB64,
|
||||
lastConnected: Date.now()
|
||||
})
|
||||
if (!mountedRef.current) return
|
||||
router.replace(`/h/${hostId}`)
|
||||
} catch (err) {
|
||||
if (!mountedRef.current) return
|
||||
console.warn('[pair] save failed', err)
|
||||
setStatus('error')
|
||||
setErrorMessage(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,41 @@
|
|||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { startPairingConnectionAttempt } from './pairing-connection-attempt'
|
||||
|
||||
describe('pairing connection attempt cleanup', () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('closes the temporary client when the overall pairing timeout fires', () => {
|
||||
vi.useFakeTimers()
|
||||
const closeClient = vi.fn()
|
||||
|
||||
const attempt = startPairingConnectionAttempt({ timeoutMs: 25_000, closeClient })
|
||||
|
||||
expect(attempt.timedOut).toBe(false)
|
||||
vi.advanceTimersByTime(24_999)
|
||||
expect(closeClient).not.toHaveBeenCalled()
|
||||
|
||||
vi.advanceTimersByTime(1)
|
||||
expect(attempt.timedOut).toBe(true)
|
||||
expect(closeClient).toHaveBeenCalledTimes(1)
|
||||
|
||||
attempt.dispose()
|
||||
expect(closeClient).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('clears the timeout and closes the temporary client when disposed early', () => {
|
||||
vi.useFakeTimers()
|
||||
const closeClient = vi.fn()
|
||||
|
||||
const attempt = startPairingConnectionAttempt({ timeoutMs: 25_000, closeClient })
|
||||
attempt.dispose()
|
||||
|
||||
expect(attempt.timedOut).toBe(false)
|
||||
expect(closeClient).toHaveBeenCalledTimes(1)
|
||||
|
||||
vi.advanceTimersByTime(25_000)
|
||||
expect(attempt.timedOut).toBe(false)
|
||||
expect(closeClient).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,46 @@
|
|||
export type PairingConnectionAttempt = {
|
||||
readonly timedOut: boolean
|
||||
dispose: () => void
|
||||
}
|
||||
|
||||
export function startPairingConnectionAttempt({
|
||||
timeoutMs,
|
||||
closeClient
|
||||
}: {
|
||||
timeoutMs: number
|
||||
closeClient: () => void
|
||||
}): PairingConnectionAttempt {
|
||||
let disposed = false
|
||||
let clientClosed = false
|
||||
let timedOut = false
|
||||
let timer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
function closeClientOnce() {
|
||||
if (clientClosed) return
|
||||
clientClosed = true
|
||||
closeClient()
|
||||
}
|
||||
|
||||
function dispose() {
|
||||
if (disposed) return
|
||||
disposed = true
|
||||
if (timer) {
|
||||
clearTimeout(timer)
|
||||
timer = null
|
||||
}
|
||||
closeClientOnce()
|
||||
}
|
||||
|
||||
timer = setTimeout(() => {
|
||||
timer = null
|
||||
timedOut = true
|
||||
dispose()
|
||||
}, timeoutMs)
|
||||
|
||||
return {
|
||||
get timedOut() {
|
||||
return timedOut
|
||||
},
|
||||
dispose
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue