diff --git a/README.md b/README.md index 052ecb4..c3a5e9d 100644 --- a/README.md +++ b/README.md @@ -84,12 +84,13 @@ try { #### Recovery and lifetime -| Condition | What the IDB backend does | -| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| The private companion exits after it started (crash, external kill, idle shutdown, simulator shutdown) | The next call starts a fresh private companion, then runs the command. | -| A command fails with a lost companion (`Failed to connect to companion`) or an invalid simulator HID connection (`dtuhidd ... invalidated / interrupted / did not answer`) | The backend replaces its private companion and sends the command **once** more. The companion discards HID events sent on an invalid connection, and an unreachable companion received nothing, so the resend cannot repeat a tap. | -| The channel fails after the request may have been sent (`StatusCode.UNAVAILABLE`), or idb prints `retry: unsafe` | Read-only commands are resent once. Input commands (`tap`, `swipe`, `text`, `key`, ...) are not resent; the error says the action may have run. | -| The retry also fails | The call rejects with `IdbUnavailableError` (exported). `err.code` is `IDB_HID_UNAVAILABLE` (HID connection still invalid; usually the simulator rebooted during the session) or `IDB_COMPANION_UNAVAILABLE` (companion cannot be reached or started). The message ends with a remediation. | +| Condition | What the IDB backend does | +| ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| The private companion exits after it started (crash, external kill, idle shutdown, simulator shutdown) | The next call starts a fresh private companion, then runs the command. | +| A command fails with a lost companion (`Failed to connect to companion`) or an invalid simulator HID connection (`dtuhidd connection ... has been invalidated`, or `... would be discarded`, which also covers a failed dtuhidd liveness check) | The backend replaces its private companion and sends the command **once** more. The companion refuses the HID event that hits an invalid connection, and an unreachable companion received nothing. When the connection was already stale (the usual case), nothing landed and the resend is safe. If the simulator died partway through a multi-event command (`ui text`, `ui swipe`), a leading part of it may already have landed. | +| The channel fails after the request may have been sent (`StatusCode.UNAVAILABLE`) | Read-only commands (`describe`, `list-apps`, `screenshot`, `log`, `ui describe-all`, `ui describe-point`, `ui describe`, `ui wait`) are resent once. All other commands (`ui tap`, `ui swipe`, `ui text`, `ui key`, `launch`, `terminate`, ...) are not resent; the error says the action may have run. | +| The retry also fails | The call rejects with `IdbUnavailableError` (exported). `err.code` is `IDB_HID_UNAVAILABLE` (HID connection still invalid; usually the simulator rebooted during the session) or `IDB_COMPANION_UNAVAILABLE` (companion cannot be reached or started). The message ends with a remediation. | +| The backend is disposed while a command is running, and that command fails | The command is not resent. The call rejects with `IDB command interrupted: the backend was disposed during the call`; `err.cause` is the original idb error. | `backend.reconnect?.()` replaces the backend's own connection on demand (iOS: its private companion). It is optional on `DeviceBackend`; the lazy backend forwards it. It never reboots or shuts down a device. diff --git a/src/backends/idb-backend.test.ts b/src/backends/idb-backend.test.ts index e5d608d..c860d00 100644 --- a/src/backends/idb-backend.test.ts +++ b/src/backends/idb-backend.test.ts @@ -1302,16 +1302,29 @@ describe('IdbBackend companion recovery', () => { expect(connectionMocks.connect).toHaveBeenCalledTimes(2); }); - it('honours an explicit unsafe retry verdict', async () => { - mockExecStrict.mockRejectedValue( - new Error(`${hidInvalidated}\nretry: unsafe (outcome_unknown)`), - ); + it.each([ + ['launch', async (): Promise => backend.openApp('io.metamask')], + ['terminate', async (): Promise => backend.closeApp('io.metamask')], + ])( + 'does not resend idb %s when delivery is uncertain', + async (command, run) => { + mockExecStrict.mockImplementation(async (cmd) => { + if (cmd === 'xcrun') { + return ''; + } + throw new Error('stderr: StatusCode.UNAVAILABLE: Socket closed'); + }); - await expect(backend.tapCoordinates(1, 1)).rejects.toMatchObject({ - code: 'IDB_HID_UNAVAILABLE', - }); - expect(tapCalls()).toHaveLength(1); - }); + await run(); + + const idbCalls = mockExecStrict.mock.calls.filter( + ([cmd, args]) => cmd !== 'xcrun' && args?.[0] === command, + ); + expect(idbCalls).toHaveLength(1); + // The companion was still replaced for the next command. + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + }, + ); it('resends a read-only command when delivery is uncertain', async () => { mockExecStrict @@ -1323,6 +1336,145 @@ describe('IdbBackend companion recovery', () => { expect(connectionMocks.connect).toHaveBeenCalledTimes(2); }); + it.each([ + [ + 'describe', + async (): Promise => backend.getDeviceInfo(), + '{"name":"iPhone"}', + ], + [ + 'list-apps', + async (): Promise => backend.getAppState('io.metamask'), + '', + ], + [ + 'screenshot', + async (): Promise => + backend.screenshot('/tmp/shot.png', { encode: false }), + '', + ], + ['log', async (): Promise => backend.getLogs(5), ''], + ])( + 'resends read-only idb %s when delivery is uncertain', + async (command, run, output) => { + mockExecStrict + .mockRejectedValueOnce(new Error('StatusCode.UNAVAILABLE')) + .mockResolvedValueOnce(output); + + await run(); + + const idbCalls = mockExecStrict.mock.calls.filter( + ([, args]) => args?.[0] === command, + ); + expect(idbCalls).toHaveLength(2); + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + }, + ); + + it('reports disposal during a command as an interruption, without a resend', async () => { + await backend.ensureConnected(); + let fail!: (error: Error) => void; + mockExecStrict.mockReturnValueOnce( + new Promise((_resolve, reject) => { + fail = reject; + }), + ); + const tapping = backend.tapCoordinates(1, 2).catch((e: unknown) => e); + await vi.waitFor(() => expect(tapCalls()).toHaveLength(1)); + + await backend.dispose(); + const cause = new Error(companionGone); + fail(cause); + const error = await tapping; + + expect(error).not.toBeInstanceOf(IdbUnavailableError); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + 'IDB command interrupted: the backend was disposed during the call', + ); + expect((error as Error).message).not.toContain( + 'idb_companion is installed', + ); + expect((error as Error).cause).toBe(cause); + expect(tapCalls()).toHaveLength(1); + expect(connectionMocks.connect).toHaveBeenCalledTimes(1); + }); + + it('returns the first non-strict failure when disposed during a command', async () => { + await backend.ensureConnected(); + let finish!: (result: execModule.ExecResult) => void; + mockExec.mockImplementation(async (cmd, args) => + cmd !== 'xcrun' && args?.[1] === 'key-sequence' + ? new Promise((resolve) => { + finish = resolve; + }) + : { exitCode: 0, stdout: '', stderr: '' }, + ); + const typing = backend.typeText('abc').catch((e: unknown) => e); + await vi.waitFor(() => expect(finish).toBeDefined()); + + await backend.dispose(); + finish({ exitCode: 1, stdout: '', stderr: hidInvalidated }); + + // The failed key-sequence is returned, not resent; the next idb command + // then fails fast because the backend is disposed. + expect(await typing).toMatchObject({ + message: 'IDB backend has been disposed', + }); + expect( + mockExec.mock.calls.filter(([, args]) => args?.[1] === 'key-sequence'), + ).toHaveLength(1); + expect(connectionMocks.connect).toHaveBeenCalledTimes(1); + }); + + it('does not resend when disposed after joining a newer companion', async () => { + await backend.ensureConnected(); + let fail!: (error: Error) => void; + mockExecStrict.mockReturnValueOnce( + new Promise((_resolve, reject) => { + fail = reject; + }), + ); + const tapping = backend.tapCoordinates(1, 2).catch((e: unknown) => e); + await vi.waitFor(() => expect(tapCalls()).toHaveLength(1)); + // Another caller replaces the companion while the tap is in flight. + await backend.reconnect(); + // Dispose exactly when the failed tap checks the new companion, so + // ensureConnected() itself succeeds. + let disposing: Promise | undefined; + Object.defineProperty(connectionState, 'alive', { + configurable: true, + get: (): boolean => { + disposing ??= backend.dispose(); + return true; + }, + }); + try { + fail(new Error(hidInvalidated)); + expect(await tapping).toMatchObject({ + message: + 'IDB command interrupted: the backend was disposed during the call', + }); + await disposing; + } finally { + Object.defineProperty(connectionState, 'alive', { + configurable: true, + writable: true, + value: true, + }); + } + expect(disposing).toBeDefined(); + expect(tapCalls()).toHaveLength(1); + }); + + it('keeps the connect error when cleanup after a failed connect also fails', async () => { + connectionMocks.connect.mockRejectedValueOnce(new Error('startup failed')); + connectionMocks.dispose.mockRejectedValueOnce(new Error('stop failed')); + + await expect(backend.ensureConnected()).rejects.toThrow('startup failed'); + expect(connectionMocks.dispose).toHaveBeenCalledTimes(1); + }); + it('reports a failed reconnect as IDB_COMPANION_UNAVAILABLE', async () => { await backend.ensureConnected(); mockExecStrict.mockRejectedValue(new Error(hidInvalidated)); diff --git a/src/backends/idb-backend.ts b/src/backends/idb-backend.ts index 4f3f402..3cbb9e6 100644 --- a/src/backends/idb-backend.ts +++ b/src/backends/idb-backend.ts @@ -146,7 +146,8 @@ export class IdbBackend implements DeviceBackend { this.#generation += 1; this.#connected = true; } catch (error) { - await this.#connection.dispose(); + // Best-effort cleanup: a failed stop must not hide the connect error. + await this.#connection.dispose().catch(() => undefined); this.#connecting = undefined; throw error; } @@ -206,10 +207,16 @@ export class IdbBackend implements DeviceBackend { * Run one idb command through this backend's private companion and recover * once from a lost companion or an invalidated simulator HID connection: * classify the failure, start a fresh private companion, and send the - * command once more. HID events sent on an invalidated dtuhidd connection - * are discarded and an unreachable companion received nothing, so the - * retry cannot repeat an action. When delivery is uncertain - * (`StatusCode.UNAVAILABLE`), mutating `ui` commands are not resent. + * command once more. + * + * The companion refuses the HID event that hits an invalidated dtuhidd + * connection, so that event was not delivered, and an unreachable companion + * received nothing. When the HID connection was already stale (the observed + * failure), the first event fails and the resend is safe. If the simulator + * died partway through a multi-event command (`ui text`, `ui swipe`), a + * leading part of it may already have landed. When delivery is uncertain + * (`StatusCode.UNAVAILABLE`), only read-only commands are resent. A command + * is never resent after the backend is disposed. * * @param args - The idb arguments (used to decide whether a resend is safe). * @param run - Sends the command once. @@ -244,6 +251,9 @@ export class IdbBackend implements DeviceBackend { await this.ensureConnected(); } } catch (error) { + if (this.#disposed) { + return interrupted(first, strict); + } if (!strict) { return first.settle(); } @@ -254,6 +264,10 @@ export class IdbBackend implements DeviceBackend { { cause: error, mayHaveRun: !resend }, ); } + if (this.#disposed) { + // Disposal started while another call's reconnect was finishing. + return interrupted(first, strict); + } if (!resend) { if (!strict) { return first.settle(); @@ -977,6 +991,34 @@ async function attempt( } } +/** + * Settle a failed first attempt when the backend was disposed before the + * recovery could run. Disposal is a teardown, not a device fault, so the + * command is not resent and no device remediation is given. + * + * @param first - The failed first attempt. + * @param strict - Throw instead of returning the failed result. + * @returns The first result, for non-strict callers. + */ +function interrupted(first: Attempt, strict: boolean): Result { + if (!strict) { + return first.settle(); + } + throw new Error( + 'IDB command interrupted: the backend was disposed during the call', + { cause: first.error }, + ); +} + +/** Top-level idb commands that only read device state. */ +const READ_ONLY_COMMANDS = new Set([ + 'describe', + 'list-apps', + 'screenshot', + 'log', +]); + +/** `idb ui` subcommands that only read device state. */ const READ_ONLY_UI_COMMANDS = new Set([ 'describe-all', 'describe-point', @@ -986,12 +1028,18 @@ const READ_ONLY_UI_COMMANDS = new Set([ /** * Whether an idb command can change the device (a resend could repeat it). + * Only commands listed as read-only are safe; any other command, including + * one added later, counts as mutating. * * @param args - The idb arguments. - * @returns True for `ui` input commands (tap, swipe, text, key, ...). + * @returns False only for known read-only commands. */ function isMutatingIdbCommand(args: string[]): boolean { - return args[0] === 'ui' && !READ_ONLY_UI_COMMANDS.has(args[1] ?? ''); + const [command = '', subcommand = ''] = args; + if (command === 'ui') { + return !READ_ONLY_UI_COMMANDS.has(subcommand); + } + return !READ_ONLY_COMMANDS.has(command); } function collectAlertTexts(elements: UIElement[]): string[] { diff --git a/src/backends/idb-errors.test.ts b/src/backends/idb-errors.test.ts index f4dd518..1dd9a50 100644 --- a/src/backends/idb-errors.test.ts +++ b/src/backends/idb-errors.test.ts @@ -10,12 +10,7 @@ describe('classifyIdbFailure', () => { true, ], [ - 'dtuhidd connection (com.apple.coredevice.feature.remote.hid.digitizer) was interrupted', - 'IDB_HID_UNAVAILABLE', - true, - ], - [ - 'dtuhidd did not answer the liveness probe (attempt 3)', + 'dtuhidd did not answer a liveness probe in 5 attempts (timed out) — the daemon is not running and launchd is not keeping it up, so every HID event sent to it would be discarded without error', 'IDB_HID_UNAVAILABLE', true, ], @@ -30,20 +25,24 @@ describe('classifyIdbFailure', () => { false, ], [ - 'The dtuhidd connection (x) has been invalidated\nretry: unsafe (outcome_unknown)', + 'stderr: The dtuhidd connection (x) has been invalidated', 'IDB_HID_UNAVAILABLE', - false, + true, ], ])('classifies %s', (text, code, notDelivered) => { expect(classifyIdbFailure(text)).toStrictEqual({ code, notDelivered }); }); - it.each(['Element not found', 'No such app', ''])( - 'leaves other failures unclassified: %s', - (text) => { - expect(classifyIdbFailure(text)).toBeUndefined(); - }, - ); + it.each([ + 'Element not found', + 'No such app', + '', + // Companion log lines only; they never reach idb stderr. + 'dtuhidd connection (com.apple.coredevice.feature.remote.hid.digitizer) was interrupted', + 'dtuhidd did not answer the liveness probe (attempt 3 of 5): timed out', + ])('leaves other failures unclassified: %s', (text) => { + expect(classifyIdbFailure(text)).toBeUndefined(); + }); }); describe('IdbUnavailableError', () => { diff --git a/src/backends/idb-errors.ts b/src/backends/idb-errors.ts index e2131ef..23448da 100644 --- a/src/backends/idb-errors.ts +++ b/src/backends/idb-errors.ts @@ -4,16 +4,34 @@ export type IdbErrorCode = 'IDB_HID_UNAVAILABLE' | 'IDB_COMPANION_UNAVAILABLE'; export type IdbFailure = { code: IdbErrorCode; /** - * True when the failure proves the command was not delivered, so sending - * it again cannot repeat an action. HID events sent on an invalidated - * dtuhidd connection are discarded, and a companion that cannot be reached - * received nothing. + * True when the failing event or request was refused before delivery. + * + * - HID: the companion refuses the event that hits an invalidated dtuhidd + * connection, so that event was not delivered. An idb command can be a + * stream of events (`ui text` sends one down/up pair per character, + * `ui swipe` sends many touches). When the connection was already stale, + * the first event fails and nothing landed; this is the observed failure, + * and a resend through a fresh companion is safe. When the simulator dies + * partway through a command, a leading part of it (some characters or + * touches) may already have landed. + * - Companion not reached: the request was never sent. */ notDelivered: boolean; }; +// Thrown errors of the idb_companion HID transport (facebook/idb v1.7.1, +// FBSimulatorControl/HID/SimulatorHIDError.swift). Companion log lines (for +// example "was interrupted" or a single failed liveness probe) never reach +// idb stderr, so they are not matched. const HID_PATTERNS = [ - /dtuhidd .*(?:invalidated|interrupted|did not answer)/iu, + // dtuhidConnectionInvalidated, thrown by SimulatorDTUHIDConnection.write(): + // "The dtuhidd connection () has been invalidated; events sent on it + // would be discarded". + /dtuhidd connection .*has been invalidated/iu, + // The invalidated error above, and dtuhidUnresponsive, thrown by + // SimulatorDTUHIDConnection.connect() before any event is written: + // "dtuhidd did not answer a liveness probe in 5 attempts ... every HID event + // sent to it would be discarded without error". /events? sent (?:on|to) it would be discarded/iu, ]; @@ -21,8 +39,6 @@ const COMPANION_NOT_REACHED = /Failed to connect to companion/iu; const COMPANION_UNAVAILABLE = 'StatusCode.UNAVAILABLE'; -const UNSAFE_VERDICT = /^retry: unsafe\b/mu; - /** * Classify an idb failure from its stderr (or an execStrict error message). * @@ -30,13 +46,11 @@ const UNSAFE_VERDICT = /^retry: unsafe\b/mu; * @returns The failure kind, or undefined for any other error. */ export function classifyIdbFailure(text: string): IdbFailure | undefined { - // An explicit companion verdict (idb --retry-verdict) always wins. - const unsafe = UNSAFE_VERDICT.test(text); if (HID_PATTERNS.some((pattern) => pattern.test(text))) { - return { code: 'IDB_HID_UNAVAILABLE', notDelivered: !unsafe }; + return { code: 'IDB_HID_UNAVAILABLE', notDelivered: true }; } if (COMPANION_NOT_REACHED.test(text)) { - return { code: 'IDB_COMPANION_UNAVAILABLE', notDelivered: !unsafe }; + return { code: 'IDB_COMPANION_UNAVAILABLE', notDelivered: true }; } if (text.includes(COMPANION_UNAVAILABLE)) { // The channel failed, but possibly after the request was sent. diff --git a/src/backends/index.ts b/src/backends/index.ts index 26564b7..bdeb6d5 100644 --- a/src/backends/index.ts +++ b/src/backends/index.ts @@ -82,7 +82,8 @@ export async function createBackend( try { await backend.ensureConnected(); } catch (error) { - await backend.dispose?.(); + // Best-effort cleanup: a failed dispose must not hide the connect error. + await backend.dispose?.().catch(() => undefined); throw error; } return backend; diff --git a/src/backends/lazy-backend.test.ts b/src/backends/lazy-backend.test.ts index 61e0201..f5cc582 100644 --- a/src/backends/lazy-backend.test.ts +++ b/src/backends/lazy-backend.test.ts @@ -500,6 +500,23 @@ describe('createLazyBackend', () => { ); }); + it('keeps the connect error when cleanup of the failed backend also fails', async () => { + const { createBackend } = await import('./index.js'); + mockDetectPlatform.mockResolvedValue({ + platform: 'ios', + deviceId: 'sim-a', + }); + mockEnsureConnected.mockRejectedValueOnce( + new Error('Private IDB companion exited before readiness (1)'), + ); + mockDispose.mockRejectedValueOnce(new Error('stop failed')); + + await expect(createBackend()).rejects.toThrow( + 'Private IDB companion exited before readiness (1)', + ); + expect(mockDispose).toHaveBeenCalledTimes(1); + }); + it('allows retry after non-multi-device error', async () => { const createLazyBackend = await getLazyBackend(); mockDetectPlatform