Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions packages/provider-webdriver/src/webdriver-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,29 @@ test('installApp aborts an in-flight provider request when its binding is cancel
}
});

// #2509: a page-source read that outlived its request was dropped by the client
// while Appium kept walking the tree server-side. Providers serialize commands
// per session, so every later request queued behind an orphan nobody was
// waiting for. Cancellation must reach `/source` the way it reaches `/install_app`.
test('source aborts an in-flight provider request when its binding is cancelled', async () => {
const controller = new AbortController();
let requestSignal: AbortSignal | undefined;
const client = await connectedClient(
async (_input, init) =>
await new Promise<Response>((_resolve, reject) => {
requestSignal = init?.signal ?? undefined;
init?.signal?.addEventListener('abort', () => reject(init.signal?.reason as Error));
}),
);

const pending = client.source({ signal: controller.signal });
await Promise.resolve();
controller.abort(new Error('request cancelled'));

await assert.rejects(pending, /request cancelled/);
assert.equal(requestSignal?.aborted, true);
});

// #1774: `POST /session` is the one non-idempotent request, and a retry after a
// failed create is a second billed device session. A 5xx that the transport
// would retry on any other route must reach the caller unretried here.
Expand Down
10 changes: 8 additions & 2 deletions packages/provider-webdriver/src/webdriver-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,14 @@ export class WebDriverClient {
await this.sessionRequest('POST', '/orientation', { orientation });
}

async source(): Promise<string> {
const value = await this.sessionRequest('GET', '/source');
/**
* The driver's whole page source. This is the provider's most expensive call and
* the only one whose duration the device's own UI decides — a screen that never
* goes idle has no reason to settle — so a caller that can be cancelled should
* hand its `signal` down rather than abandon a read still running server-side.
*/
async source(overrides?: WebDriverRequestOverrides): Promise<string> {
const value = await this.sessionRequest('GET', '/source', undefined, overrides);
if (typeof value !== 'string') {
throw new AppError('COMMAND_FAILED', 'WebDriver source response was not a string', {
valueType: typeof value,
Expand Down
81 changes: 81 additions & 0 deletions packages/provider-webdriver/src/webdriver-interactor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { AppError } from '@agent-device/kernel/errors';
import { createCloudWebDriverCapabilities } from './capabilities.ts';
import type { WebDriverClient, W3CActionSequence } from './webdriver-client.ts';
import { createWebDriverInteractor } from './webdriver-interactor.ts';
import { isWebDriverRequestTimeout } from './webdriver-transport.ts';

// #1658: `fill` used to send its keys in the request right after the tap. A
// WebView input takes first responder asynchronously, so on a web login form
Expand Down Expand Up @@ -291,6 +292,86 @@ test('Android WebDriver interactor keeps legacy-derived source facts at its call
assert.equal(source.mock.calls.length, 1);
});

// #2509: the interactor took a request-bound signal and named it away. A capture
// that ran past its budget could therefore never be cancelled: the client gave up
// while the provider kept walking the tree, and being per-session-serial it made
// every later command queue behind an orphan nobody was waiting for.
test('Android snapshot binds the provider source read to its request signal', async () => {
const controller = new AbortController();
const forwarded: Array<{ signal?: AbortSignal } | undefined> = [];
const interactor = createWebDriverInteractor({
client: {
source: async (overrides?: { signal?: AbortSignal }) => {
forwarded.push(overrides);
return ANDROID_ONBOARDING_SOURCE;
},
} as unknown as WebDriverClient,
backend: 'android',
capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'android' }),
});

await interactor.snapshot({ signal: controller.signal });

assert.deepEqual(forwarded, [{ signal: controller.signal }]);
});

// The iOS acquisition adapter reads the same route, so it needs the same binding.
test('iOS snapshot binds the provider source read to its request signal', async () => {
const controller = new AbortController();
const forwarded: Array<{ signal?: AbortSignal } | undefined> = [];
const interactor = createWebDriverInteractor({
client: {
source: async (overrides?: { signal?: AbortSignal }) => {
forwarded.push(overrides);
return '<AppiumAUT><XCUIElementTypeApplication x="0" y="0" width="390" height="844" /></AppiumAUT>';
},
} as unknown as WebDriverClient,
backend: 'xctest',
capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'ios' }),
targetId: 'ios-1',
});

await interactor.snapshot({ signal: controller.signal });

assert.deepEqual(forwarded, [{ signal: controller.signal }]);
});

