fix: kill wda if StartAgent times out launching on simulator #124
Conversation
WalkthroughThe changes enhance WebDriverAgent installation by introducing architecture-aware artifact selection (arm64 vs x86_64), attempting local binary retrieval first before falling back to GitHub download, improving error handling with proper error wrapping, and adding process termination fallback when agent startup fails. Changes
Sequence DiagramsequenceDiagram
participant SimDev as SimulatorDevice
participant LocalFS as Local Filesystem
participant GitHub as GitHub
participant WDA as WebDriverAgent
rect rgb(200, 220, 255)
Note over SimDev,WDA: InstallWebDriverAgent Flow
SimDev->>SimDev: Detect architecture (arm64/x86_64)
SimDev->>LocalFS: Check MOBILECLI_WDA_PATH<br/>(or default location)
alt Local binary exists
LocalFS-->>SimDev: WDA binary found
SimDev->>SimDev: Log: using local WDA
else Local binary not found
rect rgb(255, 240, 200)
SimDev->>GitHub: Download WDA from GitHub
GitHub-->>SimDev: WDA binary + dependencies
SimDev->>SimDev: Unzip artifacts
end
end
SimDev->>SimDev: Conditional cleanup of downloads
end
rect rgb(220, 255, 220)
Note over SimDev,WDA: StartAgent Flow
SimDev->>SimDev: Environment-driven port discovery
SimDev->>WDA: Start WebDriverAgent process
SimDev->>WDA: WaitForAgent
alt WaitForAgent succeeds
WDA-->>SimDev: Agent ready
else WaitForAgent fails
rect rgb(255, 200, 200)
SimDev->>WDA: Terminate WDA process (fallback)
end
SimDev-->>SimDev: Return error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
devices/simulator.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
devices/simulator.go (2)
utils/logger.go (1)
Verbose(19-23)utils/zipfile.go (1)
Unzip(12-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
devices/simulator.go (5)
11-11: LGTM!The
runtimeimport is correctly added to support architecture detection.
294-299: LGTM!The architecture detection logic correctly maps Go's "amd64" to x86_64 and defaults to arm64.
306-329: LGTM!The download logic is well-structured with proper error handling, cleanup on failure, and informative logging.
331-414: LGTM!The refactored installation logic correctly implements the two-tier approach (local file first, GitHub fallback) with appropriate conditional cleanup. The error wrapping improvements and verbose logging enhance maintainability.
608-610: LGTM! This correctly implements the fix described in the PR title.The termination fallback properly cleans up the WDA process when agent startup fails, preventing dangling processes. Ignoring the termination error is appropriate since the original WaitForAgent error is more important to surface.
| tmpFile, err := os.CreateTemp("", "wda-*.zip") | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to create temp file: %v", err) | ||
| } | ||
| _ = tmpFile.Close() | ||
|
|
||
| utils.Verbose("Downloading WebDriverAgent to: %s", tmpFile.Name()) | ||
|
|
||
| err = utils.DownloadFile(url, tmpFile.Name()) | ||
| if err != nil { | ||
| _ = os.Remove(tmpFile.Name()) | ||
| return "", fmt.Errorf("failed to download WebDriverAgent: %w", err) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify WebDriverAgent v9.15.1 and check for latest version
echo "=== Checking WebDriverAgent v9.15.1 ==="
if gh api repos/appium/WebDriverAgent/releases/tags/v9.15.1 --jq '.tag_name, .published_at' 2>/dev/null; then
echo "✓ Version v9.15.1 found"
else
echo "✗ Version v9.15.1 NOT found"
fi
echo -e "\n=== Checking latest WebDriverAgent release ==="
gh api repos/appium/WebDriverAgent/releases/latest --jq '.tag_name, .published_at' 2>/dev/null
echo -e "\n=== Checking recent releases (last 5) ==="
gh api repos/appium/WebDriverAgent/releases --jq '.[0:5] | .[] | "\(.tag_name) - \(.published_at)"' 2>/dev/nullRepository: mobile-next/mobilecli
Length of output: 432
Update the hardcoded WebDriverAgent version from v9.15.1 to the latest stable release.
The version v9.15.1 (released July 2025) is outdated. The latest stable release is v10.2.3 (released November 2025). This represents a full major version gap and 4+ months of updates. Consider updating the hardcoded version to v10.2.3 or implementing a mechanism to automatically fetch the latest release to avoid future manual updates.
🤖 Prompt for AI Agents
In devices/simulator.go around lines 301 to 304, the WebDriverAgent download URL
is hardcoded to release v9.15.1; update the string to use v10.2.3 (the current
stable) or replace the hardcoded version with a small helper that fetches the
latest release tag from GitHub API and uses that tag to construct the URL.
Ensure any HTTP call handles errors and falls back to a sensible default tag,
and update unit/integration tests if they assert the old URL.
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.