From 280291ff4c4be5f9cd42222de7a0cad7e071076f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 4 Jul 2026 09:00:53 +0200 Subject: [PATCH 1/2] feat: classify adb failures with actionable hints at the executor level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android COMMAND_FAILED errors surfaced raw adb stderr with only the generic retry-with-debug hint; the sole stderr classification lived in snapshot.ts and was retry-only. Add a central classifier in adb-executor.ts that recognizes the common adb failure families (device offline/unauthorized/not found, more than one device, no devices, server version mismatch, connection drops, INSTALL_FAILED_* variants) and attaches the resolved hint — plus retriable for clearly transient families — to the AppError details in both executor funnels (local serial exec and provider-scoped exec), so every adb call site benefits without per-site changes. The adb devices -l discovery call is wrapped too, where server-mismatch/no-devices failures actually surface. snapshot.ts now consumes the shared classifier for its retry decision, keeping only its snapshot-specific dump-race patterns. normalizeError lifts details.retriable to the top-level error (mirroring the hint lift) and the daemon typed-error graft lets a throw-site classification win over the code-level policy. Also fix the port-reverse removal path throwing a bare Error that surfaced as UNKNOWN. Update session-appstate-input-perf.test.ts: its Android sampling-failure fixture uses stderr "error: device offline", which the classifier now recognizes, so error.hint carries the actionable adb-reconnect hint instead of the generic "retry with --debug". The result shape (hint string, details.metric/package, code) is unchanged — only the hint text improved, which is this change's intent — so the test expectation moves to the new hint rather than reverting production. --- .../session-appstate-input-perf.test.ts | 2 +- src/daemon/request-router.ts | 4 +- src/kernel/errors.ts | 11 + .../android/__tests__/adb-executor.test.ts | 191 +++++++++++++++++ src/platforms/android/adb-executor.ts | 195 ++++++++++++++++-- src/platforms/android/devices.ts | 13 +- src/platforms/android/snapshot.ts | 14 +- src/utils/__tests__/errors.test.ts | 16 ++ 8 files changed, 418 insertions(+), 28 deletions(-) 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..4340f2230 100644 --- a/src/platforms/android/__tests__/adb-executor.test.ts +++ b/src/platforms/android/__tests__/adb-executor.test.ts @@ -18,6 +18,8 @@ vi.mock('../../../utils/exec.ts', async (importOriginal) => { }); import { + attachAdbFailureHint, + classifyAdbFailure, createAndroidPortReverseManager, createDeviceAdbExecutor, createLocalAndroidAdbProvider, @@ -28,6 +30,7 @@ import { withAndroidAdbProvider, } from '../adb-executor.ts'; import { runCmd, runCmdBackground } from '../../../utils/exec.ts'; +import { AppError } from '../../../kernel/errors.ts'; const mockRunCmd = vi.mocked(runCmd); const mockRunCmdBackground = vi.mocked(runCmdBackground); @@ -305,3 +308,191 @@ 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('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/adb-executor.ts b/src/platforms/android/adb-executor.ts index d94eed60f..a2f0f2e92 100644 --- a/src/platforms/android/adb-executor.ts +++ b/src/platforms/android/adb-executor.ts @@ -185,24 +185,176 @@ 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; +} + +function withAdbFailureHintExecutor(exec: AndroidAdbExecutor): AndroidAdbExecutor { + return async (args, options) => { + try { + return await exec(args, options); + } catch (error) { + throw attachAdbFailureHint(error); + } + }; +} + 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 withAdbFailureHintExecutor( + 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 { @@ -386,7 +538,13 @@ export async function withAndroidAdbProvider( ): Promise { if (!provider) return await fn(); const normalized = typeof provider === 'function' ? { exec: provider } : provider; - const scope = { provider: normalized, serial: options.serial }; + // Wrap once at scope installation so every consumer — the command-executor + // override and direct resolveAndroidAdb* lookups — gets classified failure hints. + const enriched: AndroidAdbProvider = { + ...normalized, + exec: withAdbFailureHintExecutor(normalized.exec), + }; + const scope = { provider: enriched, serial: options.serial }; const override = createAndroidCommandExecutorOverride(scope); return await androidAdbProviderScope.run( scope, @@ -439,7 +597,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/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/snapshot.ts b/src/platforms/android/snapshot.ts index 528117fe5..f84b5853f 100644 --- a/src/platforms/android/snapshot.ts +++ b/src/platforms/android/snapshot.ts @@ -23,6 +23,7 @@ import { type AndroidUiHierarchy, } from './ui-hierarchy.ts'; import { + 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; @@ -724,7 +723,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'); From ff45566296289f3744c16ce636df191287c27671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 4 Jul 2026 10:41:21 +0200 Subject: [PATCH 2/2] feat: classify tolerated adb failures and semantic provider methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up closing two classifier coverage gaps: 1. allowFailure paths bypassed the classifier: the executor wrapper only enriches THROWN errors, but many Android flows run adb with allowFailure, inspect the nonzero result, and throw a fresh AppError built from its stdout/stderr — so e.g. "device offline" during uninstall still surfaced the generic hint. There was no common construction point for these errors, so androidAdbResultError in adb-executor.ts now IS that point: it builds the COMMAND_FAILED error from a tolerated result and runs it through the classifier, keeping hint strings central. The result-inspecting throw sites (app launch/uninstall, appearance read, keyboard/clipboard queries, package listing, logcat capture, helper APK installs, heap-dump pull, uiautomator dump) route through it; a site hint (heap-dump pull) still wins because attachAdbFailureHint never overwrites. Composes with #1072 rather than replacing it: androidAdbResultError builds its details via execFailureDetails for nonzero exits, so normalizeError still suffixes the curated message with the first stderr line while the classified hint/retriable/adbFailure ride along. Semantic failures thrown at exit 0 (the am start error-on-stdout path) stay unflagged, matching #1072's deliberate exit-0 exclusion. 2. Provider-scoped enrichment only wrapped exec, but the transfer helpers prefer the semantic provider methods, so a provider whose install rejects with INSTALL_FAILED output kept the generic hint. Enrichment now lives in normalizeAndroidAdbProvider — the single funnel every provider passes through (scope installation and explicitly passed providers alike) — wrapping exec plus pull/install/installBundle; a WeakSet keeps repeated normalization from stacking wrappers. The local provider needs no wrap since its methods delegate to the already enriched serial executor. Tests: androidAdbResultError classification, hint-plus-excerpt composition through normalizeError, exit-0 no-excerpt guard, site-hint precedence, and provider install/pull classification in adb-executor.test.ts; an allowFailure regression on a real production path (keyboard state query with a nonzero device-offline result) in device-input-state.test.ts. --- .../android/__tests__/adb-executor.test.ts | 108 +++++++++++++++++- .../__tests__/device-input-state.test.ts | 22 ++++ src/platforms/android/adb-executor.ts | 82 ++++++++++--- src/platforms/android/app-helpers.ts | 10 +- src/platforms/android/app-lifecycle.ts | 14 +-- src/platforms/android/device-input-state.ts | 19 ++- src/platforms/android/logcat.ts | 14 +-- src/platforms/android/multitouch-helper.ts | 10 +- src/platforms/android/perf.ts | 16 ++- src/platforms/android/settings.ts | 8 +- .../android/snapshot-helper-install.ts | 12 +- src/platforms/android/snapshot.ts | 10 +- 12 files changed, 243 insertions(+), 82 deletions(-) diff --git a/src/platforms/android/__tests__/adb-executor.test.ts b/src/platforms/android/__tests__/adb-executor.test.ts index 4340f2230..108a9bf0c 100644 --- a/src/platforms/android/__tests__/adb-executor.test.ts +++ b/src/platforms/android/__tests__/adb-executor.test.ts @@ -18,6 +18,7 @@ vi.mock('../../../utils/exec.ts', async (importOriginal) => { }); import { + androidAdbResultError, attachAdbFailureHint, classifyAdbFailure, createAndroidPortReverseManager, @@ -30,7 +31,7 @@ import { withAndroidAdbProvider, } from '../adb-executor.ts'; import { runCmd, runCmdBackground } from '../../../utils/exec.ts'; -import { AppError } from '../../../kernel/errors.ts'; +import { AppError, normalizeError } from '../../../kernel/errors.ts'; const mockRunCmd = vi.mocked(runCmd); const mockRunCmdBackground = vi.mocked(runCmdBackground); @@ -467,6 +468,111 @@ test('port reverse removal failures surface as classified AppErrors, not bare Er 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', 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 a2f0f2e92..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, @@ -325,22 +326,79 @@ export function attachAdbFailureHint(error: T): T { return error; } -function withAdbFailureHintExecutor(exec: AndroidAdbExecutor): AndroidAdbExecutor { - return async (args, options) => { +/** + * 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 exec(args, options); + 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 withAdbFailureHintExecutor( + 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. @@ -467,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 = { @@ -537,13 +592,10 @@ export async function withAndroidAdbProvider( fn: () => Promise, ): Promise { if (!provider) return await fn(); - const normalized = typeof provider === 'function' ? { exec: provider } : provider; - // Wrap once at scope installation so every consumer — the command-executor - // override and direct resolveAndroidAdb* lookups — gets classified failure hints. - const enriched: AndroidAdbProvider = { - ...normalized, - exec: withAdbFailureHintExecutor(normalized.exec), - }; + // 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( 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/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 f84b5853f..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,7 @@ import { type AndroidUiHierarchy, } from './ui-hierarchy.ts'; import { + androidAdbResultError, classifyAdbFailure, resolveAndroidAdbExecutor, resolveAndroidAdbProvider, @@ -682,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;