// #2509 asked for an error that names the problem: a screen that never goes idle
// (looping video, live marquee) keeps the provider's tree walk from settling, and
// on rented hardware every second of it is billed. The reason code stays the
// transport's; what the capture adds is what it means.
test('a source capture that runs out of budget keeps the timeout reason and names the cause', async () => {
const interactor = createWebDriverInteractor({
client: { source: async () => throwWebDriverSourceTimeout() } as unknown as WebDriverClient,
backend: 'android',
capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'android' }),
});

await assert.rejects(interactor.snapshot(), (error: unknown) => {
assert.ok(error instanceof AppError);
assert.equal(error.details?.reason, 'webdriver_request_timeout');
assert.equal(isWebDriverRequestTimeout(error), true);
assert.match(String(error.details?.hint), /never goes idle/);
// Reviewing #2509 found the advice that failed there was a longer `--timeout`,
// which cannot reach this read. The hint says so and offers what does work.
assert.match(String(error.details?.hint), /does not grow with --timeout/);
assert.match(String(error.details?.hint), /screenshot/);
return true;
});
});

function throwWebDriverSourceTimeout(): never {
throw new AppError('COMMAND_FAILED', 'WebDriver GET /source timed out after 30000ms.', {
reason: 'webdriver_request_timeout',
method: 'GET',
path: '/source',
timeoutMs: 30_000,
});
}

const ANDROID_ONBOARDING_SOURCE =
'<hierarchy rotation="0"><android.widget.Button content-desc="Continue" bounds="[0,0][100,40]" displayed="true" enabled="true" /></hierarchy>';

