Handle locked and reconnecting iOS devices#823
Conversation
Improve physical iOS device launch reliability in xtask. When launching on a physical iPhone fails due to lock state, retry with a bounded delay instead of failing immediately. When resolving devices, refresh the selected device connection before selecting an available target, and add as a shorthand alias for .
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds graceful handling for two physical iOS device edge cases during
Confidence Score: 4/5The change is confined to the xtask dev-tooling layer and does not touch production app code; the worst outcome of any bug is a confusing error message during local development. The locked-device and reconnection retry logic is straightforward and well-structured. The only notable issue is an unreachable Ok(Vec::new()) at the end of available_ios_devices_with_connection_refresh — it is dead code for the current constant value but could quietly return an empty list if the constant were ever reduced to zero, making the downstream error harder to trace. rust/xtask/src/ios.rs — specifically the available_ios_devices_with_connection_refresh function and its post-loop fallback. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[build-run-ios] --> B[available_ios_devices_with_connection_refresh]
B --> C{device found\nin listing?}
C -- yes --> D[run_ios_device: install app]
C -- no, attempt < max --> E[refresh_matching_ios_device_connection\ndevicectl device info]
E --> F[sleep 1s] --> B
C -- no, last attempt --> D
D --> G[launch_ios_device_app]
G --> H{launch\nsucceeded?}
H -- yes --> I[✅ App running]
H -- no, locked --> J{past 30s\ndeadline?}
J -- no --> K[print warning + say prompt\non first occurrence]
K --> L[sleep 5s] --> G
J -- yes --> M[❌ bail: unlock iPhone and retry]
H -- no, other error --> N[❌ bail with stdout/stderr]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[build-run-ios] --> B[available_ios_devices_with_connection_refresh]
B --> C{device found\nin listing?}
C -- yes --> D[run_ios_device: install app]
C -- no, attempt < max --> E[refresh_matching_ios_device_connection\ndevicectl device info]
E --> F[sleep 1s] --> B
C -- no, last attempt --> D
D --> G[launch_ios_device_app]
G --> H{launch\nsucceeded?}
H -- yes --> I[✅ App running]
H -- no, locked --> J{past 30s\ndeadline?}
J -- no --> K[print warning + say prompt\non first occurrence]
K --> L[sleep 5s] --> G
J -- yes --> M[❌ bail: unlock iPhone and retry]
H -- no, other error --> N[❌ bail with stdout/stderr]
Reviews (1): Last reviewed commit: "Handle locked and reconnecting iOS devic..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fn available_ios_devices_with_connection_refresh( | ||
| sh: &Shell, | ||
| selector: &DeviceSelector, | ||
| ) -> Result<Vec<ResolvedDevice>> { | ||
| for attempt in 0..IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS { | ||
| let devices = available_ios_devices(sh)?; | ||
| if ios_devices_include_selector(&devices, selector) { | ||
| return Ok(devices); | ||
| } | ||
|
|
||
| if attempt + 1 == IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS { | ||
| return Ok(devices); | ||
| } | ||
|
|
||
| refresh_matching_ios_device_connection(sh, selector)?; | ||
| std::thread::sleep(Duration::from_secs(1)); | ||
| } | ||
|
|
||
| Ok(Vec::new()) | ||
| } |
There was a problem hiding this comment.
Ok(Vec::new()) after the loop is unreachable for any IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS >= 1. With the current constant of 2, the loop body always exits via an explicit return on the last iteration (attempt + 1 == IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS). The dead path is harmless today, but if the constant were ever set to 0 the function would silently return an empty vec instead of attempting any lookup, making the downstream "No available paired iOS device found" error confusing to diagnose. Making this unreachability explicit removes the ambiguity.
| fn available_ios_devices_with_connection_refresh( | |
| sh: &Shell, | |
| selector: &DeviceSelector, | |
| ) -> Result<Vec<ResolvedDevice>> { | |
| for attempt in 0..IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS { | |
| let devices = available_ios_devices(sh)?; | |
| if ios_devices_include_selector(&devices, selector) { | |
| return Ok(devices); | |
| } | |
| if attempt + 1 == IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS { | |
| return Ok(devices); | |
| } | |
| refresh_matching_ios_device_connection(sh, selector)?; | |
| std::thread::sleep(Duration::from_secs(1)); | |
| } | |
| Ok(Vec::new()) | |
| } | |
| fn available_ios_devices_with_connection_refresh( | |
| sh: &Shell, | |
| selector: &DeviceSelector, | |
| ) -> Result<Vec<ResolvedDevice>> { | |
| assert!( | |
| IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS >= 1, | |
| "IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS must be at least 1" | |
| ); | |
| for attempt in 0..IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS { | |
| let devices = available_ios_devices(sh)?; | |
| if ios_devices_include_selector(&devices, selector) { | |
| return Ok(devices); | |
| } | |
| if attempt + 1 == IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS { | |
| return Ok(devices); | |
| } | |
| refresh_matching_ios_device_connection(sh, selector)?; | |
| std::thread::sleep(Duration::from_secs(1)); | |
| } | |
| unreachable!("loop always returns on the last iteration") | |
| } |
Guard the iOS device connection refresh loop against a zero retry count by validating `IOS_DEVICE_CONNECTION_RETRY_ATTEMPTS` up front and returning an explicit error. This also removes the fallback empty result in favor of an unreachable assertion, making the retry logic's control flow clearer and safer.
1 participant
No description provided.