diff --git a/mobile/src/transport/mobile-relay-invite-director.test.ts b/mobile/src/transport/mobile-relay-invite-director.test.ts index 04508024d07..77a0cdde270 100644 --- a/mobile/src/transport/mobile-relay-invite-director.test.ts +++ b/mobile/src/transport/mobile-relay-invite-director.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest' -import { resolvePairingInviteThroughDirector } from './mobile-relay-invite-director' +import { + RelayDirectorMoveNotNewerError, + resolvePairingInviteThroughDirector +} from './mobile-relay-invite-director' class FakeSocket { sent: string[] = [] @@ -59,7 +62,7 @@ describe('pairing invite director resolution', () => { }) }) - it('rejects same/older epochs and untrusted extra fields', async () => { + it('rejects malformed moves with untrusted extra fields', async () => { const socket = new FakeSocket() const resolving = resolvePairingInviteThroughDirector({ relay, @@ -75,6 +78,31 @@ describe('pairing invite director resolution', () => { }) }) - await expect(resolving).rejects.toThrow(/not strictly newer/) + await expect(resolving).rejects.toThrow(/invalid relay director move/) + }) + + it('describes a same-epoch move without accepting it', async () => { + const socket = new FakeSocket() + const resolving = resolvePairingInviteThroughDirector({ + relay, + createSocket: () => socket as unknown as WebSocket + }) + socket.onmessage?.({ + data: JSON.stringify({ + type: 'relay-moved', + v: 1, + cellUrl: relay.cellUrl, + assignmentEpoch: relay.assignmentEpoch + }) + }) + + const error = await resolving.catch((value: unknown) => value) + expect(error).toBeInstanceOf(RelayDirectorMoveNotNewerError) + expect(error).toMatchObject({ + cellUrl: relay.cellUrl, + assignmentEpoch: relay.assignmentEpoch, + currentCellUrl: relay.cellUrl, + currentAssignmentEpoch: relay.assignmentEpoch + }) }) }) diff --git a/mobile/src/transport/mobile-relay-invite-director.ts b/mobile/src/transport/mobile-relay-invite-director.ts index 5dc83f526b6..408f83c8c09 100644 --- a/mobile/src/transport/mobile-relay-invite-director.ts +++ b/mobile/src/transport/mobile-relay-invite-director.ts @@ -1,6 +1,27 @@ import type { PairingRelay } from '../../../src/shared/mobile-relay-pairing-offer' import { RelayMovedSchema } from '../../../src/shared/mobile-relay-phone-protocol' +export class RelayDirectorMoveNotNewerError extends Error { + readonly cellUrl: string + readonly assignmentEpoch: number + readonly currentCellUrl: string + readonly currentAssignmentEpoch: number + + constructor(args: { + cellUrl: string + assignmentEpoch: number + currentCellUrl: string + currentAssignmentEpoch: number + }) { + super('relay director move was not strictly newer') + this.name = 'RelayDirectorMoveNotNewerError' + this.cellUrl = args.cellUrl + this.assignmentEpoch = args.assignmentEpoch + this.currentCellUrl = args.currentCellUrl + this.currentAssignmentEpoch = args.currentAssignmentEpoch + } +} + export function resolvePairingInviteThroughDirector(args: { relay: PairingRelay timeoutMs?: number @@ -38,8 +59,19 @@ export function resolvePairingInviteThroughDirector(args: { return } const moved = RelayMovedSchema.safeParse(value) - if (!moved.success || moved.data.assignmentEpoch <= args.relay.assignmentEpoch) { - finish(new Error('relay director move was not strictly newer')) + if (!moved.success) { + finish(new Error('invalid relay director move')) + return + } + if (moved.data.assignmentEpoch <= args.relay.assignmentEpoch) { + finish( + new RelayDirectorMoveNotNewerError({ + cellUrl: moved.data.cellUrl, + assignmentEpoch: moved.data.assignmentEpoch, + currentCellUrl: args.relay.cellUrl, + currentAssignmentEpoch: args.relay.assignmentEpoch + }) + ) return } settled = true diff --git a/mobile/src/transport/pairing-relay-candidate.test.ts b/mobile/src/transport/pairing-relay-candidate.test.ts index 07d315a487d..8551716a44f 100644 --- a/mobile/src/transport/pairing-relay-candidate.test.ts +++ b/mobile/src/transport/pairing-relay-candidate.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import type { PairingCandidateClient } from './mobile-relay-physical-client' import { RelayOuterError } from './mobile-relay-physical-client' import { createRecoveringPairingRelayCandidate } from './pairing-relay-candidate' +import { RelayDirectorMoveNotNewerError } from './mobile-relay-invite-director' import type { MobileRelayPairingJournal } from './mobile-relay-pairing-journal' import type { ConnectionLogEntry } from './types' @@ -89,6 +90,66 @@ describe('recovering pairing relay candidate', () => { expect(stale.close).toHaveBeenCalledOnce() }) + it('retries the authoritative cell when the director confirms the same assignment', async () => { + const stale = client(Promise.reject(new RelayOuterError(1006))) + const target = client(Promise.resolve(success())) + const persistMove = vi.fn() + let connects = 0 + const candidate = createRecoveringPairingRelayCandidate({ + journal, + connect: (relay) => { + expect(relay.cellUrl).toBe(journal.metadata.relay.cellUrl) + expect(relay.assignmentEpoch).toBe(journal.metadata.relay.assignmentEpoch) + return connects++ === 0 ? stale : target + }, + resolveDirector: async (relay) => { + throw new RelayDirectorMoveNotNewerError({ + cellUrl: relay.cellUrl, + assignmentEpoch: relay.assignmentEpoch, + currentCellUrl: relay.cellUrl, + currentAssignmentEpoch: relay.assignmentEpoch + }) + }, + persistMove, + now: () => 1, + random: () => 0, + sleep: async () => {} + }) + + await expect(candidate.sendRequest('status.get')).resolves.toEqual(success()) + expect(connects).toBe(2) + expect(persistMove).not.toHaveBeenCalled() + }) + + it('does not retry a same-epoch move to a different cell', async () => { + const stale = client(Promise.reject(new RelayOuterError(1006))) + const resolveDirector = vi.fn(async (relay) => { + throw new RelayDirectorMoveNotNewerError({ + cellUrl: 'https://relay-c2.onorca.dev', + assignmentEpoch: relay.assignmentEpoch, + currentCellUrl: relay.cellUrl, + currentAssignmentEpoch: relay.assignmentEpoch + }) + }) + let connects = 0 + const candidate = createRecoveringPairingRelayCandidate({ + journal, + connect: () => { + connects++ + return stale + }, + resolveDirector, + persistMove: vi.fn(), + now: () => 1, + random: () => 0, + sleep: async () => {} + }) + + await expect(candidate.sendRequest('status.get')).rejects.toThrow(/not strictly newer/) + expect(resolveDirector).toHaveBeenCalledTimes(3) + expect(connects).toBe(1) + }) + it('does not ask the director to reinterpret endpoint-scoped host-offline', async () => { const offline = client(Promise.reject(new RelayOuterError(4404))) const resolveDirector = vi.fn() diff --git a/mobile/src/transport/pairing-relay-candidate.ts b/mobile/src/transport/pairing-relay-candidate.ts index 2293ce1855f..63281a072a0 100644 --- a/mobile/src/transport/pairing-relay-candidate.ts +++ b/mobile/src/transport/pairing-relay-candidate.ts @@ -1,6 +1,7 @@ import type { PairingRelay } from '../../../src/shared/mobile-relay-pairing-offer' import type { MobileRelayPairingJournal } from './mobile-relay-pairing-journal' import { RelayOuterError, type PairingCandidateClient } from './mobile-relay-physical-client' +import { RelayDirectorMoveNotNewerError } from './mobile-relay-invite-director' import { createPairingRelayLogger, pairingRelayErrorDetail } from './pairing-relay-log' import { redactSocketEndpoint } from './socket-event-debug' import type { ConnectionLogSink } from './types' @@ -88,6 +89,35 @@ export function createRecoveringPairingRelayCandidate(args: { return await client.sendRequest(method, params) } catch (error) { lastError = error + if (isCurrentAssignmentMove(error, relay)) { + log( + 'info', + 'Relay: director kept current assignment', + redactSocketEndpoint(relay.cellUrl) + ) + client.close() + await backOff(attempt) + if (closed) { + throw new Error('relay pairing client closed') + } + client = args.connect(relay, args.onLog) + try { + return await client.sendRequest(method, params) + } catch (retryError) { + lastError = retryError + log( + 'warn', + `Relay: recovery attempt ${attempt + 1} failed`, + pairingRelayErrorDetail(retryError) + ) + if (!isDirectorRecoverable(retryError) || attempt + 1 >= maxAttempts) { + log('error', 'Relay: recovery gave up', `after ${attempt + 1} attempt(s)`) + throw retryError + } + await backOff(attempt) + continue + } + } log('warn', `Relay: recovery attempt ${attempt + 1} failed`, pairingRelayErrorDetail(error)) if (!isDirectorRecoverable(error) || attempt + 1 >= maxAttempts) { log('error', 'Relay: recovery gave up', `after ${attempt + 1} attempt(s)`) @@ -100,6 +130,16 @@ export function createRecoveringPairingRelayCandidate(args: { } } +function isCurrentAssignmentMove(error: unknown, relay: PairingRelay): boolean { + return ( + error instanceof RelayDirectorMoveNotNewerError && + error.assignmentEpoch === relay.assignmentEpoch && + error.cellUrl === relay.cellUrl && + error.currentAssignmentEpoch === relay.assignmentEpoch && + error.currentCellUrl === relay.cellUrl + ) +} + function pairingRelayFromJournal(journal: MobileRelayPairingJournal): PairingRelay { return { ...journal.metadata.relay,