Skip to content

fix(devices): keep the device list when AVD enumeration fails - #11649

Open
thelonewander3r wants to merge 1 commit into
pingdotgg:mainfrom
thelonewander3r:fix/device-list-partial-failure
Open

thelonewander3r wants to merge 1 commit into
pingdotgg:mainfrom
thelonewander3r:fix/device-list-partial-failure

Conversation

@thelonewander3r

@thelonewander3r thelonewander3r commented Sep 13, 2026 •

Copy link
Copy Markdown

What Changed

fetchDevices no longer returns a DeviceOperationError when emulator -list-avds exits non-zero. The failure is collected into the same detail channel the hub's own errors already use, and the devices gathered before it are returned.

Why

The hub reports per-backend problems through list.errors while still returning whatever it found, and refresh forwards those into hostStatuses[hostId].detail with the host still ready. The AVD call was the one place that escalated instead:

if (avds.code !== 0) {
  return yield* new DeviceOperationError({ operation: "list", ... });
}

That throws before publish ever runs, so a broken Android toolchain takes down the entire list — the iOS simulators already fetched are discarded, the host is marked failed, and the UI keeps showing stale devices with no indication which platform actually broke. The user-visible result is "Device list failed: The device command failed (exit code 127)" and nothing else.

Degrading keeps the working platforms listing and names Android as the part that failed.

Exit 127 gets its own message: that code comes from ProcessRunner when the spawn never happened, so there is no exit status worth showing and the stderr behind it is an internal error type. Other codes came from the emulator itself, where the first line of stderr is usually the whole explanation.

Tests

Two cases in DeviceService.test.ts: a spawn failure (127) and a real emulator failure (exit 2 with stderr). Both assert the host stays ready and the detail carries the right text. Restoring the error return makes both fail with expected 'failed' to be 'ready'.

The fixture gained one optional parameter to stub the host command result.

Related

Stands alone, but shares a symptom with #11647, which fixes the stale SDK resolution that produced the 127 in the first place. This one limits the blast radius of any such failure; that one removes this cause.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Device status now remains available when Android emulator discovery fails.
    • Added clearer error details for missing emulator commands, including guidance on installing the Android Emulator or configuring ANDROID_HOME.
    • Device status now reports both backend and emulator-discovery failures together.

`fetchDevices` turned any non-zero exit from `emulator -list-avds` into a
`DeviceOperationError`, discarding the simulators and emulators the hub had
already returned and marking the whole host failed. The hub reports its own
per-backend problems through `detail` while still returning what it found, so
one platform's toolchain being broken should not be the exception.

Collect the failure into the same `detail` channel instead. The host stays
ready, the platforms that do work keep listing, and the message names Android
as the part that failed rather than leaving the user with an exit code.

Exit 127 is ProcessRunner's code for a spawn that never happened, so it is
reported as the emulator command not starting rather than as an exit status.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 13, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 5a311bb

Macroscope's review found this PR approvable — This narrowly scoped device-discovery fix preserves working device results when AVD enumeration fails and surfaces an actionable diagnostic instead of failing the entire host. The behavior is isolated to existing error handling and is covered by focused tests.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 47782abf-e2ad-4f09-99c3-a41bcf43578b

📥 Commits

Reviewing files that changed from the base of the PR and between 77bca8b and 5a311bb.

📒 Files selected for processing (2)
  • apps/server/src/device/DeviceService.test.ts
  • apps/server/src/device/DeviceService.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

DeviceService.fetchDevices now preserves device discovery when emulator -list-avds fails. It adds formatted failure details to existing backend errors. Tests cover exit 127 and exit 2 while confirming the host remains ready.

Changes

AVD failure handling

Layer / File(s) Summary
AVD failure reporting
apps/server/src/device/DeviceService.ts
The service formats AVD command failures, aggregates them with backend errors, and continues to parse successful AVD listings.
AVD failure tests
apps/server/src/device/DeviceService.test.ts
The test fixture accepts custom host command results. Tests cover spawn failure and emulator failure while the host remains ready.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 5a311

AVD enumeration failures now leave other discovered devices available and provide actionable status details. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preserving the device list when AVD enumeration fails.
Description check ✅ Passed The description explains what changed, why the change is needed, and how it was tested. The UI Changes section is correctly omitted because the change is not a UI change. The repository checklist is m…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant