diff --git a/src/daemon/handlers/__tests__/session-appstate-input-perf.test.ts b/src/daemon/handlers/__tests__/session-appstate-input-perf.test.ts index 767183b9c..fce873874 100644 --- a/src/daemon/handlers/__tests__/session-appstate-input-perf.test.ts +++ b/src/daemon/handlers/__tests__/session-appstate-input-perf.test.ts @@ -443,7 +443,7 @@ test('perf preserves successful metrics and normalizes per-metric Android sampli expect(memory?.available).toBe(false); expect(memory?.reason).toBe('error: device offline'); expect(memory?.error?.code).toBe('COMMAND_FAILED'); - expect(memory?.error?.hint).toMatch(/retry with --debug/i); + expect(memory?.error?.hint).toMatch(/adb reconnect/i); expect(memory?.error?.details?.metric).toBe('memory'); expect(memory?.error?.details?.package).toBe('com.example.app'); expect(cpu?.available).toBe(true); diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index ef74ed195..da7e950d3 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -313,7 +313,9 @@ function enrichDaemonError(command: string, error: DaemonError): DaemonError { ? supportedPlatformsForCommand(command) : []; const supportedOn = supportedPlatforms.length > 0 ? supportedPlatforms.join(', ') : undefined; - const retriable = retriableForErrorCode(error.code); + // A throw-site classification (lifted from details by normalizeError) wins + // over the conservative code-level policy. + const retriable = error.retriable ?? retriableForErrorCode(error.code); if (supportedOn === undefined && retriable === undefined) return error; return { ...error, diff --git a/src/kernel/errors.ts b/src/kernel/errors.ts index 6682b5e27..c63841e3a 100644 --- a/src/kernel/errors.ts +++ b/src/kernel/errors.ts @@ -34,6 +34,7 @@ type AppErrorDetails = Record & { hint?: string; diagnosticId?: string; logPath?: string; + retriable?: boolean; }; export type NormalizedError = { @@ -42,6 +43,12 @@ export type NormalizedError = { hint?: string; diagnosticId?: string; logPath?: string; + /** + * Lifted from `details.retriable` when a throw site classified the failure as + * clearly transient (or clearly not). Included only when set, so the default + * error wire shape is unchanged. + */ + retriable?: boolean; details?: Record; }; @@ -91,6 +98,8 @@ export function normalizeError( (details && typeof details.logPath === 'string' ? details.logPath : undefined) ?? context.logPath; const hint = detailHint ?? defaultHintForCode(appErr.code); + const retriable = + details && typeof details.retriable === 'boolean' ? details.retriable : undefined; const cleanDetails = stripDiagnosticMeta(details); const message = maybeEnrichCommandFailedMessage(appErr.code, appErr.message, details); @@ -100,6 +109,7 @@ export function normalizeError( hint, diagnosticId, logPath, + ...(retriable !== undefined ? { retriable } : {}), details: cleanDetails, }; } @@ -148,6 +158,7 @@ function stripDiagnosticMeta( delete output.hint; delete output.diagnosticId; delete output.logPath; + delete output.retriable; return Object.keys(output).length > 0 ? output : undefined; } diff --git a/src/platforms/android/__tests__/adb-executor.test.ts b/src/platforms/android/__tests__/adb-executor.test.ts index 6702260e0..108a9bf0c 100644 --- a/src/platforms/android/__tests__/adb-executor.test.ts +++ b/src/platforms/android/__tests__/adb-executor.test.ts @@ -18,6 +18,9 @@ vi.mock('../../../utils/exec.ts', async (importOriginal) => { }); import { + androidAdbResultError, + attachAdbFailureHint, + classifyAdbFailure, createAndroidPortReverseManager, createDeviceAdbExecutor, createLocalAndroidAdbProvider, @@ -28,6 +31,7 @@ import { withAndroidAdbProvider, } from '../adb-executor.ts'; import { runCmd, runCmdBackground } from '../../../utils/exec.ts'; +import { AppError, normalizeError } from '../../../kernel/errors.ts'; const mockRunCmd = vi.mocked(runCmd); const mockRunCmdBackground = vi.mocked(runCmdBackground); @@ -305,3 +309,296 @@ test('explicit transfer helpers keep exec-shaped fallback for older providers', ['install', '-r', '/app.apk'], ]); }); + +test('classifyAdbFailure recognizes the common adb failure families', () => { + const cases: Array<[stderr: string, reason: string, retriable: boolean | undefined]> = [ + ['adb: device offline', 'device_offline', true], + [ + "error: device unauthorized.\nThis adb server's $ADB_VENDOR_KEYS is not set", + 'device_unauthorized', + undefined, + ], + ['adb: more than one device/emulator', 'multiple_devices', undefined], + ['error: more than one device and emulator', 'multiple_devices', undefined], + ['error: no devices/emulators found', 'no_devices', undefined], + ["error: device 'emulator-5554' not found", 'device_not_found', true], + ['error: device not found', 'device_not_found', true], + [ + "adb server version (40) doesn't match this client (41); killing...", + 'server_version_mismatch', + true, + ], + [ + "error: protocol fault (couldn't read status): Connection reset by peer", + 'connection_dropped', + true, + ], + ['error: transport error', 'connection_dropped', true], + [ + 'adb: failed to install app.apk: Failure [INSTALL_FAILED_INSUFFICIENT_STORAGE]', + 'install_insufficient_storage', + undefined, + ], + [ + 'adb: failed to install app.apk: Failure [INSTALL_FAILED_UPDATE_INCOMPATIBLE: signatures do not match]', + 'install_update_incompatible', + undefined, + ], + [ + 'adb: failed to install app.apk: Failure [INSTALL_FAILED_VERSION_DOWNGRADE]', + 'install_version_downgrade', + undefined, + ], + [ + 'adb: failed to install app.apk: Failure [INSTALL_FAILED_NO_MATCHING_ABIS]', + 'install_failed', + undefined, + ], + ]; + for (const [stderr, reason, retriable] of cases) { + const classification = classifyAdbFailure(stderr); + assert.equal(classification?.reason, reason, `reason for: ${stderr}`); + assert.equal(classification?.retriable, retriable, `retriable for: ${stderr}`); + assert.ok((classification?.hint ?? '').length > 0, `hint for: ${stderr}`); + } +}); + +test('classifyAdbFailure matches install verdicts on stdout but transport families only on stderr', () => { + const installFromStdout = classifyAdbFailure('', 'Failure [INSTALL_FAILED_UPDATE_INCOMPATIBLE]'); + assert.equal(installFromStdout?.reason, 'install_update_incompatible'); + // Arbitrary `adb shell` stdout (e.g. cat-ing a log) must not read as a transport failure. + assert.equal(classifyAdbFailure('', 'log line: device offline detected'), undefined); + assert.equal(classifyAdbFailure('unrelated failure output'), undefined); +}); + +test('the local adb executor attaches classified hints to thrown command failures', async () => { + mockRunCmd.mockClear(); + mockRunCmd.mockRejectedValueOnce( + new AppError('COMMAND_FAILED', 'adb exited with code 1', { + cmd: 'adb', + exitCode: 1, + stdout: '', + stderr: "error: device unauthorized.\nThis adb server's $ADB_VENDOR_KEYS is not set", + processExitError: true, + }), + ); + const adb = createDeviceAdbExecutor({ + platform: 'android', + id: 'emulator-5554', + name: 'Pixel Emulator', + kind: 'emulator', + booted: true, + }); + + const error = await adb(['shell', 'echo', 'hi']).then( + () => assert.fail('expected the adb call to reject'), + (err: unknown) => err, + ); + + assert.ok(error instanceof AppError); + assert.equal(error.details?.adbFailure, 'device_unauthorized'); + assert.match(String(error.details?.hint), /authorization prompt/i); + assert.equal(Object.hasOwn(error.details ?? {}, 'retriable'), false); +}); + +test('the local adb executor flags transient transport failures retriable', async () => { + mockRunCmd.mockClear(); + mockRunCmd.mockRejectedValueOnce( + new AppError('COMMAND_FAILED', 'adb exited with code 1', { + exitCode: 1, + stdout: '', + stderr: 'adb: device offline', + processExitError: true, + }), + ); + const adb = createDeviceAdbExecutor({ + platform: 'android', + id: 'emulator-5554', + name: 'Pixel Emulator', + kind: 'emulator', + booted: true, + }); + + const error = await adb(['shell', 'echo', 'hi']).then( + () => assert.fail('expected the adb call to reject'), + (err: unknown) => err, + ); + + assert.ok(error instanceof AppError); + assert.equal(error.details?.adbFailure, 'device_offline'); + assert.equal(error.details?.retriable, true); +}); + +test('attachAdbFailureHint preserves existing hints and ignores non-adb errors', () => { + const withHint = new AppError('COMMAND_FAILED', 'adb exited with code 1', { + stderr: 'adb: device offline', + hint: 'site-specific hint', + }); + attachAdbFailureHint(withHint); + assert.equal(withHint.details?.hint, 'site-specific hint'); + assert.equal(withHint.details?.adbFailure, 'device_offline'); + + const otherCode = new AppError('TOOL_MISSING', 'adb not found in PATH', { + stderr: 'adb: device offline', + }); + attachAdbFailureHint(otherCode); + assert.equal(Object.hasOwn(otherCode.details ?? {}, 'hint'), false); + + const plain = new Error('boom'); + assert.equal(attachAdbFailureHint(plain), plain); +}); + +test('port reverse removal failures surface as classified AppErrors, not bare Errors', async () => { + const provider = createAndroidPortReverseManager(async (args) => { + if (args[0] === 'reverse' && args[1] === '--remove') { + return { stdout: '', stderr: 'error: device offline', exitCode: 1 }; + } + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const error = await provider.remove('tcp:8081').then( + () => assert.fail('expected the removal to reject'), + (err: unknown) => err, + ); + + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.adbFailure, 'device_offline'); + assert.equal(error.details?.retriable, true); + assert.match(String(error.details?.hint), /reconnect/i); +}); + +test('androidAdbResultError classifies tolerated nonzero results like thrown executor failures', () => { + const error = androidAdbResultError( + 'adb uninstall failed for com.example.app', + { exitCode: 1, stdout: '', stderr: 'error: device offline' }, + { package: 'com.example.app' }, + ); + + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.stderr, 'error: device offline'); + assert.equal(error.details?.exitCode, 1); + assert.equal(error.details?.package, 'com.example.app'); + assert.equal(error.details?.adbFailure, 'device_offline'); + assert.equal(error.details?.retriable, true); + assert.match(String(error.details?.hint), /adb reconnect/i); +}); + +test('androidAdbResultError composes the classified hint with the stderr excerpt enrichment', () => { + const error = androidAdbResultError('adb uninstall failed for com.example.app', { + exitCode: 1, + stdout: '', + stderr: 'error: device offline', + }); + + // execFailureDetails flags processExitError, so normalizeError suffixes the + // curated message with the stderr excerpt while the classified hint rides along. + assert.equal(error.details?.processExitError, true); + const normalized = normalizeError(error); + assert.equal( + normalized.message, + 'adb uninstall failed for com.example.app: error: device offline', + ); + assert.match(String(normalized.hint), /adb reconnect/i); + assert.equal(normalized.retriable, true); +}); + +test('androidAdbResultError leaves semantic exit-0 failures without excerpt enrichment', () => { + const error = androidAdbResultError('Failed to launch com.example.app', { + exitCode: 0, + stdout: 'Error: Activity not started', + stderr: 'Warning: unrelated deprecation notice', + }); + + assert.equal(Object.hasOwn(error.details ?? {}, 'processExitError'), false); + assert.equal(normalizeError(error).message, 'Failed to launch com.example.app'); +}); + +test('androidAdbResultError keeps a site hint over the classified one', () => { + const error = androidAdbResultError( + 'Failed to pull Android heap dump', + { exitCode: 1, stdout: '', stderr: 'error: device offline' }, + { hint: 'site-specific hint' }, + ); + + assert.equal(error.details?.hint, 'site-specific hint'); + assert.equal(error.details?.adbFailure, 'device_offline'); +}); + +test('semantic provider install failures carry classified hints', async () => { + const error = await withAndroidAdbProvider( + { + exec: async () => ({ stdout: '', stderr: '', exitCode: 0 }), + install: async () => { + throw new AppError('COMMAND_FAILED', 'remote install failed', { + exitCode: 1, + stdout: 'Failure [INSTALL_FAILED_UPDATE_INCOMPATIBLE: signatures do not match]', + stderr: '', + }); + }, + }, + { serial: 'emulator-5554' }, + async () => + await installAndroidAdbPackage('/app.apk', { replace: true }).then( + () => assert.fail('expected the provider install to reject'), + (err: unknown) => err, + ), + ); + + assert.ok(error instanceof AppError); + assert.equal(error.details?.adbFailure, 'install_update_incompatible'); + assert.match(String(error.details?.hint), /incompatible signature/i); +}); + +test('explicitly passed provider pull failures carry classified hints', async () => { + const error = await pullAndroidAdbFile('/remote.mp4', '/local.mp4', { + provider: { + exec: async () => ({ stdout: '', stderr: '', exitCode: 0 }), + pull: async () => { + throw new AppError('COMMAND_FAILED', 'remote pull failed', { + exitCode: 1, + stdout: '', + stderr: 'error: device offline', + }); + }, + }, + }).then( + () => assert.fail('expected the provider pull to reject'), + (err: unknown) => err, + ); + + assert.ok(error instanceof AppError); + assert.equal(error.details?.adbFailure, 'device_offline'); + assert.equal(error.details?.retriable, true); + assert.match(String(error.details?.hint), /adb reconnect/i); +}); + +test('provider-scoped adb failures get the same classified hints as local execution', async () => { + const device = { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel Emulator', + kind: 'emulator', + booted: true, + } as const; + + const error = await withAndroidAdbProvider( + async () => { + throw new AppError('COMMAND_FAILED', 'remote adb exited with code 1', { + exitCode: 1, + stdout: '', + stderr: 'adb: more than one device/emulator', + }); + }, + { serial: 'emulator-5554' }, + async () => + await resolveAndroidAdbExecutor(device)(['shell', 'echo', 'hi']).then( + () => assert.fail('expected the provider-scoped call to reject'), + (err: unknown) => err, + ), + ); + + assert.ok(error instanceof AppError); + assert.equal(error.details?.adbFailure, 'multiple_devices'); + assert.match(String(error.details?.hint), /--serial/); +}); diff --git a/src/platforms/android/__tests__/device-input-state.test.ts b/src/platforms/android/__tests__/device-input-state.test.ts index 8853345f7..dab7d3f00 100644 --- a/src/platforms/android/__tests__/device-input-state.test.ts +++ b/src/platforms/android/__tests__/device-input-state.test.ts @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; import { getAndroidKeyboardStatusWithAdb } from '../device-input-state.ts'; import type { AndroidAdbExecutor } from '../adb-executor.ts'; +import { AppError } from '../../../kernel/errors.ts'; test('getAndroidKeyboardStatusWithAdb exposes active input method package', async () => { const adb: AndroidAdbExecutor = async (args) => { @@ -27,3 +28,24 @@ test('getAndroidKeyboardStatusWithAdb exposes active input method package', asyn }); }); }); + +test('getAndroidKeyboardStatusWithAdb classifies tolerated adb failures with actionable hints', async () => { + // allowFailure regression: the executor returns the nonzero result instead of + // throwing, so the classified hint must come from the result-to-error path. + const adb: AndroidAdbExecutor = async () => ({ + stdout: '', + stderr: 'error: device offline', + exitCode: 1, + }); + + const error = await getAndroidKeyboardStatusWithAdb(adb).then( + () => assert.fail('expected the keyboard query to reject'), + (err: unknown) => err, + ); + + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.adbFailure, 'device_offline'); + assert.equal(error.details?.retriable, true); + assert.match(String(error.details?.hint), /adb reconnect/i); +}); diff --git a/src/platforms/android/adb-executor.ts b/src/platforms/android/adb-executor.ts index d94eed60f..fc6985d06 100644 --- a/src/platforms/android/adb-executor.ts +++ b/src/platforms/android/adb-executor.ts @@ -2,6 +2,7 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import type { Readable, Writable } from 'node:stream'; import type { DeviceInfo } from '../../kernel/device.ts'; import { + execFailureDetails, runCmd, runCmdBackground, withCommandExecutorOverride, @@ -185,24 +186,233 @@ type AndroidAdbProviderScope = { const androidAdbProviderScope = new AsyncLocalStorage(); +export type AdbFailureClassification = { + /** Machine-readable failure family, attached to error details as `adbFailure`. */ + reason: + | 'device_offline' + | 'device_unauthorized' + | 'device_not_found' + | 'multiple_devices' + | 'no_devices' + | 'connection_dropped' + | 'server_version_mismatch' + | 'install_insufficient_storage' + | 'install_update_incompatible' + | 'install_version_downgrade' + | 'install_failed'; + hint: string; + /** Set only for clearly transient families where an unchanged retry can succeed. */ + retriable?: boolean; +}; + +type AdbFailureMatcher = AdbFailureClassification & { + /** Tested against lowercased output. */ + pattern: RegExp; + /** + * Install verdicts often land on stdout (`Failure [INSTALL_FAILED_*]` from pm), + * so those matchers also scan stdout. Everything else is stderr-only to avoid + * misreading arbitrary `adb shell` output as a transport failure. + */ + matchStdout?: boolean; +}; + +// Ordered most-specific first; the first match wins. +const ADB_FAILURE_MATCHERS: readonly AdbFailureMatcher[] = [ + { + reason: 'device_unauthorized', + pattern: /device unauthorized|device still authorizing/, + hint: 'USB debugging is not authorized — accept the authorization prompt on the device screen (re-plug the cable if none appears), then retry.', + }, + { + reason: 'device_offline', + pattern: /device offline/, + hint: 'The device is connected but offline — wait for it to finish booting or run adb reconnect, then retry.', + retriable: true, + }, + { + reason: 'multiple_devices', + pattern: /more than one (?:device\/emulator|device and emulator)/, + hint: 'Multiple Android devices are connected — pass --serial (see adb devices) to select one.', + }, + { + reason: 'no_devices', + pattern: /no devices\/emulators found|no devices found/, + hint: 'No Android devices detected — boot an emulator or connect a device and verify it appears in adb devices.', + }, + { + reason: 'device_not_found', + pattern: /device (?:'[^']*' )?not found/, + hint: 'The device disconnected or is restarting — verify it is listed in adb devices, then retry.', + retriable: true, + }, + { + reason: 'server_version_mismatch', + pattern: /adb server version \(\d+\) doesn't match this client/, + hint: 'Multiple adb installs conflict — adb restarts its server automatically, so retry; align PATH to a single adb to stop recurrences.', + retriable: true, + }, + { + reason: 'connection_dropped', + pattern: /transport error|connection reset|broken pipe|protocol fault/, + hint: 'The adb connection dropped — retry; if it persists, run adb kill-server and reconnect the device.', + retriable: true, + }, + { + reason: 'install_insufficient_storage', + pattern: /install_failed_insufficient_storage/, + hint: 'The device is out of storage — free up space or uninstall unused apps, then retry the install.', + matchStdout: true, + }, + { + reason: 'install_update_incompatible', + pattern: /install_failed_update_incompatible/, + hint: 'The installed app has an incompatible signature — uninstall the existing app first, then retry the install.', + matchStdout: true, + }, + { + reason: 'install_version_downgrade', + pattern: /install_failed_version_downgrade/, + hint: 'The APK is older than the installed app — uninstall the app first (or install with downgrade allowed), then retry.', + matchStdout: true, + }, + { + reason: 'install_failed', + pattern: /install_failed_\w+|install_parse_failed_\w+/, + hint: 'The Android package installer rejected the APK — see the INSTALL_FAILED code in the error output for the exact cause.', + matchStdout: true, + }, +]; + +/** + * Maps well-known adb failure output to an actionable hint (and a `retriable` + * flag for clearly transient families). Matches stderr; install verdicts also + * match stdout. Returns undefined for unrecognized output. + */ +export function classifyAdbFailure( + stderr: string, + stdout = '', +): AdbFailureClassification | undefined { + const stderrText = stderr.toLowerCase(); + const stdoutText = stdout.toLowerCase(); + for (const { pattern, matchStdout, ...classification } of ADB_FAILURE_MATCHERS) { + if (pattern.test(stderrText) || (matchStdout && pattern.test(stdoutText))) { + return classification; + } + } + return undefined; +} + +/** + * Enriches a failed adb command error in place with the classified hint, + * `retriable` flag, and machine-readable `adbFailure` family, so every adb call + * site surfaces guidance without per-site classification. No-op for errors that + * are not adb command failures or that carry no recognized failure output; an + * existing hint or retriable verdict is never overwritten. + */ +export function attachAdbFailureHint(error: T): T { + if (!(error instanceof AppError) || error.code !== 'COMMAND_FAILED') return error; + const stderr = typeof error.details?.stderr === 'string' ? error.details.stderr : ''; + const stdout = typeof error.details?.stdout === 'string' ? error.details.stdout : ''; + const classification = classifyAdbFailure(stderr, stdout); + if (!classification) return error; + error.details = { + ...error.details, + adbFailure: classification.reason, + ...(typeof error.details?.hint === 'string' ? {} : { hint: classification.hint }), + ...(classification.retriable !== undefined && error.details?.retriable === undefined + ? { retriable: classification.retriable } + : {}), + }; + return error; +} + +/** + * Builds the COMMAND_FAILED AppError for a failed `allowFailure` adb result and + * runs it through the failure classifier. This is the shared construction point + * for call sites that tolerate a failure to inspect it and then throw — those + * errors never cross the executor throw path, so they classify here instead. + * Site-provided `details` win on key collisions, and a site `hint` is preserved + * over the classified one. + * + * Nonzero exits build their details via execFailureDetails, whose + * processExitError flag makes normalizeError append the first stderr line to + * the curated message — the classified hint and the stderr-excerpt enrichment + * compose instead of competing. Semantic failures thrown at exit 0 (e.g. an + * `am start` error printed on a successful exit) stay unflagged so a stray + * stderr line never decorates a message the process exit does not back up. + */ +export function androidAdbResultError( + message: string, + result: Pick, + details?: Record, +): AppError { + const failureDetails = + result.exitCode === 0 + ? { stdout: result.stdout, stderr: result.stderr, exitCode: result.exitCode, ...details } + : execFailureDetails(result, details); + return attachAdbFailureHint(new AppError('COMMAND_FAILED', message, failureDetails)); +} + +function withAdbFailureHints( + call: (...args: Args) => Promise, +): (...args: Args) => Promise { + return async (...args) => { + try { + return await call(...args); + } catch (error) { + throw attachAdbFailureHint(error); + } + }; +} + +// Providers already enriched by withAdbFailureHintProvider, so repeated +// normalization resolves to the same object instead of stacking wrappers. +const adbFailureHintProviders = new WeakSet(); + +/** + * Wraps every promise-returning adb funnel a provider exposes — `exec` plus the + * semantic `pull`/`install`/`installBundle` methods that bypass it — so a + * provider failure (e.g. an INSTALL_FAILED verdict from `provider.install`) + * carries the same classified hint as local execution. Applied once inside + * {@link normalizeAndroidAdbProvider}, the single funnel every provider passes + * through; the local provider needs no wrap because its methods delegate to the + * already-enriched serial executor. + */ +function withAdbFailureHintProvider(provider: AndroidAdbProvider): AndroidAdbProvider { + if (adbFailureHintProviders.has(provider)) return provider; + const enriched: AndroidAdbProvider = { + ...provider, + exec: withAdbFailureHints(provider.exec), + ...(provider.pull ? { pull: withAdbFailureHints(provider.pull) } : {}), + ...(provider.install ? { install: withAdbFailureHints(provider.install) } : {}), + ...(provider.installBundle + ? { installBundle: withAdbFailureHints(provider.installBundle) } + : {}), + }; + adbFailureHintProviders.add(enriched); + return enriched; +} + export function createDeviceAdbExecutor(device: DeviceInfo): AndroidAdbExecutor { return createSerialAdbExecutor(device.id); } function createSerialAdbExecutor(serial: string): AndroidAdbExecutor { - return async (args, options) => - // Local adb execution must escape any active provider scope to avoid routing - // tunnel-backed providers back into themselves when they shell out to adb. - await withoutCommandExecutorOverride( - async () => - await runCmd('adb', ['-s', serial, ...args], { - ...options, - // Some `adb shell` children can survive killing the adb parent and keep - // requests open past timeout. Give each adb call its own process group - // so timeout/abort cleanup can tear down the whole local command tree. - detached: process.platform !== 'win32', - }), - ); + return withAdbFailureHints( + async (args, options) => + // Local adb execution must escape any active provider scope to avoid routing + // tunnel-backed providers back into themselves when they shell out to adb. + await withoutCommandExecutorOverride( + async () => + await runCmd('adb', ['-s', serial, ...args], { + ...options, + // Some `adb shell` children can survive killing the adb parent and keep + // requests open past timeout. Give each adb call its own process group + // so timeout/abort cleanup can tear down the whole local command tree. + detached: process.platform !== 'win32', + }), + ), + ); } function createSerialAdbSpawner(serial: string): AndroidAdbSpawner { @@ -315,10 +525,7 @@ export function createAndroidPortReverseManager( function normalizeAndroidAdbProvider( provider: AndroidAdbProvider | AndroidAdbExecutor, ): AndroidAdbProvider { - if (typeof provider === 'function') { - return { exec: provider }; - } - return provider; + return withAdbFailureHintProvider(typeof provider === 'function' ? { exec: provider } : provider); } type AndroidAdbTransferProviderOptions = { @@ -385,8 +592,11 @@ export async function withAndroidAdbProvider( fn: () => Promise, ): Promise { if (!provider) return await fn(); - const normalized = typeof provider === 'function' ? { exec: provider } : provider; - const scope = { provider: normalized, serial: options.serial }; + // Normalization wraps once at scope installation, so every consumer — the + // command-executor override and direct resolveAndroidAdb* lookups — gets + // classified failure hints on exec and the semantic provider methods alike. + const enriched = normalizeAndroidAdbProvider(provider); + const scope = { provider: enriched, serial: options.serial }; const override = createAndroidCommandExecutorOverride(scope); return await androidAdbProviderScope.run( scope, @@ -439,7 +649,14 @@ function createExecAndroidPortReverseProvider(adb: AndroidAdbExecutor): AndroidP timeoutMs: options?.timeoutMs, }); if (result.exitCode !== 0 && !isMissingReverseMapping(result.stdout, result.stderr)) { - throw new Error(`Failed to remove Android port reverse ${local}: ${result.stderr}`); + throw attachAdbFailureHint( + new AppError('COMMAND_FAILED', `Failed to remove Android port reverse ${local}`, { + local, + stdout: result.stdout, + stderr: result.stderr, + exitCode: result.exitCode, + }), + ); } for (const locals of owned.values()) { locals.delete(local); diff --git a/src/platforms/android/app-helpers.ts b/src/platforms/android/app-helpers.ts index cb34ab5e4..49fc3ddda 100644 --- a/src/platforms/android/app-helpers.ts +++ b/src/platforms/android/app-helpers.ts @@ -1,7 +1,5 @@ -import { AppError } from '../../kernel/errors.ts'; -import { execFailureDetails } from '../../utils/exec.ts'; import { resolveAppsFilter, type AppsFilter } from '../../contracts/app-inventory.ts'; -import type { AndroidAdbExecutor } from './adb-executor.ts'; +import { androidAdbResultError, type AndroidAdbExecutor } from './adb-executor.ts'; import { parseAndroidForegroundApp, parseAndroidLaunchablePackages, @@ -102,11 +100,7 @@ function parseAndroidLaunchablePackageOutput(stdout: string): string[] { async function listAndroidUserInstalledPackagesWithAdb(adb: AndroidAdbExecutor): Promise { const result = await adb(['shell', 'pm', 'list', 'packages', '-3'], { allowFailure: true }); if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to list Android user-installed apps', - execFailureDetails(result), - ); + throw androidAdbResultError('Failed to list Android user-installed apps', result); } return parseAndroidUserInstalledPackages(result.stdout); } diff --git a/src/platforms/android/app-lifecycle.ts b/src/platforms/android/app-lifecycle.ts index 9645d2adc..23406ff95 100644 --- a/src/platforms/android/app-lifecycle.ts +++ b/src/platforms/android/app-lifecycle.ts @@ -1,7 +1,7 @@ import { promises as fs } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { execFailureDetails, resolveFileOverridePath, runCmd, whichCmd } from '../../utils/exec.ts'; +import { resolveFileOverridePath, runCmd, whichCmd } from '../../utils/exec.ts'; import { AppError } from '../../kernel/errors.ts'; import { sleep } from '../../utils/timeouts.ts'; import type { AppsFilter } from '../../contracts/app-inventory.ts'; @@ -11,6 +11,7 @@ import { createAppResolutionCache, type AppResolutionCacheScope } from '../app-r import { waitForAndroidBoot } from './devices.ts'; import { runAndroidAdb } from './adb.ts'; import { + androidAdbResultError, createAndroidPortReverseManager, installAndroidAdbPackage, resolveAndroidAdbProvider, @@ -479,10 +480,7 @@ async function openAndroidPackage( if (!(await isAndroidPackageInstalled(device, packageName))) { throw buildAndroidPackageNotInstalledError(packageName); } - throw new AppError('COMMAND_FAILED', `Failed to launch ${packageName}`, { - stdout: primaryResult.stdout, - stderr: primaryResult.stderr, - }); + throw androidAdbResultError(`Failed to launch ${packageName}`, primaryResult); } await runAndroidAdb(device, buildAndroidActivityLaunchArgs(component, launchCategory, options)); } @@ -726,11 +724,7 @@ async function uninstallAndroidApp(device: DeviceInfo, app: string): Promise<{ p if (result.exitCode !== 0) { const output = `${result.stdout}\n${result.stderr}`.toLowerCase(); if (!output.includes('unknown package') && !output.includes('not installed')) { - throw new AppError( - 'COMMAND_FAILED', - `adb uninstall failed for ${resolved.value}`, - execFailureDetails(result), - ); + throw androidAdbResultError(`adb uninstall failed for ${resolved.value}`, result); } } return { package: resolved.value }; diff --git a/src/platforms/android/device-input-state.ts b/src/platforms/android/device-input-state.ts index 1cd837577..5702be031 100644 --- a/src/platforms/android/device-input-state.ts +++ b/src/platforms/android/device-input-state.ts @@ -1,9 +1,12 @@ import { emitDiagnostic } from '../../utils/diagnostics.ts'; -import { execFailureDetails } from '../../utils/exec.ts'; import type { DeviceInfo } from '../../kernel/device.ts'; import { AppError } from '../../kernel/errors.ts'; import { isClipboardShellUnsupported, sleep } from './adb.ts'; -import { resolveAndroidAdbExecutor, type AndroidAdbExecutor } from './adb-executor.ts'; +import { + androidAdbResultError, + resolveAndroidAdbExecutor, + type AndroidAdbExecutor, +} from './adb-executor.ts'; import { classifyAndroidInputOwner, isFallbackAndroidInputMethodPackage, @@ -85,11 +88,7 @@ export async function getAndroidKeyboardStatusWithAdb( allowFailure: true, }); if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to query Android keyboard state', - execFailureDetails(result), - ); + throw androidAdbResultError('Failed to query Android keyboard state', result); } return parseAndroidKeyboardState(result.stdout); } @@ -327,11 +326,7 @@ async function runAndroidClipboardShellCommand( ); } if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - `Failed to ${operation} Android clipboard text`, - execFailureDetails(result), - ); + throw androidAdbResultError(`Failed to ${operation} Android clipboard text`, result); } return result.stdout; } diff --git a/src/platforms/android/devices.ts b/src/platforms/android/devices.ts index f7ddb6cf2..f20889f3e 100644 --- a/src/platforms/android/devices.ts +++ b/src/platforms/android/devices.ts @@ -6,6 +6,7 @@ import type { DeviceInfo } from '../../kernel/device.ts'; import { Deadline, retryWithPolicy } from '../../utils/retry.ts'; import { resolveAndroidSerialAllowlist } from '../../utils/device-isolation.ts'; import { bootFailureHint, classifyBootFailure } from '../boot-diagnostics.ts'; +import { attachAdbFailureHint } from './adb-executor.ts'; import { ensureAndroidSdkPathConfigured } from './sdk.ts'; const EMULATOR_SERIAL_PREFIX = 'emulator-'; @@ -282,10 +283,14 @@ function parseAndroidDeviceEntries(rawOutput: string): AndroidDeviceEntry[] { } async function listAndroidDeviceEntries(): Promise { - const result = await runCmd('adb', ['devices', '-l'], { - timeoutMs: ANDROID_BOOT_PROP_TIMEOUT_MS, - }); - return parseAndroidDeviceEntries(result.stdout); + try { + const result = await runCmd('adb', ['devices', '-l'], { + timeoutMs: ANDROID_BOOT_PROP_TIMEOUT_MS, + }); + return parseAndroidDeviceEntries(result.stdout); + } catch (error) { + throw attachAdbFailureHint(error); + } } export function parseAndroidAvdList(rawOutput: string): string[] { diff --git a/src/platforms/android/logcat.ts b/src/platforms/android/logcat.ts index 25de20aeb..aa06328d0 100644 --- a/src/platforms/android/logcat.ts +++ b/src/platforms/android/logcat.ts @@ -1,7 +1,11 @@ import fs from 'node:fs'; import { AppError } from '../../kernel/errors.ts'; -import { execFailureDetails } from '../../utils/exec.ts'; -import type { AndroidAdbExecutor, AndroidAdbProcess, AndroidAdbProvider } from './adb-executor.ts'; +import { + androidAdbResultError, + type AndroidAdbExecutor, + type AndroidAdbProcess, + type AndroidAdbProvider, +} from './adb-executor.ts'; export type AndroidLogcatCaptureOptions = { lines?: number; @@ -29,11 +33,7 @@ export async function captureAndroidLogcatWithAdb( signal: options.signal, }); if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to capture Android logcat', - execFailureDetails(result), - ); + throw androidAdbResultError('Failed to capture Android logcat', result); } return result.stdout; } diff --git a/src/platforms/android/multitouch-helper.ts b/src/platforms/android/multitouch-helper.ts index 76370bad0..cbf657d05 100644 --- a/src/platforms/android/multitouch-helper.ts +++ b/src/platforms/android/multitouch-helper.ts @@ -8,6 +8,7 @@ import { findProjectRoot, readVersion } from '../../utils/version.ts'; import type { DeviceInfo } from '../../kernel/device.ts'; import type { TransformGestureParams } from '../../core/scroll-gesture.ts'; import { + androidAdbResultError, installAndroidAdbPackage, resolveAndroidAdbExecutor, resolveAndroidAdbProvider, @@ -528,11 +529,10 @@ export async function ensureAndroidMultiTouchHelper(options: { timeoutMs: ANDROID_MULTITOUCH_HELPER_INSTALL_TIMEOUT_MS, }); if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to install Android multi-touch helper', - execFailureDetails(result, { packageName, versionCode }), - ); + throw androidAdbResultError('Failed to install Android multi-touch helper', result, { + packageName, + versionCode, + }); } installedMultiTouchHelpers.add(cacheKey); return { diff --git a/src/platforms/android/perf.ts b/src/platforms/android/perf.ts index 5e9b5e21b..45a40d785 100644 --- a/src/platforms/android/perf.ts +++ b/src/platforms/android/perf.ts @@ -4,7 +4,11 @@ import type { DeviceInfo } from '../../kernel/device.ts'; import { AppError } from '../../kernel/errors.ts'; import { execFailureDetails } from '../../utils/exec.ts'; import { splitNonEmptyTrimmedLines } from '../../utils/parsing.ts'; -import { resolveAndroidAdbExecutor, type AndroidAdbExecutor } from './adb-executor.ts'; +import { + androidAdbResultError, + resolveAndroidAdbExecutor, + type AndroidAdbExecutor, +} from './adb-executor.ts'; import { parseNumericToken } from './perf-parsing.ts'; import { roundPercent } from '../perf-utils.ts'; export { @@ -150,17 +154,19 @@ export async function captureAndroidHeapSnapshot( }); if (pullResult.exitCode !== 0) { await cleanupLocalArtifact(outPath, hadLocalArtifact); - throw new AppError( - 'COMMAND_FAILED', + // The site hint wins over the classified one (attachAdbFailureHint never + // overwrites), but the classifier still tags adbFailure/retriable. + throw androidAdbResultError( `Failed to pull Android heap dump for ${packageName}`, - execFailureDetails(pullResult, { + pullResult, + { kind: 'android-hprof', package: packageName, pid, remotePath, path: outPath, hint: 'Verify the daemon can write the requested --out path and retry. The heap dump stays on-device only until cleanup runs.', - }), + }, ); } diff --git a/src/platforms/android/settings.ts b/src/platforms/android/settings.ts index 5c3f0f51a..f99a28161 100644 --- a/src/platforms/android/settings.ts +++ b/src/platforms/android/settings.ts @@ -1,5 +1,4 @@ import { AppError } from '../../kernel/errors.ts'; -import { execFailureDetails } from '../../utils/exec.ts'; import type { DeviceInfo } from '../../kernel/device.ts'; import { requireLocationCoordinates } from '../../utils/location-coordinates.ts'; import { @@ -14,6 +13,7 @@ import { import { parseAppearanceAction } from '../appearance.ts'; import { parseSettingState } from '../setting-state.ts'; import { runAndroidAdb } from './adb.ts'; +import { androidAdbResultError } from './adb-executor.ts'; import { resolveAndroidApp } from './app-lifecycle.ts'; const ANDROID_ANIMATION_SCALE_SETTINGS = [ @@ -253,11 +253,7 @@ async function resolveAndroidAppearanceTarget( allowFailure: true, }); if (currentResult.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to read current Android appearance', - execFailureDetails(currentResult), - ); + throw androidAdbResultError('Failed to read current Android appearance', currentResult); } const current = parseAndroidAppearance(currentResult.stdout, currentResult.stderr); if (!current) { diff --git a/src/platforms/android/snapshot-helper-install.ts b/src/platforms/android/snapshot-helper-install.ts index 1c78c5aa0..c18428d01 100644 --- a/src/platforms/android/snapshot-helper-install.ts +++ b/src/platforms/android/snapshot-helper-install.ts @@ -1,10 +1,9 @@ -import { AppError } from '../../kernel/errors.ts'; -import { execFailureDetails } from '../../utils/exec.ts'; import { readAndroidSnapshotHelperInstallOptions, verifyAndroidSnapshotHelperArtifact, } from './snapshot-helper-artifact.ts'; import { + androidAdbResultError, installAndroidAdbPackage, type AndroidAdbExecutor, type AndroidAdbProvider, @@ -118,11 +117,10 @@ export async function ensureAndroidSnapshotHelper(options: { ); if (result.exitCode !== 0) { forgetInstalledSnapshotHelper(installCacheKey); - throw new AppError( - 'COMMAND_FAILED', - 'Failed to install Android snapshot helper', - execFailureDetails(result, { packageName, versionCode }), - ); + throw androidAdbResultError('Failed to install Android snapshot helper', result, { + packageName, + versionCode, + }); } rememberInstalledSnapshotHelper(installCacheKey, versionCode); diff --git a/src/platforms/android/snapshot.ts b/src/platforms/android/snapshot.ts index 528117fe5..0dda4069f 100644 --- a/src/platforms/android/snapshot.ts +++ b/src/platforms/android/snapshot.ts @@ -1,7 +1,6 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import { withRetry } from '../../utils/retry.ts'; -import { execFailureDetails } from '../../utils/exec.ts'; import { AppError, normalizeError, toAppErrorCode } from '../../kernel/errors.ts'; import { emitDiagnostic, withDiagnosticTimer } from '../../utils/diagnostics.ts'; import type { DeviceInfo } from '../../kernel/device.ts'; @@ -23,6 +22,8 @@ import { type AndroidUiHierarchy, } from './ui-hierarchy.ts'; import { + androidAdbResultError, + classifyAdbFailure, resolveAndroidAdbExecutor, resolveAndroidAdbProvider, type AndroidAdbProvider, @@ -56,12 +57,10 @@ const HELPER_CAPTURE_TIMEOUT_MS = 5_000; const HELPER_COMMAND_TIMEOUT_MS = 30_000; const HELPER_RUNTIME_RESET_DELAY_MS = 150; const HELPER_RUNTIME_RESET_TIMEOUT_MS = 2_000; -const RETRYABLE_ADB_STDERR_PATTERNS = [ - 'device offline', - 'device not found', - 'transport error', - 'connection reset', - 'broken pipe', +// Transient adb transport families (device offline/not found, transport error, +// connection reset, broken pipe) come from the shared classifier; these extras +// are snapshot-specific races (dump timeouts, dump-file reads) worth retrying. +const SNAPSHOT_ONLY_RETRYABLE_ADB_STDERR_PATTERNS = [ 'timed out', 'no such file or directory', ] as const; @@ -683,11 +682,9 @@ async function dumpUiHierarchyOnce(adb: AndroidAdbExecutor): Promise { }); const reportedPath = readDumpPath(dumpResult.stdout, dumpResult.stderr); if (dumpResult.exitCode !== 0 && !reportedPath) { - throw new AppError( - 'COMMAND_FAILED', - 'uiautomator dump did not return XML', - execFailureDetails(dumpResult, { reason: 'missing_fresh_dump' }), - ); + throw androidAdbResultError('uiautomator dump did not return XML', dumpResult, { + reason: 'missing_fresh_dump', + }); } const actualPath = reportedPath ?? dumpPath; @@ -724,7 +721,8 @@ function isRetryableAdbError(err: unknown): boolean { if (err.code !== 'COMMAND_FAILED') return false; const rawStderr = err.details?.stderr; const stderr = (typeof rawStderr === 'string' ? rawStderr : '').toLowerCase(); - return RETRYABLE_ADB_STDERR_PATTERNS.some((pattern) => stderr.includes(pattern)); + if (classifyAdbFailure(stderr)?.retriable === true) return true; + return SNAPSHOT_ONLY_RETRYABLE_ADB_STDERR_PATTERNS.some((pattern) => stderr.includes(pattern)); } function isUiHierarchyDumpTimeout(err: unknown): err is AppError { diff --git a/src/utils/__tests__/errors.test.ts b/src/utils/__tests__/errors.test.ts index f14e445c9..65f949b13 100644 --- a/src/utils/__tests__/errors.test.ts +++ b/src/utils/__tests__/errors.test.ts @@ -78,6 +78,22 @@ test('normalizeError provides app discovery guidance for app-not-installed error ); }); +test('normalizeError lifts details.retriable to the top level and strips it from details', () => { + const normalized = normalizeError( + new AppError('COMMAND_FAILED', 'adb exited with code 1', { + stderr: 'error: device offline', + retriable: true, + }), + ); + assert.equal(normalized.retriable, true); + assert.equal(Object.hasOwn(normalized.details ?? {}, 'retriable'), false); +}); + +test('normalizeError omits retriable when the throw site did not classify it', () => { + const normalized = normalizeError(new AppError('COMMAND_FAILED', 'adb exited with code 1')); + assert.equal(Object.hasOwn(normalized, 'retriable'), false); +}); + test('toAppErrorCode falls back when code is missing or empty', () => { assert.equal(toAppErrorCode(undefined), 'COMMAND_FAILED'); assert.equal(toAppErrorCode(''), 'COMMAND_FAILED');