Skip to content

Commit 1d875ba

Browse files
thymikeethymikee
committed
fix: harden ios screenshot fallback timeouts
1 parent 4df80af commit 1d875ba

3 files changed

Lines changed: 173 additions & 27 deletions

File tree

‎src/platforms/ios/__tests__/index.test.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
resolveSimulatorRunnerScreenshotCandidatePaths,
2626
} from '../screenshot.ts';
2727
import type { DeviceInfo } from '../../../utils/device.ts';
28+
import { withDiagnosticsScope } from '../../../utils/diagnostics.ts';
2829
import { AppError } from '../../../utils/errors.ts';
2930

3031
const IOS_TEST_DEVICE: DeviceInfo = {
@@ -146,6 +147,14 @@ test('shouldRetryIosSimulatorScreenshot detects simulator screen-surface timeout
146147
assert.equal(shouldRetryIosSimulatorScreenshot(error), true);
147148
});
148149

150+
test('shouldRetryIosSimulatorScreenshot detects timed out simctl screenshot command', () => {
151+
const error = new AppError('COMMAND_FAILED', 'xcrun timed out after 20000ms', {
152+
args: ['simctl', 'io', 'sim-1', 'screenshot', '/tmp/out.png'],
153+
timeoutMs: 20_000,
154+
});
155+
assert.equal(shouldRetryIosSimulatorScreenshot(error), true);
156+
});
157+
149158
test('shouldRetryIosSimulatorScreenshot ignores unrelated screenshot failures', () => {
150159
const error = new AppError('COMMAND_FAILED', 'Failed to capture iOS screenshot', {
151160
stderr: 'No such file or directory',
@@ -184,6 +193,83 @@ test('captureSimulatorScreenshotWithFallback falls back to runner after retry ex
184193
assert.equal(runnerCalls, 1);
185194
});
186195

196+
test('captureSimulatorScreenshotWithFallback falls back to runner after simctl screenshot timeout', async () => {
197+
let runnerCalls = 0;
198+
await captureSimulatorScreenshotWithFallback(
199+
IOS_TEST_SIMULATOR,
200+
'/tmp/out.png',
201+
'com.example.app',
202+
{
203+
ensureBooted: async () => {},
204+
captureWithRetry: async () => {
205+
throw new AppError('COMMAND_FAILED', 'xcrun timed out after 20000ms', {
206+
args: ['simctl', 'io', 'sim-1', 'screenshot', '/tmp/out.png'],
207+
timeoutMs: 20_000,
208+
});
209+
},
210+
captureWithRunner: async () => {
211+
runnerCalls += 1;
212+
},
213+
shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot,
214+
},
215+
);
216+
assert.equal(runnerCalls, 1);
217+
});
218+
219+
test('captureSimulatorScreenshotWithFallback emits fallback diagnostic before using runner', async () => {
220+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'agent-device-ios-screenshot-diag-test-'));
221+
const logPath = path.join(tmpDir, 'diag.ndjson');
222+
try {
223+
await withDiagnosticsScope(
224+
{
225+
debug: true,
226+
logPath,
227+
session: 'ios-test',
228+
requestId: 'req-1',
229+
command: 'screenshot',
230+
},
231+
async () => {
232+
await captureSimulatorScreenshotWithFallback(
233+
IOS_TEST_SIMULATOR,
234+
'/tmp/out.png',
235+
'com.example.app',
236+
{
237+
ensureBooted: async () => {},
238+
captureWithRetry: async () => {
239+
throw new AppError('COMMAND_FAILED', 'xcrun timed out after 20000ms', {
240+
args: ['simctl', 'io', 'sim-1', 'screenshot', '/tmp/out.png'],
241+
timeoutMs: 20_000,
242+
});
243+
},
244+
captureWithRunner: async () => {},
245+
shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot,
246+
},
247+
);
248+
},
249+
);
250+
251+
const log = await waitForFileText(logPath);
252+
assert.match(log, /"phase":"ios_screenshot_fallback"/);
253+
assert.match(log, /"from":"simctl_screenshot"/);
254+
assert.match(log, /"to":"runner"/);
255+
} finally {
256+
await fs.rm(tmpDir, { recursive: true, force: true });
257+
}
258+
});
259+
260+
async function waitForFileText(filePath: string, attempts = 20): Promise<string> {
261+
let lastError: unknown;
262+
for (let attempt = 0; attempt < attempts; attempt += 1) {
263+
try {
264+
return await fs.readFile(filePath, 'utf8');
265+
} catch (error) {
266+
lastError = error;
267+
await new Promise((resolve) => setTimeout(resolve, 10));
268+
}
269+
}
270+
throw lastError;
271+
}
272+
187273
test('resolveSimulatorRunnerScreenshotCandidatePaths includes tmp-based and basename fallbacks', () => {
188274
const containerPath = '/tmp/container';
189275
const candidates = resolveSimulatorRunnerScreenshotCandidatePaths(

‎src/platforms/ios/config.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,18 @@ export const IOS_DEVICECTL_TIMEOUT_MS = resolveTimeoutMs(
2525
1_000,
2626
);
2727

28+
export const IOS_SIMULATOR_SCREENSHOT_TIMEOUT_MS = resolveTimeoutMs(
29+
process.env.AGENT_DEVICE_IOS_SIMULATOR_SCREENSHOT_TIMEOUT_MS,
30+
20_000,
31+
1_000,
32+
);
33+
34+
export const IOS_RUNNER_SCREENSHOT_COPY_TIMEOUT_MS = resolveTimeoutMs(
35+
process.env.AGENT_DEVICE_IOS_RUNNER_SCREENSHOT_COPY_TIMEOUT_MS,
36+
20_000,
37+
1_000,
38+
);
39+
2840
export const IOS_SIMULATOR_SCREENSHOT_RETRY_MAX_ATTEMPTS = 5;
2941
export const IOS_SIMULATOR_SCREENSHOT_RETRY_BASE_DELAY_MS = 1_000;
3042
export const IOS_SIMULATOR_SCREENSHOT_RETRY_MAX_DELAY_MS = 5_000;

‎src/platforms/ios/screenshot.ts‎

Lines changed: 75 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
11
import { promises as fs } from 'node:fs';
22
import path from 'node:path';
33
import type { DeviceInfo } from '../../utils/device.ts';
4+
import { emitDiagnostic } from '../../utils/diagnostics.ts';
45
import { AppError } from '../../utils/errors.ts';
56
import { runCmd } from '../../utils/exec.ts';
6-
import { retryWithPolicy } from '../../utils/retry.ts';
7+
import { Deadline, retryWithPolicy } from '../../utils/retry.ts';
78

89
import {
10+
IOS_RUNNER_SCREENSHOT_COPY_TIMEOUT_MS,
911
IOS_SIMULATOR_SCREENSHOT_RETRY_BASE_DELAY_MS,
1012
IOS_SIMULATOR_SCREENSHOT_RETRY_MAX_ATTEMPTS,
1113
IOS_SIMULATOR_SCREENSHOT_RETRY_MAX_DELAY_MS,
14+
IOS_SIMULATOR_SCREENSHOT_TIMEOUT_MS,
1215
} from './config.ts';
1316
import { runIosDevicectl } from './devicectl.ts';
1417
import { runIosRunnerCommand, IOS_RUNNER_CONTAINER_BUNDLE_IDS } from './runner-client.ts';
@@ -57,6 +60,7 @@ export async function screenshotIos(device: DeviceInfo, outPath: string, appBund
5760
if (!shouldFallbackToRunnerForIosScreenshot(error)) {
5861
throw error;
5962
}
63+
emitScreenshotFallbackDiagnostic(device, 'devicectl_screenshot', 'runner', error);
6064
}
6165

6266
await captureScreenshotViaRunner(device, outPath, appBundleId);
@@ -80,18 +84,22 @@ export async function captureSimulatorScreenshotWithFallback(
8084
if (!deps.shouldFallbackToRunner(error)) {
8185
throw error;
8286
}
87+
emitScreenshotFallbackDiagnostic(device, 'simctl_screenshot', 'runner', error);
8388
}
8489
await deps.captureWithRunner(device, outPath, appBundleId);
8590
}
8691

8792
async function captureSimulatorScreenshotWithRetry(device: DeviceInfo, outPath: string): Promise<void> {
93+
const deadline = Deadline.fromTimeoutMs(IOS_SIMULATOR_SCREENSHOT_TIMEOUT_MS);
8894
await focusIosSimulatorWindow();
8995
await retryWithPolicy(
90-
async ({ attempt }) => {
96+
async ({ attempt, deadline: attemptDeadline }) => {
9197
if (attempt > 1) {
9298
await focusIosSimulatorWindow();
9399
}
94-
await runSimctl(device, ['io', device.id, 'screenshot', outPath]);
100+
await runSimctl(device, ['io', device.id, 'screenshot', outPath], {
101+
timeoutMs: Math.max(1_000, attemptDeadline?.remainingMs() ?? IOS_SIMULATOR_SCREENSHOT_TIMEOUT_MS),
102+
});
95103
},
96104
{
97105
maxAttempts: IOS_SIMULATOR_SCREENSHOT_RETRY_MAX_ATTEMPTS,
@@ -100,7 +108,7 @@ async function captureSimulatorScreenshotWithRetry(device: DeviceInfo, outPath:
100108
jitter: 0.2,
101109
shouldRetry: (error) => shouldRetryIosSimulatorScreenshot(error),
102110
},
103-
{ phase: 'ios_simulator_screenshot' },
111+
{ deadline, phase: 'ios_simulator_screenshot' },
104112
);
105113
}
106114

@@ -129,28 +137,28 @@ async function copyRunnerScreenshotFromDevice(
129137
remoteFileName: string,
130138
outPath: string,
131139
): Promise<void> {
140+
const deadline = Deadline.fromTimeoutMs(IOS_RUNNER_SCREENSHOT_COPY_TIMEOUT_MS);
132141
let copyResult = { exitCode: 1, stdout: '', stderr: '' };
133142
for (const bundleId of IOS_RUNNER_CONTAINER_BUNDLE_IDS) {
134-
copyResult = await runCmd(
135-
'xcrun',
136-
[
137-
'devicectl',
138-
'device',
139-
'copy',
140-
'from',
141-
'--device',
142-
device.id,
143-
'--source',
144-
remoteFileName,
145-
'--destination',
146-
outPath,
147-
'--domain-type',
148-
'appDataContainer',
149-
'--domain-identifier',
150-
bundleId,
151-
],
152-
{ allowFailure: true },
153-
);
143+
copyResult = await runCmd('xcrun', [
144+
'devicectl',
145+
'device',
146+
'copy',
147+
'from',
148+
'--device',
149+
device.id,
150+
'--source',
151+
remoteFileName,
152+
'--destination',
153+
outPath,
154+
'--domain-type',
155+
'appDataContainer',
156+
'--domain-identifier',
157+
bundleId,
158+
], {
159+
allowFailure: true,
160+
timeoutMs: resolveDeadlineTimeoutMs(deadline, IOS_RUNNER_SCREENSHOT_COPY_TIMEOUT_MS, 'runner screenshot copy'),
161+
});
154162
if (copyResult.exitCode === 0) {
155163
return;
156164
}
@@ -164,10 +172,16 @@ async function copyRunnerScreenshotFromSimulator(
164172
remoteFileName: string,
165173
outPath: string,
166174
): Promise<void> {
175+
const deadline = Deadline.fromTimeoutMs(IOS_RUNNER_SCREENSHOT_COPY_TIMEOUT_MS);
167176
let lastError = 'Unable to locate runner container for simulator screenshot';
168177
for (const bundleId of IOS_RUNNER_CONTAINER_BUNDLE_IDS) {
169178
const containerResult = await runSimctl(device, ['get_app_container', device.id, bundleId, 'data'], {
170179
allowFailure: true,
180+
timeoutMs: resolveDeadlineTimeoutMs(
181+
deadline,
182+
IOS_RUNNER_SCREENSHOT_COPY_TIMEOUT_MS,
183+
'runner screenshot container lookup',
184+
),
171185
});
172186
if (containerResult.exitCode !== 0) {
173187
const stderr = containerResult.stderr.trim();
@@ -194,6 +208,34 @@ async function copyRunnerScreenshotFromSimulator(
194208
throw new AppError('COMMAND_FAILED', `Failed to capture iOS screenshot: ${lastError}`);
195209
}
196210

211+
function resolveDeadlineTimeoutMs(deadline: Deadline, timeoutMs: number, step: string): number {
212+
const remainingMs = deadline.remainingMs();
213+
if (remainingMs > 0) return remainingMs;
214+
throw new AppError('COMMAND_FAILED', `iOS ${step} timed out after ${timeoutMs}ms`, {
215+
timeoutMs,
216+
step,
217+
});
218+
}
219+
220+
function emitScreenshotFallbackDiagnostic(
221+
device: DeviceInfo,
222+
from: 'simctl_screenshot' | 'devicectl_screenshot',
223+
to: 'runner',
224+
error: unknown,
225+
): void {
226+
emitDiagnostic({
227+
level: 'warn',
228+
phase: 'ios_screenshot_fallback',
229+
data: {
230+
platform: device.platform,
231+
deviceKind: device.kind,
232+
from,
233+
to,
234+
reason: error instanceof Error ? error.message : String(error),
235+
},
236+
});
237+
}
238+
197239
export function resolveSimulatorRunnerScreenshotCandidatePaths(containerPath: string, remoteFileName: string): string[] {
198240
const normalizedContainerPath = path.resolve(containerPath);
199241
const rawRemotePath = remoteFileName.trim();
@@ -253,12 +295,18 @@ export function shouldFallbackToRunnerForIosScreenshot(error: unknown): boolean
253295
export function shouldRetryIosSimulatorScreenshot(error: unknown): boolean {
254296
if (!(error instanceof AppError)) return false;
255297
if (error.code !== 'COMMAND_FAILED') return false;
256-
const details = (error.details ?? {}) as { stdout?: unknown; stderr?: unknown };
298+
const details = (error.details ?? {}) as { stdout?: unknown; stderr?: unknown; args?: unknown };
257299
const stdout = typeof details.stdout === 'string' ? details.stdout : '';
258300
const stderr = typeof details.stderr === 'string' ? details.stderr : '';
259-
const combined = `${error.message}\n${stdout}\n${stderr}`.toLowerCase();
301+
const args = Array.isArray(details.args)
302+
? details.args
303+
.filter((value): value is string => typeof value === 'string')
304+
.join(' ')
305+
: '';
306+
const combined = `${error.message}\n${stdout}\n${stderr}\n${args}`.toLowerCase();
260307
return (
261308
combined.includes('timeout waiting for screen surfaces') ||
262-
(combined.includes('nsposixerrordomain') && combined.includes('code=60') && combined.includes('screenshot'))
309+
(combined.includes('nsposixerrordomain') && combined.includes('code=60') && combined.includes('screenshot')) ||
310+
(combined.includes('timed out') && combined.includes('screenshot'))
263311
);
264312
}

0 commit comments

Comments
 (0)