async function runFill(world: ReturnType<typeof createTextEntryWorld>) {
vi.useFakeTimers();
try {
Expand Down
54 changes: 50 additions & 4 deletions packages/provider-webdriver/src/webdriver-interactor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
type CloudWebDriverProviderCapabilities,
} from './capabilities.ts';
import type { W3CPointerAction, WebDriverClient, WebDriverWindowRect } from './webdriver-client.ts';
import { isWebDriverRequestTimeout } from './webdriver-transport.ts';
import { touchPointer } from './webdriver-gestures.ts';
import {
scrollFrameFromAndroidWebDriverSource,
Expand Down Expand Up @@ -303,20 +304,39 @@ class WebDriverInteractor implements Interactor {
await this.client.screenshot(outPath);
}

async snapshot(_options?: SnapshotOptions) {
async snapshot(options?: SnapshotOptions) {
this.requireSupport('snapshot');
return await this.captureSource(options?.signal);
}

/**
* One page-source read, bound to the request that asked for it. Providers answer
* commands one at a time, so a read the client merely gave up on is not gone: the
* driver keeps walking the tree and every later command queues behind it (#2509).
*/
private async captureSource(signal?: AbortSignal) {
const source = await this.readSource(signal);
if (this.backend === 'xctest') {
const { captureWebDriverIosSnapshot } = await import('./webdriver-ios-snapshot.ts');
return await captureWebDriverIosSnapshot(this.client, this.targetId);
const { acquireWebDriverIosSnapshot } = await import('./webdriver-ios-snapshot.ts');
return acquireWebDriverIosSnapshot(source, this.targetId);
}
const { parseWebDriverSourceFacts } = await import('./webdriver-source.ts');
return {
backend: 'android' as const,
producer: 'appium-source' as const,
nodes: parseWebDriverSourceFacts(await this.client.source(), 'android').nodes,
nodes: parseWebDriverSourceFacts(source, 'android').nodes,
};
}

private async readSource(signal?: AbortSignal): Promise<string> {
try {
return await this.client.source(signal === undefined ? {} : { signal });
} catch (error) {
if (!isWebDriverRequestTimeout(error)) throw error;
throw webDriverSourceTimeoutError(error);
}
}

async back(_mode?: BackMode): Promise<void> {
this.requireSupport('back');
await this.client.back();
Expand Down Expand Up @@ -573,3 +593,29 @@ function webDriverOperationForGesture(plan: GesturePlan): CloudWebDriverOperatio
return 'rotateGesture';
}
}

/**
* A source read that ran out of budget is the one WebDriver timeout a caller can
* do something about, and #2509 showed it reading as an unexplained hang: the
* driver answers this call by walking the live UI tree, so a screen that never
* goes idle — looping video, live ticker, continuous animation — gives the walk no
* reason to settle. The transport's reason code is kept as-is; what the capture
* adds is what the wait was waiting for, and the one thing the caller cannot do,
* since this read's budget is the transport's own and no wider than the command's
* `--timeout` envelope around it.
*/
function webDriverSourceTimeoutError(error: AppError): AppError {
return new AppError(
'COMMAND_FAILED',
'The cloud driver did not finish reading the screen in its budget.',
{
...error.details,
hint:
'A screen that never goes idle (looping video, live ticker, continuous animation) ' +
"gives the driver no moment to read the UI tree. This read has the transport's own " +
'budget and does not grow with --timeout; take a screenshot, or drive the screen from ' +
'refs an earlier snapshot already captured.',
},
error,
);
}
13 changes: 4 additions & 9 deletions packages/provider-webdriver/src/webdriver-ios-snapshot.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import assert from 'node:assert/strict';
import { test, vi } from 'vitest';
import {
captureWebDriverIosSnapshot,
acquireWebDriverIosSnapshot,
} from './webdriver-ios-snapshot.ts';
import { test } from 'vitest';
import { acquireWebDriverIosSnapshot } from './webdriver-ios-snapshot.ts';

const SOURCE = `<AppiumAUT>
<XCUIElementTypeApplication type="XCUIElementTypeApplication" name="Example" label="Example" enabled="true" visible="true" x="0" y="0" width="390" height="844">
Expand All @@ -13,9 +10,8 @@ const SOURCE = `<AppiumAUT>
</XCUIElementTypeApplication>
</AppiumAUT>`;

test('Appium iOS adapter returns provider facts with explicit unavailable residue', async () => {
const source = vi.fn(async () => SOURCE);
const result = await captureWebDriverIosSnapshot({ source }, 'cloud-ios-1');
test('Appium iOS adapter returns provider facts with explicit unavailable residue', () => {
const result = acquireWebDriverIosSnapshot(SOURCE, 'cloud-ios-1');

assert.equal(result.stage, 'acquired');
assert.equal(result.acquisition.producer, 'appium-source');
Expand All @@ -34,7 +30,6 @@ test('Appium iOS adapter returns provider facts with explicit unavailable residu
{ kind: 'unavailable-fact', fact: 'acquisition-depth' },
{ kind: 'unavailable-fact', fact: 'truncation' },
]);
assert.equal(source.mock.calls.length, 1);
});

test('Appium iOS adapter preserves provider-reported node facts for the host presenter', () => {
Expand Down
9 changes: 1 addition & 8 deletions packages/provider-webdriver/src/webdriver-ios-snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,11 @@ import {
resolveIosViewportEvidenceFromRoots,
} from '@agent-device/capture-kit/ios-snapshot-acquisition';
import type { SnapshotRuntimeAcquiredResult } from '@agent-device/contracts/interactor-types';
import type { WebDriverClient } from './webdriver-client.ts';
import { parseWebDriverSourceFacts } from './webdriver-source.ts';

const APPIUM_PRODUCER = 'appium-source' as const;

export async function captureWebDriverIosSnapshot(
client: Pick<WebDriverClient, 'source'>,
targetId?: string,
): Promise<SnapshotRuntimeAcquiredResult> {
return acquireWebDriverIosSnapshot(await client.source(), targetId);
}

/** Turns one already-read Appium page source into acquired iOS facts. */
export function acquireWebDriverIosSnapshot(
source: string,
targetId?: string,
Expand Down
Loading
Loading