From b46797fc8ec5f816a8af3d8b35fb832d8a9c0649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 28 Sep 2026 12:45:55 +0200 Subject: [PATCH 01/13] test(provider-webdriver): prove mutating WebDriver routes get resent today A tap, a key send, and a back navigation each resend their mutating request when it times out, because the transport's default retry policy treats a timeout as retriable for every route alike, mutations included. Characterizes the bug through the real WebDriverClient/interactor before it is fixed. --- .../src/webdriver-mutation-resend.test.ts | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 packages/provider-webdriver/src/webdriver-mutation-resend.test.ts diff --git a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts new file mode 100644 index 0000000000..b8bad81057 --- /dev/null +++ b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts @@ -0,0 +1,103 @@ +import assert from 'node:assert/strict'; +import { afterEach, test } from 'vitest'; +import { createCloudWebDriverCapabilities } from './capabilities.ts'; +import { WebDriverClient } from './webdriver-client.ts'; +import { createWebDriverInteractor } from './webdriver-interactor.ts'; + +const realFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = realFetch; +}); + +/** + * A grid that answers `POST /session` immediately, then never answers the one + * configured mutating route until the transport's own request timeout aborts + * it. Every other route answers immediately with an empty value. This is the + * shape a hung `POST .../actions` or `.../keys` takes on a real provider: the + * driver received the request and is still working it, not a connection + * failure. + */ +function hangingWebDriverFetch(hangOnPathSuffix: string): { + fetch: typeof globalThis.fetch; + callsFor: (pathSuffix: string) => number; +} { + const paths: string[] = []; + const fetch: typeof globalThis.fetch = async (input, init) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + paths.push(url.pathname); + if (url.pathname.endsWith('/session')) { + return new Response(JSON.stringify({ value: { sessionId: 'wd-1', capabilities: {} } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + if (url.pathname.endsWith(hangOnPathSuffix)) { + return await new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(init.signal?.reason as Error)); + }); + } + return new Response(JSON.stringify({ value: null }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }; + return { fetch, callsFor: (pathSuffix) => paths.filter((p) => p.endsWith(pathSuffix)).length }; +} + +/** + * A real `WebDriverClient` and `createWebDriverInteractor`, connected through + * a driver that hangs on `hangOnPathSuffix`. `timeoutMs` is the smallest the + * transport accepts that still lets `AbortSignal.timeout` and the fetch stub's + * abort listener race deterministically; `retryDelayMs` is cut to keep a + * retried attempt's sleep out of the test budget. + */ +async function connectedWebDriverInteractor(hangOnPathSuffix: string) { + const { fetch, callsFor } = hangingWebDriverFetch(hangOnPathSuffix); + globalThis.fetch = fetch; + const client = new WebDriverClient({ + clientVersion: '0.0.0-test', + endpoint: 'http://cloud-webdriver.test/wd/hub/', + requestPolicy: { timeoutMs: 20, retryDelayMs: 5 }, + }); + await client.createSession({ platformName: 'Android' }); + const interactor = createWebDriverInteractor({ + client, + backend: 'android', + capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'android' }), + }); + return { interactor, callsFor }; +} + +// A tap whose `POST .../actions` request times out is RESENT today: the +// transport's default retry policy (timeout is retriable, one retry) applies +// to every WebDriver route alike, so a mutation gets the same second attempt +// as a safe read. That second attempt can double the touch the driver is +// still processing from the first. +test('a timed-out tap resends POST .../actions today', async () => { + const { interactor, callsFor } = await connectedWebDriverInteractor('/actions'); + + await assert.rejects(interactor.tap(10, 20), () => true); + + assert.equal(callsFor('/actions'), 2); +}); + +// Same shape for text entry: a hung `POST .../keys` is resent, risking a +// doubled key stream if the first attempt's keys were already delivered. +test('timed-out keys resend POST .../keys today', async () => { + const { interactor, callsFor } = await connectedWebDriverInteractor('/keys'); + + await assert.rejects(interactor.type('hello'), () => true); + + assert.equal(callsFor('/keys'), 2); +}); + +// A third mutating route, distinct from the gesture/text-entry paths: a +// hung `POST .../back` is resent too, risking a doubled back navigation. +test('a timed-out back resends POST .../back today', async () => { + const { interactor, callsFor } = await connectedWebDriverInteractor('/back'); + + await assert.rejects(interactor.back(), () => true); + + assert.equal(callsFor('/back'), 2); +}); From 73013ae094966cbf9ef4d50e09009be51d2be763 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 28 Sep 2026 12:48:49 +0200 Subject: [PATCH 02/13] fix(provider-webdriver): never resend a mutating WebDriver request Every mutating route (actions, keys, back, orientation/rotation, app lifecycle, install, delete-session, and the mutating mobile: scripts) now gets exactly one attempt via a named MUTATION_REQUEST_POLICY, instead of inheriting the transport's default one-retry budget meant for idempotent reads. A resend after a timeout or a 5xx cannot tell whether the first attempt's side effect already landed, so retrying risked a doubled tap, key send, or navigation. The transport also now classifies every failed request's outcome via details.dispatched: 'unknown' for a timeout or a >=500 answer (the driver may have started or finished the mutation), and 'no' for a connection refusal before any bytes were sent. This is the vocabulary feat/dispatch-disclosure formalizes as DispatchDisclosure in @agent-device/contracts; the mutation policy is why a mutation never gets the chance to act on either value. The characterization test added in the previous commit is flipped into the regression guard: one request per mutation, dispatched asserted, and a read route kept retried once to prove the policy is narrowly scoped. --- .../src/webdriver-client.ts | 85 +++++++++++++++---- .../src/webdriver-interactor.ts | 28 ++++-- .../src/webdriver-mutation-resend.test.ts | 62 ++++++++++---- .../src/webdriver-transport.test.ts | 77 ++++++++++++++++- .../src/webdriver-transport.ts | 64 +++++++++++++- 5 files changed, 271 insertions(+), 45 deletions(-) diff --git a/packages/provider-webdriver/src/webdriver-client.ts b/packages/provider-webdriver/src/webdriver-client.ts index b9cc59d87a..be937e11c1 100644 --- a/packages/provider-webdriver/src/webdriver-client.ts +++ b/packages/provider-webdriver/src/webdriver-client.ts @@ -1,6 +1,7 @@ import fs from 'node:fs/promises'; import { AppError } from '@agent-device/kernel/errors'; import { + MUTATION_REQUEST_POLICY, WebDriverTransport, type WebDriverAuth, type WebDriverRequestOverrides, @@ -105,46 +106,80 @@ export class WebDriverClient { // fallow-ignore-next-line unused-class-member async deleteSession(): Promise { const sessionId = this.requireSessionId(); - await this.requestValue('DELETE', `/session/${sessionId}`); + await this.requestValue('DELETE', `/session/${sessionId}`, undefined, MUTATION_REQUEST_POLICY); this.sessionId = undefined; } async installApp(appPath: string, signal?: AbortSignal): Promise { - await this.sessionRequest('POST', '/appium/device/install_app', { appPath }, { signal }); + await this.sessionRequest( + 'POST', + '/appium/device/install_app', + { appPath }, + { + ...MUTATION_REQUEST_POLICY, + signal, + }, + ); } async activateApp(appId: string): Promise { try { - await this.sessionRequest('POST', '/appium/device/activate_app', { appId }); + await this.sessionRequest( + 'POST', + '/appium/device/activate_app', + { appId }, + MUTATION_REQUEST_POLICY, + ); } catch { - await this.executeScript('mobile: activateApp', [{ appId, bundleId: appId }]); + await this.executeScript( + 'mobile: activateApp', + [{ appId, bundleId: appId }], + MUTATION_REQUEST_POLICY, + ); } } async terminateApp(appId: string): Promise { try { - await this.sessionRequest('POST', '/appium/device/terminate_app', { appId }); + await this.sessionRequest( + 'POST', + '/appium/device/terminate_app', + { appId }, + MUTATION_REQUEST_POLICY, + ); } catch { - await this.executeScript('mobile: terminateApp', [{ appId, bundleId: appId }]); + await this.executeScript( + 'mobile: terminateApp', + [{ appId, bundleId: appId }], + MUTATION_REQUEST_POLICY, + ); } } async performActions(actions: W3CActionSequence[]): Promise { - await this.sessionRequest('POST', '/actions', { actions }); + await this.sessionRequest('POST', '/actions', { actions }, MUTATION_REQUEST_POLICY); } async releaseActions(): Promise { - await this.sessionRequest('DELETE', '/actions', undefined, { retryAttempts: 0 }); + await this.sessionRequest('DELETE', '/actions', undefined, MUTATION_REQUEST_POLICY); } async sendKeys(text: string): Promise { - await this.sessionRequest('POST', '/keys', { value: Array.from(text) }); + await this.sessionRequest( + 'POST', + '/keys', + { value: Array.from(text) }, + MUTATION_REQUEST_POLICY, + ); } async hideKeyboard(): Promise { - await this.sessionRequest('POST', '/appium/device/hide_keyboard', undefined, { - retryAttempts: 0, - }); + await this.sessionRequest( + 'POST', + '/appium/device/hide_keyboard', + undefined, + MUTATION_REQUEST_POLICY, + ); } /** @@ -231,17 +266,22 @@ export class WebDriverClient { } async back(): Promise { - await this.sessionRequest('POST', '/back'); + await this.sessionRequest('POST', '/back', undefined, MUTATION_REQUEST_POLICY); } /** Exact four-way display rotation. `z` is degrees: 0, 90, 180, or 270. */ async setRotation(degrees: number): Promise { - await this.sessionRequest('POST', '/rotation', { x: 0, y: 0, z: degrees }); + await this.sessionRequest( + 'POST', + '/rotation', + { x: 0, y: 0, z: degrees }, + MUTATION_REQUEST_POLICY, + ); } /** Two-way orientation. Values are uppercase per the WebDriver protocol. */ async setOrientation(orientation: 'PORTRAIT' | 'LANDSCAPE'): Promise { - await this.sessionRequest('POST', '/orientation', { orientation }); + await this.sessionRequest('POST', '/orientation', { orientation }, MUTATION_REQUEST_POLICY); } /** @@ -274,8 +314,19 @@ export class WebDriverClient { return readWindowRect(await this.sessionRequest('GET', '/window/rect')); } - async executeScript(script: string, args: unknown[] = []): Promise { - return await this.sessionRequest('POST', '/execute/sync', { script, args }); + /** + * `mobile:` scripts cover both reads (`getClipboard`) and mutations + * (`activateApp`, `deepLink`, `pressButton`, `setClipboard`, …) behind one + * generic route, so the caller — which knows what the script does — states + * the retry policy; omitting `overrides` keeps the transport default for a + * read. + */ + async executeScript( + script: string, + args: unknown[] = [], + overrides?: WebDriverRequestOverrides, + ): Promise { + return await this.sessionRequest('POST', '/execute/sync', { script, args }, overrides); } private async sessionRequest( diff --git a/packages/provider-webdriver/src/webdriver-interactor.ts b/packages/provider-webdriver/src/webdriver-interactor.ts index 6b677904c9..af352dfce4 100644 --- a/packages/provider-webdriver/src/webdriver-interactor.ts +++ b/packages/provider-webdriver/src/webdriver-interactor.ts @@ -22,7 +22,7 @@ import { type CloudWebDriverProviderCapabilities, } from './capabilities.ts'; import type { W3CPointerAction, WebDriverClient, WebDriverWindowRect } from './webdriver-client.ts'; -import { isWebDriverRequestTimeout } from './webdriver-transport.ts'; +import { isWebDriverRequestTimeout, MUTATION_REQUEST_POLICY } from './webdriver-transport.ts'; import { touchPointer } from './webdriver-gestures.ts'; import { scrollFrameFromAndroidWebDriverSource, @@ -143,7 +143,11 @@ class WebDriverInteractor implements Interactor { }, ): Promise { if (options?.url) { - await this.client.executeScript('mobile: deepLink', [{ url: options.url, package: app }]); + await this.client.executeScript( + 'mobile: deepLink', + [{ url: options.url, package: app }], + MUTATION_REQUEST_POLICY, + ); return; } const appId = options?.appBundleId ?? app; @@ -152,7 +156,7 @@ class WebDriverInteractor implements Interactor { } async openDevice(): Promise { - await this.client.executeScript('mobile: activateApp', [{}]); + await this.client.executeScript('mobile: activateApp', [{}], MUTATION_REQUEST_POLICY); } async close(app: string): Promise { @@ -344,7 +348,11 @@ class WebDriverInteractor implements Interactor { async home(): Promise { this.requireSupport('home'); - await this.client.executeScript('mobile: pressButton', [{ name: 'home' }]); + await this.client.executeScript( + 'mobile: pressButton', + [{ name: 'home' }], + MUTATION_REQUEST_POLICY, + ); } async setOrientation(orientation: DeviceRotation): Promise { @@ -354,7 +362,11 @@ class WebDriverInteractor implements Interactor { async appSwitcher(): Promise { this.requireSupport('appSwitcher'); - await this.client.executeScript('mobile: pressButton', [{ name: 'appSwitch' }]); + await this.client.executeScript( + 'mobile: pressButton', + [{ name: 'appSwitch' }], + MUTATION_REQUEST_POLICY, + ); } async tvRemote(_button: TvRemoteButton, _durationMs?: number): Promise { @@ -369,7 +381,11 @@ class WebDriverInteractor implements Interactor { async writeClipboard(text: string): Promise { this.requireSupport('clipboard.write'); - await this.client.executeScript('mobile: setClipboard', [{ content: text }]); + await this.client.executeScript( + 'mobile: setClipboard', + [{ content: text }], + MUTATION_REQUEST_POLICY, + ); } async setSetting( diff --git a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts index b8bad81057..d81eefbbf9 100644 --- a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts +++ b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts @@ -1,5 +1,6 @@ import assert from 'node:assert/strict'; import { afterEach, test } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; import { createCloudWebDriverCapabilities } from './capabilities.ts'; import { WebDriverClient } from './webdriver-client.ts'; import { createWebDriverInteractor } from './webdriver-interactor.ts'; @@ -69,35 +70,60 @@ async function connectedWebDriverInteractor(hangOnPathSuffix: string) { return { interactor, callsFor }; } -// A tap whose `POST .../actions` request times out is RESENT today: the -// transport's default retry policy (timeout is retriable, one retry) applies -// to every WebDriver route alike, so a mutation gets the same second attempt -// as a safe read. That second attempt can double the touch the driver is -// still processing from the first. -test('a timed-out tap resends POST .../actions today', async () => { +// A tap whose `POST .../actions` request times out gets exactly one attempt: +// a resend cannot tell whether the touch the driver is still processing from +// the first attempt already landed, so a second attempt risks a doubled +// gesture instead of a safe no-op. The thrown error still discloses that the +// outcome is unresolved via `details.dispatched`. +test('a timed-out tap is never resent', async () => { const { interactor, callsFor } = await connectedWebDriverInteractor('/actions'); - await assert.rejects(interactor.tap(10, 20), () => true); + await assert.rejects(interactor.tap(10, 20), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }); - assert.equal(callsFor('/actions'), 2); + assert.equal(callsFor('/actions'), 1); }); -// Same shape for text entry: a hung `POST .../keys` is resent, risking a -// doubled key stream if the first attempt's keys were already delivered. -test('timed-out keys resend POST .../keys today', async () => { +// Same shape for text entry: a hung `POST .../keys` must not be resent, or a +// doubled key stream could reach the field the first attempt already typed +// into. +test('timed-out keys are never resent', async () => { const { interactor, callsFor } = await connectedWebDriverInteractor('/keys'); - await assert.rejects(interactor.type('hello'), () => true); + await assert.rejects(interactor.type('hello'), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }); - assert.equal(callsFor('/keys'), 2); + assert.equal(callsFor('/keys'), 1); }); -// A third mutating route, distinct from the gesture/text-entry paths: a -// hung `POST .../back` is resent too, risking a doubled back navigation. -test('a timed-out back resends POST .../back today', async () => { +// A third mutating route, distinct from the gesture/text-entry paths: a hung +// `POST .../back` must not be resent, or a doubled back navigation could +// leave the app a screen further back than the caller asked for. +test('a timed-out back is never resent', async () => { const { interactor, callsFor } = await connectedWebDriverInteractor('/back'); - await assert.rejects(interactor.back(), () => true); + await assert.rejects(interactor.back(), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }); + + assert.equal(callsFor('/back'), 1); +}); + +// The mutation policy is narrowly scoped: a read route (page source) keeps +// the transport's default retry budget, so a timeout there still resends +// once, exactly as it did before this change. +test('a timed-out read is still retried once', async () => { + const { interactor, callsFor } = await connectedWebDriverInteractor('/source'); + + await assert.rejects(interactor.snapshot(), () => true); - assert.equal(callsFor('/back'), 2); + assert.equal(callsFor('/source'), 2); }); diff --git a/packages/provider-webdriver/src/webdriver-transport.test.ts b/packages/provider-webdriver/src/webdriver-transport.test.ts index 7d85b57c37..3f1b63fa79 100644 --- a/packages/provider-webdriver/src/webdriver-transport.test.ts +++ b/packages/provider-webdriver/src/webdriver-transport.test.ts @@ -1,7 +1,12 @@ import assert from 'node:assert/strict'; import { afterEach, test } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; -import { WebDriverTransport, isWebDriverRequestTimeout } from './webdriver-transport.ts'; +import { + MUTATION_REQUEST_POLICY, + WebDriverTransport, + isWebDriverConnectRefused, + isWebDriverRequestTimeout, +} from './webdriver-transport.ts'; const realFetch = globalThis.fetch; @@ -94,3 +99,73 @@ test('cancels a retry delay when the request binding aborts', async () => { await assert.rejects(pending, /abort|cancel/i); assert.equal(calls, 1); }); + +// Fetch's own network-error shape (refused connection, unresolved host) is a +// bare TypeError with no HTTP status. It must still classify as retriable — +// preserving today's behavior for a read — and disclose that no bytes of the +// request ever reached the driver. +test('a connection refusal classifies as unreached and is still retried by default', async () => { + const transport = new WebDriverTransport({ + clientVersion: '0.0.0-test', + endpoint: 'http://cloud-webdriver.test/wd/hub/', + requestPolicy: { timeoutMs: 30_000, retryDelayMs: 1 }, + }); + let calls = 0; + globalThis.fetch = async () => { + calls += 1; + throw new TypeError('fetch failed'); + }; + + await assert.rejects(transport.requestValue('GET', '/session/wd-1/source'), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.ok(isWebDriverConnectRefused(error)); + assert.equal(error.details?.dispatched, 'no'); + return true; + }); + assert.equal(calls, 2); +}); + +// The one attempt `MUTATION_REQUEST_POLICY` grants must hold regardless of +// which ambiguous failure the request hit — connect refusal included, not +// only a timeout or a 5xx. +test('a connection refusal is not resent under the mutation policy', async () => { + const transport = new WebDriverTransport({ + clientVersion: '0.0.0-test', + endpoint: 'http://cloud-webdriver.test/wd/hub/', + requestPolicy: { timeoutMs: 30_000, retryDelayMs: 1 }, + }); + let calls = 0; + globalThis.fetch = async () => { + calls += 1; + throw new TypeError('fetch failed'); + }; + + await assert.rejects( + transport.requestValue('POST', '/session/wd-1/actions', {}, MUTATION_REQUEST_POLICY), + ); + assert.equal(calls, 1); +}); + +// A 5xx means the driver answered — it received and processed the request — +// but not whether the mutation it described completed before it failed. +test('a 5xx response discloses an unresolved outcome', async () => { + const transport = new WebDriverTransport({ + clientVersion: '0.0.0-test', + endpoint: 'http://cloud-webdriver.test/wd/hub/', + requestPolicy: { timeoutMs: 30_000, retryAttempts: 0 }, + }); + globalThis.fetch = async () => + new Response(JSON.stringify({ value: { message: 'grid unavailable' } }), { + status: 503, + headers: { 'Content-Type': 'application/json' }, + }); + + await assert.rejects( + transport.requestValue('POST', '/session/wd-1/actions', {}), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }, + ); +}); diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index 735592b34e..95735ee341 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -29,6 +29,14 @@ export function isWebDriverRequestTimeout(error: unknown): error is AppError { return error instanceof AppError && error.details?.reason === WEBDRIVER_REQUEST_TIMEOUT_REASON; } +/** Machine-readable `details.reason` of a request that never left the client. */ +const WEBDRIVER_CONNECT_REFUSED_REASON = 'webdriver_connect_refused'; + +/** A request that failed before any bytes reached the driver (refused/unreachable host). */ +export function isWebDriverConnectRefused(error: unknown): error is AppError { + return error instanceof AppError && error.details?.reason === WEBDRIVER_CONNECT_REFUSED_REASON; +} + export type WebDriverRequestOverrides = { retryAttempts?: number; /** @@ -41,6 +49,21 @@ export type WebDriverRequestOverrides = { signal?: AbortSignal; }; +/** + * Every mutating WebDriver route gets exactly one attempt. A resend after an + * ambiguous outcome (the request timed out, or the driver answered 5xx after + * receiving it) cannot tell whether the first attempt's side effect — a tap, a + * key send, a navigation — already landed, so retrying risks a doubled action + * rather than a safe no-op. Reads keep the transport's default retry budget. + * + * `'no'` and `'unknown'` are the two values this transport can attach to a + * failed request's `details.dispatched`; `feat/dispatch-disclosure` formalizes + * that vocabulary as `DispatchDisclosure` (`'no' | 'yes' | 'unknown'`) in + * `@agent-device/contracts`. This policy is why a mutation never gets the + * chance to resend regardless of which value it carries. + */ +export const MUTATION_REQUEST_POLICY: WebDriverRequestOverrides = { retryAttempts: 0 }; + export type WebDriverTransportOptions = { clientVersion: string; endpoint: string | URL; @@ -174,6 +197,12 @@ export class WebDriverTransport { if (timeoutSignal.aborted && !requestSignal?.aborted) { throw webdriverTimeoutError(method, path, timeoutMs, error); } + // Fetch rejects with a bare TypeError for a network failure — refused + // connection, unresolved host, failed handshake — that never put a byte + // on the wire. A caller-driven abort keeps its own reason instead. + if (!requestSignal?.aborted && error instanceof TypeError) { + throw webdriverConnectRefusedError(method, path, error); + } throw error; } } @@ -214,7 +243,13 @@ function webdriverError(status: number, payload: unknown): AppError { typeof (value as { message?: unknown }).message === 'string' ? (value as { message: string }).message : `WebDriver request failed with HTTP ${status}.`; - return new AppError('COMMAND_FAILED', message, { status, response: payload }); + return new AppError('COMMAND_FAILED', message, { + status, + response: payload, + // A 5xx means the driver received and processed the request, but not + // whether the mutation it described completed before it failed. + ...(status >= 500 ? { dispatched: 'unknown' as const } : {}), + }); } function webdriverTimeoutError( @@ -226,16 +261,39 @@ function webdriverTimeoutError( return new AppError( 'COMMAND_FAILED', `WebDriver ${method} ${path} timed out after ${timeoutMs}ms.`, - { reason: WEBDRIVER_REQUEST_TIMEOUT_REASON, method, path, timeoutMs }, + { + reason: WEBDRIVER_REQUEST_TIMEOUT_REASON, + method, + path, + timeoutMs, + // The transport stopped waiting; the driver may still be mid-request. + dispatched: 'unknown' as const, + }, cause instanceof Error ? cause : undefined, ); } +function webdriverConnectRefusedError(method: string, path: string, cause: TypeError): AppError { + return new AppError( + 'COMMAND_FAILED', + `WebDriver ${method} ${path} could not reach the driver.`, + { + reason: WEBDRIVER_CONNECT_REFUSED_REASON, + method, + path, + // The connection itself failed, so no bytes of this request were sent. + dispatched: 'no' as const, + }, + cause, + ); +} + function isRetriableWebDriverError(error: unknown): boolean { if (isWebDriverRequestTimeout(error)) return true; + if (isWebDriverConnectRefused(error)) return true; if (error instanceof AppError) { const status = error.details?.status; return typeof status === 'number' && status >= 500; } - return error instanceof TypeError; + return false; } From e05d54308418e6e01debb501805a9d6e098f2aa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 28 Sep 2026 12:50:59 +0200 Subject: [PATCH 03/13] refactor(provider-webdriver): extract fetch-failure classification fetchWebDriver's catch block tripped the complexity gate once it grew a second failure branch. Pull the classification into its own function; same behavior, lower cyclomatic/cognitive cost on the request path. --- .../src/webdriver-transport.ts | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index 95735ee341..44034fc091 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -191,23 +191,43 @@ export class WebDriverTransport { }); return { ok: response.ok, status: response.status, text: await response.text() }; } catch (error) { - // The caller's own cancellation keeps its reason; only the transport's - // deadline becomes a typed timeout, so callers key on `details.reason` - // instead of sniffing fetch's DOMException name. - if (timeoutSignal.aborted && !requestSignal?.aborted) { - throw webdriverTimeoutError(method, path, timeoutMs, error); - } - // Fetch rejects with a bare TypeError for a network failure — refused - // connection, unresolved host, failed handshake — that never put a byte - // on the wire. A caller-driven abort keeps its own reason instead. - if (!requestSignal?.aborted && error instanceof TypeError) { - throw webdriverConnectRefusedError(method, path, error); - } - throw error; + throw classifyWebDriverFetchFailure(error, { + method, + path, + timeoutMs, + timeoutSignal, + requestSignal, + }); } } } +/** + * A caller's own cancellation keeps its reason as-is. Only a failure the + * transport itself can explain — its deadline, or a network failure fetch + * reports as a bare TypeError — becomes a typed error, so callers key on + * `details.reason` instead of sniffing fetch's DOMException name. + */ +function classifyWebDriverFetchFailure( + error: unknown, + context: { + method: string; + path: string; + timeoutMs: number; + timeoutSignal: AbortSignal; + requestSignal: AbortSignal | undefined; + }, +): unknown { + const { method, path, timeoutMs, timeoutSignal, requestSignal } = context; + if (requestSignal?.aborted) return error; + if (timeoutSignal.aborted) return webdriverTimeoutError(method, path, timeoutMs, error); + // Fetch rejects with a bare TypeError for a network failure — refused + // connection, unresolved host, failed handshake — that never put a byte on + // the wire. + if (error instanceof TypeError) return webdriverConnectRefusedError(method, path, error); + return error; +} + function shouldRetryWebDriverRequest( error: unknown, attempt: number, From 5b3e61cc1f5b0d22ea3064ecb4031b028013eb63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 29 Sep 2026 13:02:05 +0200 Subject: [PATCH 04/13] fix(provider-webdriver): fall back to the sibling app route only when the first is unsupported activateApp and terminateApp tried their mobile: sibling route after any failure of the Appium route, so a timeout or 5xx could send the same mutation twice. A driver's unsupported-route answer (404, 405, or the W3C unknown command/method codes) now stamps dispatched: no and is the only failure that switches routes. --- .../src/webdriver-client.ts | 7 ++- .../src/webdriver-mutation-resend.test.ts | 50 +++++++++++++++++-- .../src/webdriver-transport.ts | 31 ++++++++++++ 3 files changed, 82 insertions(+), 6 deletions(-) diff --git a/packages/provider-webdriver/src/webdriver-client.ts b/packages/provider-webdriver/src/webdriver-client.ts index be937e11c1..8569469a1b 100644 --- a/packages/provider-webdriver/src/webdriver-client.ts +++ b/packages/provider-webdriver/src/webdriver-client.ts @@ -1,6 +1,7 @@ import fs from 'node:fs/promises'; import { AppError } from '@agent-device/kernel/errors'; import { + isWebDriverRouteUnsupported, MUTATION_REQUEST_POLICY, WebDriverTransport, type WebDriverAuth, @@ -130,7 +131,8 @@ export class WebDriverClient { { appId }, MUTATION_REQUEST_POLICY, ); - } catch { + } catch (error) { + if (!isWebDriverRouteUnsupported(error)) throw error; await this.executeScript( 'mobile: activateApp', [{ appId, bundleId: appId }], @@ -147,7 +149,8 @@ export class WebDriverClient { { appId }, MUTATION_REQUEST_POLICY, ); - } catch { + } catch (error) { + if (!isWebDriverRouteUnsupported(error)) throw error; await this.executeScript( 'mobile: terminateApp', [{ appId, bundleId: appId }], diff --git a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts index d81eefbbf9..a8357264c4 100644 --- a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts +++ b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts @@ -19,7 +19,10 @@ afterEach(() => { * driver received the request and is still working it, not a connection * failure. */ -function hangingWebDriverFetch(hangOnPathSuffix: string): { +function hangingWebDriverFetch( + hangOnPathSuffix: string, + unsupportedPathSuffix?: string, +): { fetch: typeof globalThis.fetch; callsFor: (pathSuffix: string) => number; } { @@ -33,6 +36,12 @@ function hangingWebDriverFetch(hangOnPathSuffix: string): { headers: { 'Content-Type': 'application/json' }, }); } + if (unsupportedPathSuffix && url.pathname.endsWith(unsupportedPathSuffix)) { + return new Response( + JSON.stringify({ value: { error: 'unknown command', message: 'no such route' } }), + { status: 404, headers: { 'Content-Type': 'application/json' } }, + ); + } if (url.pathname.endsWith(hangOnPathSuffix)) { return await new Promise((_resolve, reject) => { init?.signal?.addEventListener('abort', () => reject(init.signal?.reason as Error)); @@ -53,8 +62,11 @@ function hangingWebDriverFetch(hangOnPathSuffix: string): { * abort listener race deterministically; `retryDelayMs` is cut to keep a * retried attempt's sleep out of the test budget. */ -async function connectedWebDriverInteractor(hangOnPathSuffix: string) { - const { fetch, callsFor } = hangingWebDriverFetch(hangOnPathSuffix); +async function connectedWebDriverInteractor( + hangOnPathSuffix: string, + unsupportedPathSuffix?: string, +) { + const { fetch, callsFor } = hangingWebDriverFetch(hangOnPathSuffix, unsupportedPathSuffix); globalThis.fetch = fetch; const client = new WebDriverClient({ clientVersion: '0.0.0-test', @@ -67,7 +79,7 @@ async function connectedWebDriverInteractor(hangOnPathSuffix: string) { backend: 'android', capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'android' }), }); - return { interactor, callsFor }; + return { client, interactor, callsFor }; } // A tap whose `POST .../actions` request times out gets exactly one attempt: @@ -127,3 +139,33 @@ test('a timed-out read is still retried once', async () => { assert.equal(callsFor('/source'), 2); }); + +// App activation has a sibling route (`mobile: activateApp`) for drivers without the Appium one. +// A timeout on the first route is not "unsupported": the driver may already be activating the app, +// so switching routes would send the mutation twice. +test('a timed-out app activation does not fall back to the sibling route', async () => { + const { client, callsFor } = await connectedWebDriverInteractor('/appium/device/activate_app'); + + await assert.rejects(client.activateApp('com.example.app'), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }); + + assert.equal(callsFor('/appium/device/activate_app'), 1); + assert.equal(callsFor('/execute/sync'), 0); +}); + +// A driver that answers the first route with "unknown command" never ran it, so the sibling route +// is the one attempt that reaches the device. +test('an unsupported app-termination route falls back to the sibling route once', async () => { + const { client, callsFor } = await connectedWebDriverInteractor( + '/never-hangs', + '/appium/device/terminate_app', + ); + + await client.terminateApp('com.example.app'); + + assert.equal(callsFor('/appium/device/terminate_app'), 1); + assert.equal(callsFor('/execute/sync'), 1); +}); diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index 44034fc091..331b28b80a 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -269,9 +269,40 @@ function webdriverError(status: number, payload: unknown): AppError { // A 5xx means the driver received and processed the request, but not // whether the mutation it described completed before it failed. ...(status >= 500 ? { dispatched: 'unknown' as const } : {}), + ...(isUnsupportedRouteAnswer(status, payload) ? { dispatched: 'no' as const } : {}), }); } +/** W3C `error` codes a driver answers with when it does not implement the route at all. */ +const UNSUPPORTED_ROUTE_ERROR_CODES: ReadonlySet = new Set([ + 'unknown command', + 'unknown method', +]); + +function isUnsupportedRouteAnswer(status: number, payload: unknown): boolean { + if (status === 404 || status === 405) return true; + const value = + payload && typeof payload === 'object' && 'value' in payload + ? (payload as { value?: unknown }).value + : undefined; + const code = + value && typeof value === 'object' ? (value as { error?: unknown }).error : undefined; + return typeof code === 'string' && UNSUPPORTED_ROUTE_ERROR_CODES.has(code); +} + +/** + * The driver answered that it does not implement the route, so nothing was dispatched and a + * sibling route for the same action may be tried. A timeout or 5xx is not this answer: the first + * route may already have acted. + */ +export function isWebDriverRouteUnsupported(error: unknown): error is AppError { + return ( + error instanceof AppError && + error.details?.dispatched === 'no' && + typeof error.details.status === 'number' + ); +} + function webdriverTimeoutError( method: string, path: string, From 541b5af10b199db54dd42b3492296f811b1f9d2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 29 Sep 2026 13:07:36 +0200 Subject: [PATCH 05/13] fix(provider-webdriver): a 404 naming another W3C error is not an unsupported route Only the W3C unknown command/method codes, or a bare 404/405 without a W3C error code, switch activateApp/terminateApp to the sibling route and report dispatched: no. An invalid-session 404 stays unclassified. --- .../src/webdriver-mutation-resend.test.ts | 29 +++++++++++++++++-- .../src/webdriver-transport.ts | 9 ++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts index a8357264c4..c680550742 100644 --- a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts +++ b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts @@ -22,6 +22,7 @@ afterEach(() => { function hangingWebDriverFetch( hangOnPathSuffix: string, unsupportedPathSuffix?: string, + unsupportedErrorCode = 'unknown command', ): { fetch: typeof globalThis.fetch; callsFor: (pathSuffix: string) => number; @@ -38,7 +39,7 @@ function hangingWebDriverFetch( } if (unsupportedPathSuffix && url.pathname.endsWith(unsupportedPathSuffix)) { return new Response( - JSON.stringify({ value: { error: 'unknown command', message: 'no such route' } }), + JSON.stringify({ value: { error: unsupportedErrorCode, message: 'refused' } }), { status: 404, headers: { 'Content-Type': 'application/json' } }, ); } @@ -65,8 +66,13 @@ function hangingWebDriverFetch( async function connectedWebDriverInteractor( hangOnPathSuffix: string, unsupportedPathSuffix?: string, + unsupportedErrorCode?: string, ) { - const { fetch, callsFor } = hangingWebDriverFetch(hangOnPathSuffix, unsupportedPathSuffix); + const { fetch, callsFor } = hangingWebDriverFetch( + hangOnPathSuffix, + unsupportedPathSuffix, + unsupportedErrorCode, + ); globalThis.fetch = fetch; const client = new WebDriverClient({ clientVersion: '0.0.0-test', @@ -169,3 +175,22 @@ test('an unsupported app-termination route falls back to the sibling route once' assert.equal(callsFor('/appium/device/terminate_app'), 1); assert.equal(callsFor('/execute/sync'), 1); }); + +// W3C also answers 404 for a session that no longer exists. That is not an unsupported route, so +// the sibling route is not tried and the failure is not classified as never dispatched here. +test('a 404 naming another W3C error does not fall back to the sibling route', async () => { + const { client, callsFor } = await connectedWebDriverInteractor( + '/never-hangs', + '/appium/device/terminate_app', + 'invalid session id', + ); + + await assert.rejects(client.terminateApp('com.example.app'), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.dispatched, undefined); + return true; + }); + + assert.equal(callsFor('/appium/device/terminate_app'), 1); + assert.equal(callsFor('/execute/sync'), 0); +}); diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index 331b28b80a..4e8656ca63 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -279,15 +279,20 @@ const UNSUPPORTED_ROUTE_ERROR_CODES: ReadonlySet = new Set([ 'unknown method', ]); +/** + * A W3C `unknown command`/`unknown method` code, or a bare 404/405 that carries no W3C error code + * (a server that does not know the path). A 404 that names another W3C code, such as + * `invalid session id` or `no such element`, is a different answer and never counts. + */ function isUnsupportedRouteAnswer(status: number, payload: unknown): boolean { - if (status === 404 || status === 405) return true; const value = payload && typeof payload === 'object' && 'value' in payload ? (payload as { value?: unknown }).value : undefined; const code = value && typeof value === 'object' ? (value as { error?: unknown }).error : undefined; - return typeof code === 'string' && UNSUPPORTED_ROUTE_ERROR_CODES.has(code); + if (typeof code === 'string') return UNSUPPORTED_ROUTE_ERROR_CODES.has(code); + return status === 404 || status === 405; } /** From 072ab000fd1a294ad06a7b942bd26ff27b45cd36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 30 Sep 2026 15:35:30 +0200 Subject: [PATCH 06/13] refactor(provider-webdriver): retry by HTTP method instead of per-call opt-out The transport now grants its retry budget only to an idempotent request: GET/HEAD by method, or a request that marks itself idempotent. Every POST and DELETE gets one attempt by default, so a new mutating route is safe without opting in. The MUTATION_REQUEST_POLICY constant, its 16 call-site overrides, the executeScript overrides parameter, and createSession's literal retryAttempts: 0 are gone. mobile: getClipboard, a read sent as a POST, goes through executeReadScript, which marks the request idempotent. The mutation-resend table covers every mutating route and asserts the table matches the non-GET requests and mobile: scripts found in the provider source. Mutations checked: - default retry for every method (drop the idempotency check): 17 table rows, the POST connect-refusal and POST 5xx transport tests, and the createSession no-retry client test fail (20). - executeReadScript without idempotent: the getClipboard retry test fails. - empty idempotent-method set: the GET source retry test and two GET transport retry tests fail. - rename a mobile: script at its call site: the table-coverage test fails. --- .../src/webdriver-client.ts | 99 ++---- .../src/webdriver-interactor.ts | 30 +- .../src/webdriver-mutation-resend.test.ts | 306 +++++++++++------- .../src/webdriver-transport.test.ts | 23 +- .../src/webdriver-transport.ts | 32 +- 5 files changed, 252 insertions(+), 238 deletions(-) diff --git a/packages/provider-webdriver/src/webdriver-client.ts b/packages/provider-webdriver/src/webdriver-client.ts index 8569469a1b..5c1b83a655 100644 --- a/packages/provider-webdriver/src/webdriver-client.ts +++ b/packages/provider-webdriver/src/webdriver-client.ts @@ -2,7 +2,6 @@ import fs from 'node:fs/promises'; import { AppError } from '@agent-device/kernel/errors'; import { isWebDriverRouteUnsupported, - MUTATION_REQUEST_POLICY, WebDriverTransport, type WebDriverAuth, type WebDriverRequestOverrides, @@ -94,10 +93,7 @@ export class WebDriverClient { 'POST', '/session', { capabilities: normalizeCapabilities(capabilities) }, - { - retryAttempts: 0, - timeoutMs: budgetWithin(this.sessionCreateTimeoutMs, options?.deadline), - }, + { timeoutMs: budgetWithin(this.sessionCreateTimeoutMs, options?.deadline) }, ); const session = readSession(value); this.sessionId = session.sessionId; @@ -107,82 +103,46 @@ export class WebDriverClient { // fallow-ignore-next-line unused-class-member async deleteSession(): Promise { const sessionId = this.requireSessionId(); - await this.requestValue('DELETE', `/session/${sessionId}`, undefined, MUTATION_REQUEST_POLICY); + await this.requestValue('DELETE', `/session/${sessionId}`); this.sessionId = undefined; } async installApp(appPath: string, signal?: AbortSignal): Promise { - await this.sessionRequest( - 'POST', - '/appium/device/install_app', - { appPath }, - { - ...MUTATION_REQUEST_POLICY, - signal, - }, - ); + await this.sessionRequest('POST', '/appium/device/install_app', { appPath }, { signal }); } async activateApp(appId: string): Promise { try { - await this.sessionRequest( - 'POST', - '/appium/device/activate_app', - { appId }, - MUTATION_REQUEST_POLICY, - ); + await this.sessionRequest('POST', '/appium/device/activate_app', { appId }); } catch (error) { if (!isWebDriverRouteUnsupported(error)) throw error; - await this.executeScript( - 'mobile: activateApp', - [{ appId, bundleId: appId }], - MUTATION_REQUEST_POLICY, - ); + await this.executeScript('mobile: activateApp', [{ appId, bundleId: appId }]); } } async terminateApp(appId: string): Promise { try { - await this.sessionRequest( - 'POST', - '/appium/device/terminate_app', - { appId }, - MUTATION_REQUEST_POLICY, - ); + await this.sessionRequest('POST', '/appium/device/terminate_app', { appId }); } catch (error) { if (!isWebDriverRouteUnsupported(error)) throw error; - await this.executeScript( - 'mobile: terminateApp', - [{ appId, bundleId: appId }], - MUTATION_REQUEST_POLICY, - ); + await this.executeScript('mobile: terminateApp', [{ appId, bundleId: appId }]); } } async performActions(actions: W3CActionSequence[]): Promise { - await this.sessionRequest('POST', '/actions', { actions }, MUTATION_REQUEST_POLICY); + await this.sessionRequest('POST', '/actions', { actions }); } async releaseActions(): Promise { - await this.sessionRequest('DELETE', '/actions', undefined, MUTATION_REQUEST_POLICY); + await this.sessionRequest('DELETE', '/actions'); } async sendKeys(text: string): Promise { - await this.sessionRequest( - 'POST', - '/keys', - { value: Array.from(text) }, - MUTATION_REQUEST_POLICY, - ); + await this.sessionRequest('POST', '/keys', { value: Array.from(text) }); } async hideKeyboard(): Promise { - await this.sessionRequest( - 'POST', - '/appium/device/hide_keyboard', - undefined, - MUTATION_REQUEST_POLICY, - ); + await this.sessionRequest('POST', '/appium/device/hide_keyboard'); } /** @@ -269,22 +229,17 @@ export class WebDriverClient { } async back(): Promise { - await this.sessionRequest('POST', '/back', undefined, MUTATION_REQUEST_POLICY); + await this.sessionRequest('POST', '/back'); } /** Exact four-way display rotation. `z` is degrees: 0, 90, 180, or 270. */ async setRotation(degrees: number): Promise { - await this.sessionRequest( - 'POST', - '/rotation', - { x: 0, y: 0, z: degrees }, - MUTATION_REQUEST_POLICY, - ); + await this.sessionRequest('POST', '/rotation', { x: 0, y: 0, z: degrees }); } /** Two-way orientation. Values are uppercase per the WebDriver protocol. */ async setOrientation(orientation: 'PORTRAIT' | 'LANDSCAPE'): Promise { - await this.sessionRequest('POST', '/orientation', { orientation }, MUTATION_REQUEST_POLICY); + await this.sessionRequest('POST', '/orientation', { orientation }); } /** @@ -317,19 +272,19 @@ export class WebDriverClient { return readWindowRect(await this.sessionRequest('GET', '/window/rect')); } - /** - * `mobile:` scripts cover both reads (`getClipboard`) and mutations - * (`activateApp`, `deepLink`, `pressButton`, `setClipboard`, …) behind one - * generic route, so the caller — which knows what the script does — states - * the retry policy; omitting `overrides` keeps the transport default for a - * read. - */ - async executeScript( - script: string, - args: unknown[] = [], - overrides?: WebDriverRequestOverrides, - ): Promise { - return await this.sessionRequest('POST', '/execute/sync', { script, args }, overrides); + /** A script that may change device state; it is sent once. */ + async executeScript(script: string, args: unknown[] = []): Promise { + return await this.sessionRequest('POST', '/execute/sync', { script, args }); + } + + /** A script that only reads, so it may be resent after an ambiguous failure. */ + async executeReadScript(script: string, args: unknown[] = []): Promise { + return await this.sessionRequest( + 'POST', + '/execute/sync', + { script, args }, + { idempotent: true }, + ); } private async sessionRequest( diff --git a/packages/provider-webdriver/src/webdriver-interactor.ts b/packages/provider-webdriver/src/webdriver-interactor.ts index af352dfce4..2d748e582e 100644 --- a/packages/provider-webdriver/src/webdriver-interactor.ts +++ b/packages/provider-webdriver/src/webdriver-interactor.ts @@ -22,7 +22,7 @@ import { type CloudWebDriverProviderCapabilities, } from './capabilities.ts'; import type { W3CPointerAction, WebDriverClient, WebDriverWindowRect } from './webdriver-client.ts'; -import { isWebDriverRequestTimeout, MUTATION_REQUEST_POLICY } from './webdriver-transport.ts'; +import { isWebDriverRequestTimeout } from './webdriver-transport.ts'; import { touchPointer } from './webdriver-gestures.ts'; import { scrollFrameFromAndroidWebDriverSource, @@ -143,11 +143,7 @@ class WebDriverInteractor implements Interactor { }, ): Promise { if (options?.url) { - await this.client.executeScript( - 'mobile: deepLink', - [{ url: options.url, package: app }], - MUTATION_REQUEST_POLICY, - ); + await this.client.executeScript('mobile: deepLink', [{ url: options.url, package: app }]); return; } const appId = options?.appBundleId ?? app; @@ -156,7 +152,7 @@ class WebDriverInteractor implements Interactor { } async openDevice(): Promise { - await this.client.executeScript('mobile: activateApp', [{}], MUTATION_REQUEST_POLICY); + await this.client.executeScript('mobile: activateApp', [{}]); } async close(app: string): Promise { @@ -348,11 +344,7 @@ class WebDriverInteractor implements Interactor { async home(): Promise { this.requireSupport('home'); - await this.client.executeScript( - 'mobile: pressButton', - [{ name: 'home' }], - MUTATION_REQUEST_POLICY, - ); + await this.client.executeScript('mobile: pressButton', [{ name: 'home' }]); } async setOrientation(orientation: DeviceRotation): Promise { @@ -362,11 +354,7 @@ class WebDriverInteractor implements Interactor { async appSwitcher(): Promise { this.requireSupport('appSwitcher'); - await this.client.executeScript( - 'mobile: pressButton', - [{ name: 'appSwitch' }], - MUTATION_REQUEST_POLICY, - ); + await this.client.executeScript('mobile: pressButton', [{ name: 'appSwitch' }]); } async tvRemote(_button: TvRemoteButton, _durationMs?: number): Promise { @@ -375,17 +363,13 @@ class WebDriverInteractor implements Interactor { async readClipboard(): Promise { this.requireSupport('clipboard.read'); - const value = await this.client.executeScript('mobile: getClipboard', [{}]); + const value = await this.client.executeReadScript('mobile: getClipboard', [{}]); return typeof value === 'string' ? value : ''; } async writeClipboard(text: string): Promise { this.requireSupport('clipboard.write'); - await this.client.executeScript( - 'mobile: setClipboard', - [{ content: text }], - MUTATION_REQUEST_POLICY, - ); + await this.client.executeScript('mobile: setClipboard', [{ content: text }]); } async setSetting( diff --git a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts index c680550742..fc690cfead 100644 --- a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts +++ b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts @@ -1,4 +1,6 @@ import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; import { afterEach, test } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; import { createCloudWebDriverCapabilities } from './capabilities.ts'; @@ -12,178 +14,252 @@ afterEach(() => { }); /** - * A grid that answers `POST /session` immediately, then never answers the one - * configured mutating route until the transport's own request timeout aborts - * it. Every other route answers immediately with an empty value. This is the - * shape a hung `POST .../actions` or `.../keys` takes on a real provider: the - * driver received the request and is still working it, not a connection - * failure. + * The route a request addresses: the `mobile:` script name for `POST .../execute/sync`, otherwise + * the method and path with the session id replaced by `:id`. + */ +function routeOf(input: Parameters[0], init?: RequestInit): string { + const url = new URL(input instanceof Request ? input.url : String(input)); + const method = init?.method ?? 'GET'; + const pathname = url.pathname + .replace(/^\/wd\/hub/, '') + .replace(/^\/session\/wd-1/, '/session/:id'); + if (pathname.endsWith('/execute/sync') && typeof init?.body === 'string') { + return (JSON.parse(init.body) as { script: string }).script; + } + return `${method} ${pathname}`; +} + +/** + * A grid that never answers `hangRoute` until the transport's own timeout aborts it, answers + * `unsupportedRoute` with a W3C 404, and answers every other route immediately. A hung request is + * the shape a slow driver takes on a real provider: it received the request and is still working + * on it. */ function hangingWebDriverFetch( - hangOnPathSuffix: string, - unsupportedPathSuffix?: string, + hangRoute: string, + unsupportedRoute?: string, unsupportedErrorCode = 'unknown command', -): { - fetch: typeof globalThis.fetch; - callsFor: (pathSuffix: string) => number; -} { - const paths: string[] = []; +): { fetch: typeof globalThis.fetch; sendsTo: (route: string) => number } { + const routes: string[] = []; const fetch: typeof globalThis.fetch = async (input, init) => { - const url = new URL(input instanceof Request ? input.url : String(input)); - paths.push(url.pathname); - if (url.pathname.endsWith('/session')) { - return new Response(JSON.stringify({ value: { sessionId: 'wd-1', capabilities: {} } }), { - status: 200, - headers: { 'Content-Type': 'application/json' }, - }); - } - if (unsupportedPathSuffix && url.pathname.endsWith(unsupportedPathSuffix)) { - return new Response( - JSON.stringify({ value: { error: unsupportedErrorCode, message: 'refused' } }), - { status: 404, headers: { 'Content-Type': 'application/json' } }, - ); - } - if (url.pathname.endsWith(hangOnPathSuffix)) { + const route = routeOf(input, init); + routes.push(route); + if (route === hangRoute) { return await new Promise((_resolve, reject) => { init?.signal?.addEventListener('abort', () => reject(init.signal?.reason as Error)); }); } - return new Response(JSON.stringify({ value: null }), { - status: 200, - headers: { 'Content-Type': 'application/json' }, - }); + if (route === 'POST /session') { + return Response.json({ value: { sessionId: 'wd-1', capabilities: {} } }); + } + if (route === unsupportedRoute) { + return Response.json( + { value: { error: unsupportedErrorCode, message: 'refused' } }, + { status: 404 }, + ); + } + return Response.json({ value: null }); }; - return { fetch, callsFor: (pathSuffix) => paths.filter((p) => p.endsWith(pathSuffix)).length }; + return { fetch, sendsTo: (route) => routes.filter((sent) => sent === route).length }; } /** - * A real `WebDriverClient` and `createWebDriverInteractor`, connected through - * a driver that hangs on `hangOnPathSuffix`. `timeoutMs` is the smallest the - * transport accepts that still lets `AbortSignal.timeout` and the fetch stub's - * abort listener race deterministically; `retryDelayMs` is cut to keep a - * retried attempt's sleep out of the test budget. + * A real `WebDriverClient` and `createWebDriverInteractor` over the hanging grid. The default + * transport policy retries once, so a request that is resent shows two sends. */ -async function connectedWebDriverInteractor( - hangOnPathSuffix: string, - unsupportedPathSuffix?: string, - unsupportedErrorCode?: string, -) { - const { fetch, callsFor } = hangingWebDriverFetch( - hangOnPathSuffix, - unsupportedPathSuffix, - unsupportedErrorCode, +async function connectedWebDriverInteractor(options: { + hangRoute: string; + unsupportedRoute?: string; + unsupportedErrorCode?: string; + createSession?: boolean; +}) { + const { fetch, sendsTo } = hangingWebDriverFetch( + options.hangRoute, + options.unsupportedRoute, + options.unsupportedErrorCode, ); globalThis.fetch = fetch; const client = new WebDriverClient({ clientVersion: '0.0.0-test', endpoint: 'http://cloud-webdriver.test/wd/hub/', - requestPolicy: { timeoutMs: 20, retryDelayMs: 5 }, + requestPolicy: { timeoutMs: 20, retryDelayMs: 5, sessionCreateTimeoutMs: 20 }, }); - await client.createSession({ platformName: 'Android' }); + if (options.createSession !== false) await client.createSession({ platformName: 'Android' }); const interactor = createWebDriverInteractor({ client, backend: 'android', - capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'android' }), + capabilities: createCloudWebDriverCapabilities({ + provider: 'test', + platform: 'android', + overrides: { + home: 'supported', + 'clipboard.read': 'supported', + 'clipboard.write': 'supported', + }, + }), }); - return { client, interactor, callsFor }; + return { client, interactor, sendsTo }; } -// A tap whose `POST .../actions` request times out gets exactly one attempt: -// a resend cannot tell whether the touch the driver is still processing from -// the first attempt already landed, so a second attempt risks a doubled -// gesture instead of a safe no-op. The thrown error still discloses that the -// outcome is unresolved via `details.dispatched`. -test('a timed-out tap is never resent', async () => { - const { interactor, callsFor } = await connectedWebDriverInteractor('/actions'); +type Connected = Awaited>; - await assert.rejects(interactor.tap(10, 20), (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.details?.dispatched, 'unknown'); - return true; - }); +type MutatingRouteRow = { + act: (connected: Connected) => Promise; + /** A route the grid answers as unsupported so `act` reaches the hung sibling route. */ + unsupportedRoute?: string; + createSession?: false; +}; - assert.equal(callsFor('/actions'), 1); -}); - -// Same shape for text entry: a hung `POST .../keys` must not be resent, or a -// doubled key stream could reach the field the first attempt already typed -// into. -test('timed-out keys are never resent', async () => { - const { interactor, callsFor } = await connectedWebDriverInteractor('/keys'); +const MUTATING_ROUTES: Record = { + 'POST /session': { + act: ({ client }) => client.createSession({ platformName: 'Android' }), + createSession: false, + }, + 'DELETE /session/:id': { act: ({ client }) => client.deleteSession() }, + 'POST /session/:id/appium/device/install_app': { + act: ({ client }) => client.installApp('/tmp/app.apk'), + }, + 'POST /session/:id/appium/device/activate_app': { + act: ({ client }) => client.activateApp('com.example.app'), + }, + 'POST /session/:id/appium/device/terminate_app': { + act: ({ client }) => client.terminateApp('com.example.app'), + }, + 'POST /session/:id/appium/device/hide_keyboard': { act: ({ client }) => client.hideKeyboard() }, + 'POST /session/:id/actions': { act: ({ interactor }) => interactor.tap(10, 20) }, + 'DELETE /session/:id/actions': { act: ({ client }) => client.releaseActions() }, + 'POST /session/:id/keys': { act: ({ client }) => client.sendKeys('hello') }, + 'POST /session/:id/back': { act: ({ interactor }) => interactor.back() }, + 'POST /session/:id/rotation': { act: ({ client }) => client.setRotation(90) }, + 'POST /session/:id/orientation': { act: ({ client }) => client.setOrientation('LANDSCAPE') }, + 'mobile: deepLink': { + act: ({ interactor }) => interactor.open('com.example.app', { url: 'example://home' }), + }, + 'mobile: activateApp': { act: ({ interactor }) => interactor.openDevice() }, + 'mobile: terminateApp': { + act: ({ client }) => client.terminateApp('com.example.app'), + unsupportedRoute: 'POST /session/:id/appium/device/terminate_app', + }, + 'mobile: pressButton': { + act: async ({ interactor }) => { + assert.ok(interactor.home); + await interactor.home(); + }, + }, + 'mobile: setClipboard': { + act: async ({ interactor }) => { + assert.ok(interactor.writeClipboard); + await interactor.writeClipboard('copied'); + }, + }, +}; - await assert.rejects(interactor.type('hello'), (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.details?.dispatched, 'unknown'); - return true; - }); +/** + * Every mutating route the provider sends, read from its source: each non-GET client request, and + * each `mobile:` script passed to `executeScript`. `POST .../execute/sync` itself is the carrier + * of the `mobile:` scripts, which are enumerated by name. + */ +function mutatingRoutesInSource(): string[] { + const sourceDir = path.dirname(new URL(import.meta.url).pathname); + const routes = new Set(); + for (const file of fs.readdirSync(sourceDir)) { + if (!file.endsWith('.ts') || file.endsWith('.test.ts')) continue; + const source = fs.readFileSync(path.join(sourceDir, file), 'utf8'); + for (const match of source.matchAll( + /(sessionRequest|requestValue)\(\s*'(POST|PUT|PATCH|DELETE)',\s*[`']([^`']+)[`']/g, + )) { + const [, helper, method, routePath] = match; + const absolute = helper === 'sessionRequest' ? `/session/:id${routePath}` : routePath; + const route = `${method} ${absolute!.replace('${sessionId}', ':id')}`; + if (route !== 'POST /session/:id/execute/sync') routes.add(route); + } + for (const match of source.matchAll(/\.executeScript\(\s*'(mobile: \w+)'/g)) { + routes.add(match[1]!); + } + } + return [...routes].sort(); +} - assert.equal(callsFor('/keys'), 1); +test('the mutating-route table covers every mutating route the provider sends', () => { + assert.deepEqual(Object.keys(MUTATING_ROUTES).sort(), mutatingRoutesInSource()); }); -// A third mutating route, distinct from the gesture/text-entry paths: a hung -// `POST .../back` must not be resent, or a doubled back navigation could -// leave the app a screen further back than the caller asked for. -test('a timed-out back is never resent', async () => { - const { interactor, callsFor } = await connectedWebDriverInteractor('/back'); +// A resend after a timeout cannot tell whether the first attempt's side effect already landed, so +// every mutating route gets one send and discloses that its outcome is unresolved. +for (const [route, row] of Object.entries(MUTATING_ROUTES)) { + test(`a timed-out ${route} is sent once`, async () => { + const connected = await connectedWebDriverInteractor({ + hangRoute: route, + unsupportedRoute: row.unsupportedRoute, + createSession: row.createSession, + }); - await assert.rejects(interactor.back(), (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.details?.dispatched, 'unknown'); - return true; + await assert.rejects(row.act(connected), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }); + + assert.equal(connected.sendsTo(route), 1); }); +} + +test('a timed-out GET read is resent once', async () => { + const { interactor, sendsTo } = await connectedWebDriverInteractor({ + hangRoute: 'GET /session/:id/source', + }); + + await assert.rejects(interactor.snapshot(), () => true); - assert.equal(callsFor('/back'), 1); + assert.equal(sendsTo('GET /session/:id/source'), 2); }); -// The mutation policy is narrowly scoped: a read route (page source) keeps -// the transport's default retry budget, so a timeout there still resends -// once, exactly as it did before this change. -test('a timed-out read is still retried once', async () => { - const { interactor, callsFor } = await connectedWebDriverInteractor('/source'); +test('a timed-out mobile: getClipboard read is resent once', async () => { + const { interactor, sendsTo } = await connectedWebDriverInteractor({ + hangRoute: 'mobile: getClipboard', + }); - await assert.rejects(interactor.snapshot(), () => true); + assert.ok(interactor.readClipboard); + await assert.rejects(interactor.readClipboard(), () => true); - assert.equal(callsFor('/source'), 2); + assert.equal(sendsTo('mobile: getClipboard'), 2); }); // App activation has a sibling route (`mobile: activateApp`) for drivers without the Appium one. // A timeout on the first route is not "unsupported": the driver may already be activating the app, // so switching routes would send the mutation twice. test('a timed-out app activation does not fall back to the sibling route', async () => { - const { client, callsFor } = await connectedWebDriverInteractor('/appium/device/activate_app'); - - await assert.rejects(client.activateApp('com.example.app'), (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.details?.dispatched, 'unknown'); - return true; + const { client, sendsTo } = await connectedWebDriverInteractor({ + hangRoute: 'POST /session/:id/appium/device/activate_app', }); - assert.equal(callsFor('/appium/device/activate_app'), 1); - assert.equal(callsFor('/execute/sync'), 0); + await assert.rejects(client.activateApp('com.example.app'), () => true); + + assert.equal(sendsTo('mobile: activateApp'), 0); }); // A driver that answers the first route with "unknown command" never ran it, so the sibling route // is the one attempt that reaches the device. test('an unsupported app-termination route falls back to the sibling route once', async () => { - const { client, callsFor } = await connectedWebDriverInteractor( - '/never-hangs', - '/appium/device/terminate_app', - ); + const { client, sendsTo } = await connectedWebDriverInteractor({ + hangRoute: 'none', + unsupportedRoute: 'POST /session/:id/appium/device/terminate_app', + }); await client.terminateApp('com.example.app'); - assert.equal(callsFor('/appium/device/terminate_app'), 1); - assert.equal(callsFor('/execute/sync'), 1); + assert.equal(sendsTo('POST /session/:id/appium/device/terminate_app'), 1); + assert.equal(sendsTo('mobile: terminateApp'), 1); }); // W3C also answers 404 for a session that no longer exists. That is not an unsupported route, so // the sibling route is not tried and the failure is not classified as never dispatched here. test('a 404 naming another W3C error does not fall back to the sibling route', async () => { - const { client, callsFor } = await connectedWebDriverInteractor( - '/never-hangs', - '/appium/device/terminate_app', - 'invalid session id', - ); + const { client, sendsTo } = await connectedWebDriverInteractor({ + hangRoute: 'none', + unsupportedRoute: 'POST /session/:id/appium/device/terminate_app', + unsupportedErrorCode: 'invalid session id', + }); await assert.rejects(client.terminateApp('com.example.app'), (error: unknown) => { assert.ok(error instanceof AppError); @@ -191,6 +267,6 @@ test('a 404 naming another W3C error does not fall back to the sibling route', a return true; }); - assert.equal(callsFor('/appium/device/terminate_app'), 1); - assert.equal(callsFor('/execute/sync'), 0); + assert.equal(sendsTo('POST /session/:id/appium/device/terminate_app'), 1); + assert.equal(sendsTo('mobile: terminateApp'), 0); }); diff --git a/packages/provider-webdriver/src/webdriver-transport.test.ts b/packages/provider-webdriver/src/webdriver-transport.test.ts index 3f1b63fa79..bead8469e8 100644 --- a/packages/provider-webdriver/src/webdriver-transport.test.ts +++ b/packages/provider-webdriver/src/webdriver-transport.test.ts @@ -2,7 +2,6 @@ import assert from 'node:assert/strict'; import { afterEach, test } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; import { - MUTATION_REQUEST_POLICY, WebDriverTransport, isWebDriverConnectRefused, isWebDriverRequestTimeout, @@ -125,10 +124,8 @@ test('a connection refusal classifies as unreached and is still retried by defau assert.equal(calls, 2); }); -// The one attempt `MUTATION_REQUEST_POLICY` grants must hold regardless of -// which ambiguous failure the request hit — connect refusal included, not -// only a timeout or a 5xx. -test('a connection refusal is not resent under the mutation policy', async () => { +// A POST may change device state, so it gets one attempt whichever failure it hit. +test('a connection refusal on a POST is not resent', async () => { const transport = new WebDriverTransport({ clientVersion: '0.0.0-test', endpoint: 'http://cloud-webdriver.test/wd/hub/', @@ -140,25 +137,26 @@ test('a connection refusal is not resent under the mutation policy', async () => throw new TypeError('fetch failed'); }; - await assert.rejects( - transport.requestValue('POST', '/session/wd-1/actions', {}, MUTATION_REQUEST_POLICY), - ); + await assert.rejects(transport.requestValue('POST', '/session/wd-1/actions', {})); assert.equal(calls, 1); }); // A 5xx means the driver answered — it received and processed the request — // but not whether the mutation it described completed before it failed. -test('a 5xx response discloses an unresolved outcome', async () => { +test('a 5xx response to a POST discloses an unresolved outcome and is not resent', async () => { const transport = new WebDriverTransport({ clientVersion: '0.0.0-test', endpoint: 'http://cloud-webdriver.test/wd/hub/', - requestPolicy: { timeoutMs: 30_000, retryAttempts: 0 }, + requestPolicy: { timeoutMs: 30_000, retryAttempts: 1, retryDelayMs: 1 }, }); - globalThis.fetch = async () => - new Response(JSON.stringify({ value: { message: 'grid unavailable' } }), { + let calls = 0; + globalThis.fetch = async () => { + calls += 1; + return new Response(JSON.stringify({ value: { message: 'grid unavailable' } }), { status: 503, headers: { 'Content-Type': 'application/json' }, }); + }; await assert.rejects( transport.requestValue('POST', '/session/wd-1/actions', {}), @@ -168,4 +166,5 @@ test('a 5xx response discloses an unresolved outcome', async () => { return true; }, ); + assert.equal(calls, 1); }); diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index 4e8656ca63..f9d5032a60 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -39,6 +39,12 @@ export function isWebDriverConnectRefused(error: unknown): error is AppError { export type WebDriverRequestOverrides = { retryAttempts?: number; + /** + * The request reads and changes nothing, so a resend after an ambiguous failure is safe. GET + * requests are idempotent by method; this marks a read the protocol sends as a POST, such as a + * `mobile:` read script. + */ + idempotent?: boolean; /** * Per-request transport bound, for callers whose own budget is far shorter * than the client's default. Without it a caller waiting 2s on a poll can be @@ -49,21 +55,6 @@ export type WebDriverRequestOverrides = { signal?: AbortSignal; }; -/** - * Every mutating WebDriver route gets exactly one attempt. A resend after an - * ambiguous outcome (the request timed out, or the driver answered 5xx after - * receiving it) cannot tell whether the first attempt's side effect — a tap, a - * key send, a navigation — already landed, so retrying risks a doubled action - * rather than a safe no-op. Reads keep the transport's default retry budget. - * - * `'no'` and `'unknown'` are the two values this transport can attach to a - * failed request's `details.dispatched`; `feat/dispatch-disclosure` formalizes - * that vocabulary as `DispatchDisclosure` (`'no' | 'yes' | 'unknown'`) in - * `@agent-device/contracts`. This policy is why a mutation never gets the - * chance to resend regardless of which value it carries. - */ -export const MUTATION_REQUEST_POLICY: WebDriverRequestOverrides = { retryAttempts: 0 }; - export type WebDriverTransportOptions = { clientVersion: string; endpoint: string | URL; @@ -88,6 +79,13 @@ type ResolvedWebDriverRequestPolicy = Required< NonNullable >; +/** + * Only a request that changes nothing gets the policy's retry budget. A resend after an ambiguous + * outcome (a timeout, or a 5xx after the driver received the request) cannot tell whether the first + * attempt's side effect already landed, so every other request gets exactly one attempt. + */ +const IDEMPOTENT_METHODS: ReadonlySet = new Set(['GET', 'HEAD']); + /** Focused HTTP/retry policy for one WebDriver endpoint; session semantics stay in WebDriverClient. */ export class WebDriverTransport { private readonly endpoint: URL; @@ -114,8 +112,10 @@ export class WebDriverTransport { body?: unknown, overrides?: WebDriverRequestOverrides, ): Promise { + const idempotent = overrides?.idempotent ?? IDEMPOTENT_METHODS.has(method); return await this.requestValueWithRetries(method, path, body, { - retryAttempts: overrides?.retryAttempts ?? this.requestPolicy.retryAttempts, + retryAttempts: + overrides?.retryAttempts ?? (idempotent ? this.requestPolicy.retryAttempts : 0), timeoutMs: overrides?.timeoutMs ?? this.requestPolicy.timeoutMs, signal: overrides?.signal, }); From 2925ed1ff766e2ff90ac7b9bab76e3513b33d834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 30 Sep 2026 15:37:37 +0200 Subject: [PATCH 07/13] fix(provider-webdriver): disclose no only for a failure before the driver was reached Every fetch TypeError was classified as a refused connection with dispatched: no. Undici throws the same TypeError after the driver received the request: a socket reset after the body was written, or a response body cut off after the status line. The transport now reads the socket error code from the error, its cause chain, and each AggregateError member (depth-capped), and discloses no only when every code is a pre-connect failure (ECONNREFUSED, ENOTFOUND, EAI_AGAIN, EHOSTUNREACH, ENETUNREACH, UND_ERR_CONNECT_TIMEOUT). Any other network failure, and any failure while reading the response body, is webdriver_request_interrupted with dispatched: unknown. Reads keep their retry on both, as before. The dispatched values share one local type, WebDriverDispatchDisclosure. The transport tests use real sockets instead of a hand-written TypeError: a released 127.0.0.1 port, a driver that destroys the socket after reading the body, and a driver that destroys it after writing the headers. Mutations checked: - classify every request-stage TypeError as refused: the socket-reset and body-cut-off tests fail (2). - drop ECONNREFUSED from the pre-connect set: the refused-connection test fails. - every -> some over the codes: no test fails (only a mixed AggregateError tells them apart). --- .../src/webdriver-transport.test.ts | 132 +++++++++++++++--- .../src/webdriver-transport.ts | 123 ++++++++++++---- 2 files changed, 208 insertions(+), 47 deletions(-) diff --git a/packages/provider-webdriver/src/webdriver-transport.test.ts b/packages/provider-webdriver/src/webdriver-transport.test.ts index bead8469e8..666c3cc602 100644 --- a/packages/provider-webdriver/src/webdriver-transport.test.ts +++ b/packages/provider-webdriver/src/webdriver-transport.test.ts @@ -1,4 +1,6 @@ import assert from 'node:assert/strict'; +import http from 'node:http'; +import net, { type AddressInfo } from 'node:net'; import { afterEach, test } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; import { @@ -99,46 +101,132 @@ test('cancels a retry delay when the request binding aborts', async () => { assert.equal(calls, 1); }); -// Fetch's own network-error shape (refused connection, unresolved host) is a -// bare TypeError with no HTTP status. It must still classify as retriable — -// preserving today's behavior for a read — and disclose that no bytes of the -// request ever reached the driver. -test('a connection refusal classifies as unreached and is still retried by default', async () => { - const transport = new WebDriverTransport({ - clientVersion: '0.0.0-test', - endpoint: 'http://cloud-webdriver.test/wd/hub/', - requestPolicy: { timeoutMs: 30_000, retryDelayMs: 1 }, +/** A 127.0.0.1 port nothing listens on: bound, then released. */ +async function refusedPort(): Promise { + const server = net.createServer(); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as AddressInfo; + await new Promise((resolve) => server.close(() => resolve())); + return port; +} + +/** A local driver whose every request runs `handle`; `requests` counts what reached it. */ +async function localDriver( + handle: (request: http.IncomingMessage, response: http.ServerResponse) => void, +): Promise<{ endpoint: string; requests: () => number; close: () => Promise }> { + let requests = 0; + const server = http.createServer((request, response) => { + requests += 1; + handle(request, response); }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as AddressInfo; + return { + endpoint: `http://127.0.0.1:${port}/wd/hub/`, + requests: () => requests, + close: async () => { + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + }, + }; +} + +/** Counts fetch calls while the real fetch still produces the real network failure. */ +function countFetchCalls(): () => number { let calls = 0; - globalThis.fetch = async () => { + globalThis.fetch = async (input, init) => { calls += 1; - throw new TypeError('fetch failed'); + return await realFetch(input, init); }; + return () => calls; +} + +// A refused connection was never established, so no byte of the request reached the driver. A +// GET read keeps its retry. +test('a refused connection discloses the request never reached the driver', async () => { + const transport = new WebDriverTransport({ + clientVersion: '0.0.0-test', + endpoint: `http://127.0.0.1:${await refusedPort()}/wd/hub/`, + requestPolicy: { timeoutMs: 5_000, retryDelayMs: 1 }, + }); + const calls = countFetchCalls(); await assert.rejects(transport.requestValue('GET', '/session/wd-1/source'), (error: unknown) => { - assert.ok(error instanceof AppError); assert.ok(isWebDriverConnectRefused(error)); assert.equal(error.details?.dispatched, 'no'); return true; }); - assert.equal(calls, 2); + assert.equal(calls(), 2); }); // A POST may change device state, so it gets one attempt whichever failure it hit. -test('a connection refusal on a POST is not resent', async () => { +test('a refused POST is not resent', async () => { const transport = new WebDriverTransport({ clientVersion: '0.0.0-test', - endpoint: 'http://cloud-webdriver.test/wd/hub/', - requestPolicy: { timeoutMs: 30_000, retryDelayMs: 1 }, + endpoint: `http://127.0.0.1:${await refusedPort()}/wd/hub/`, + requestPolicy: { timeoutMs: 5_000, retryDelayMs: 1 }, }); - let calls = 0; - globalThis.fetch = async () => { - calls += 1; - throw new TypeError('fetch failed'); - }; + const calls = countFetchCalls(); await assert.rejects(transport.requestValue('POST', '/session/wd-1/actions', {})); - assert.equal(calls, 1); + assert.equal(calls(), 1); +}); + +// The driver read the whole body before the socket reset, so it may have acted on it. +test('a socket reset after the driver read the body discloses an unresolved outcome', async () => { + const driver = await localDriver((request) => { + request.resume(); + request.on('end', () => request.socket.destroy()); + }); + const transport = new WebDriverTransport({ + clientVersion: '0.0.0-test', + endpoint: driver.endpoint, + requestPolicy: { timeoutMs: 5_000, retryDelayMs: 1 }, + }); + + try { + await assert.rejects( + transport.requestValue('POST', '/session/wd-1/actions', { actions: [] }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(isWebDriverConnectRefused(error), false); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }, + ); + assert.equal(driver.requests(), 1); + } finally { + await driver.close(); + } +}); + +// The status line arrived, so the driver received the request before the body was cut off. +test('a response body cut off after the headers discloses an unresolved outcome', async () => { + const driver = await localDriver((request, response) => { + request.resume(); + response.writeHead(200, { 'Content-Type': 'application/json', 'Content-Length': '100' }); + response.write('{"value":', () => request.socket.destroy()); + }); + const transport = new WebDriverTransport({ + clientVersion: '0.0.0-test', + endpoint: driver.endpoint, + requestPolicy: { timeoutMs: 5_000, retryDelayMs: 1 }, + }); + + try { + await assert.rejects( + transport.requestValue('POST', '/session/wd-1/keys', { value: ['a'] }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(isWebDriverConnectRefused(error), false); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }, + ); + assert.equal(driver.requests(), 1); + } finally { + await driver.close(); + } }); // A 5xx means the driver answered — it received and processed the request — diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index f9d5032a60..86c84c1f91 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -29,10 +29,45 @@ export function isWebDriverRequestTimeout(error: unknown): error is AppError { return error instanceof AppError && error.details?.reason === WEBDRIVER_REQUEST_TIMEOUT_REASON; } -/** Machine-readable `details.reason` of a request that never left the client. */ +/** + * What a failed request discloses about whether the driver acted on it. WebDriver never proves + * that a request executed, so a failure is either `no` (the request never reached the driver) or + * `unknown`. + */ +type WebDriverDispatchDisclosure = 'no' | 'unknown'; + +function dispatchDisclosure(dispatched: WebDriverDispatchDisclosure): { + dispatched: WebDriverDispatchDisclosure; +} { + return { dispatched }; +} + +/** Machine-readable `details.reason` of a request that never reached the driver. */ const WEBDRIVER_CONNECT_REFUSED_REASON = 'webdriver_connect_refused'; -/** A request that failed before any bytes reached the driver (refused/unreachable host). */ +/** + * Machine-readable `details.reason` of a request whose connection failed after it may have + * reached the driver: a socket reset, or a response body cut off after the status line. + */ +const WEBDRIVER_REQUEST_INTERRUPTED_REASON = 'webdriver_request_interrupted'; + +/** + * Socket error codes that prove the connection was never established, so no byte of the request + * reached the driver. + */ +const PRE_CONNECT_ERROR_CODES: ReadonlySet = new Set([ + 'ECONNREFUSED', + 'ENOTFOUND', + 'EAI_AGAIN', + 'EHOSTUNREACH', + 'ENETUNREACH', + 'UND_ERR_CONNECT_TIMEOUT', +]); + +/** How deep the `cause` and `AggregateError` chain is read for socket error codes. */ +const ERROR_CAUSE_DEPTH_LIMIT = 4; + +/** A request whose connection failed before it was established (refused or unreachable host). */ export function isWebDriverConnectRefused(error: unknown): error is AppError { return error instanceof AppError && error.details?.reason === WEBDRIVER_CONNECT_REFUSED_REASON; } @@ -176,10 +211,13 @@ export class WebDriverTransport { timeoutMs: number, requestSignal?: AbortSignal, ): Promise & { text: string }> { + const url = new URL(trimLeadingSlash(path), this.endpoint); const timeoutSignal = AbortSignal.timeout(timeoutMs); const signal = requestSignal ? AbortSignal.any([requestSignal, timeoutSignal]) : timeoutSignal; + const failure = { method, path, timeoutMs, timeoutSignal, requestSignal }; + let response: Response; try { - const response = await fetch(new URL(trimLeadingSlash(path), this.endpoint), { + response = await fetch(url, { method, headers: { Accept: 'application/json', @@ -189,24 +227,23 @@ export class WebDriverTransport { body: body === undefined ? undefined : JSON.stringify(body), signal, }); + } catch (error) { + throw classifyWebDriverFetchFailure(error, failure, 'request'); + } + try { return { ok: response.ok, status: response.status, text: await response.text() }; } catch (error) { - throw classifyWebDriverFetchFailure(error, { - method, - path, - timeoutMs, - timeoutSignal, - requestSignal, - }); + throw classifyWebDriverFetchFailure(error, failure, 'response'); } } } /** - * A caller's own cancellation keeps its reason as-is. Only a failure the - * transport itself can explain — its deadline, or a network failure fetch - * reports as a bare TypeError — becomes a typed error, so callers key on - * `details.reason` instead of sniffing fetch's DOMException name. + * A caller's own cancellation keeps its reason as-is. Only a failure the transport itself can + * explain — its deadline, or a network failure fetch reports as a TypeError — becomes a typed + * error, so callers key on `details.reason` instead of sniffing fetch's error names. A failure + * discloses `no` only when a socket error code proves the connection was never established; + * any other network failure, and every failure after the response headers arrived, is `unknown`. */ function classifyWebDriverFetchFailure( error: unknown, @@ -217,17 +254,37 @@ function classifyWebDriverFetchFailure( timeoutSignal: AbortSignal; requestSignal: AbortSignal | undefined; }, + stage: 'request' | 'response', ): unknown { const { method, path, timeoutMs, timeoutSignal, requestSignal } = context; if (requestSignal?.aborted) return error; if (timeoutSignal.aborted) return webdriverTimeoutError(method, path, timeoutMs, error); - // Fetch rejects with a bare TypeError for a network failure — refused - // connection, unresolved host, failed handshake — that never put a byte on - // the wire. - if (error instanceof TypeError) return webdriverConnectRefusedError(method, path, error); + if (stage === 'request' && failedBeforeConnect(error)) { + return webdriverConnectRefusedError(method, path, error); + } + if (stage === 'response' || error instanceof TypeError) { + return webdriverRequestInterruptedError(method, path, error); + } return error; } +function failedBeforeConnect(error: unknown): boolean { + const codes = socketErrorCodes(error, 0); + return codes.length > 0 && codes.every((code) => PRE_CONNECT_ERROR_CODES.has(code)); +} + +/** Every `code` on the error, its `cause` chain, and each `AggregateError` member. */ +function socketErrorCodes(error: unknown, depth: number): string[] { + if (depth > ERROR_CAUSE_DEPTH_LIMIT || !error || typeof error !== 'object') return []; + const { code, cause } = error as { code?: unknown; cause?: unknown }; + const members: unknown[] = error instanceof AggregateError ? error.errors : []; + return [ + ...(typeof code === 'string' ? [code] : []), + ...socketErrorCodes(cause, depth + 1), + ...members.flatMap((member) => socketErrorCodes(member, depth + 1)), + ]; +} + function shouldRetryWebDriverRequest( error: unknown, attempt: number, @@ -268,8 +325,8 @@ function webdriverError(status: number, payload: unknown): AppError { response: payload, // A 5xx means the driver received and processed the request, but not // whether the mutation it described completed before it failed. - ...(status >= 500 ? { dispatched: 'unknown' as const } : {}), - ...(isUnsupportedRouteAnswer(status, payload) ? { dispatched: 'no' as const } : {}), + ...(status >= 500 ? dispatchDisclosure('unknown') : {}), + ...(isUnsupportedRouteAnswer(status, payload) ? dispatchDisclosure('no') : {}), }); } @@ -323,13 +380,13 @@ function webdriverTimeoutError( path, timeoutMs, // The transport stopped waiting; the driver may still be mid-request. - dispatched: 'unknown' as const, + ...dispatchDisclosure('unknown'), }, cause instanceof Error ? cause : undefined, ); } -function webdriverConnectRefusedError(method: string, path: string, cause: TypeError): AppError { +function webdriverConnectRefusedError(method: string, path: string, cause: unknown): AppError { return new AppError( 'COMMAND_FAILED', `WebDriver ${method} ${path} could not reach the driver.`, @@ -337,16 +394,32 @@ function webdriverConnectRefusedError(method: string, path: string, cause: TypeE reason: WEBDRIVER_CONNECT_REFUSED_REASON, method, path, - // The connection itself failed, so no bytes of this request were sent. - dispatched: 'no' as const, + ...dispatchDisclosure('no'), + }, + cause instanceof Error ? cause : undefined, + ); +} + +function webdriverRequestInterruptedError(method: string, path: string, cause: unknown): AppError { + return new AppError( + 'COMMAND_FAILED', + `WebDriver ${method} ${path} lost its connection to the driver.`, + { + reason: WEBDRIVER_REQUEST_INTERRUPTED_REASON, + method, + path, + ...dispatchDisclosure('unknown'), }, - cause, + cause instanceof Error ? cause : undefined, ); } function isRetriableWebDriverError(error: unknown): boolean { if (isWebDriverRequestTimeout(error)) return true; if (isWebDriverConnectRefused(error)) return true; + if (error instanceof AppError && error.details?.reason === WEBDRIVER_REQUEST_INTERRUPTED_REASON) { + return true; + } if (error instanceof AppError) { const status = error.details?.status; return typeof status === 'number' && status >= 500; From bfe00170d869c53835df9e434b07c9b6ae20c84b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 30 Sep 2026 15:40:27 +0200 Subject: [PATCH 08/13] refactor(provider-webdriver): one classifier for an unsupported route Three copies decided whether a driver answer meant "route not implemented": the transport (404/405, W3C code wins), orientation (the same plus a SESSION_NOT_FOUND guard), and the client's keyboard and active-element probes (405/501, code or status). They disagreed on 404 and 501. The transport now owns the one rule: a W3C error code decides when present (unknown command / unknown method = unsupported, any other code is not); with no code, a bare 404, 405, or 501 is unsupported. It stamps details.reason: webdriver_route_unsupported with dispatched: no, and isWebDriverRouteUnsupported reads that reason. Orientation and the client probes call it; their status and code sets are deleted. SESSION_NOT_FOUND carries no reason, so the orientation guard is no longer needed. Single owner: `grep -rn "unknown command\|unknown method\|405\|501" packages/provider-webdriver/src --include='*.ts' | grep -v test` matches only webdriver-transport.ts (the two code and status sets and the isUnsupportedRouteAnswer JSDoc). The orientation test doubles now come from the real transport answering a stubbed HTTP response, instead of hand-built AppErrors. Mutations checked: - drop 501 from the status set: the bare-501 row fails. - let a status decide when a non-routing W3C code is present: 10 tests fail (classifier rows, keyboard/activeElement dead-session and no-such-element, orientation 404/405, sibling-route 404). - drop the reason from the unsupported error: 11 tests fail. --- .../src/webdriver-client.ts | 25 +----- .../src/webdriver-orientation.test.ts | 78 +++++++++++++------ .../src/webdriver-orientation.ts | 36 +-------- .../src/webdriver-transport.test.ts | 36 +++++++++ .../src/webdriver-transport.ts | 34 +++++--- 5 files changed, 118 insertions(+), 91 deletions(-) diff --git a/packages/provider-webdriver/src/webdriver-client.ts b/packages/provider-webdriver/src/webdriver-client.ts index 5c1b83a655..60183f91d7 100644 --- a/packages/provider-webdriver/src/webdriver-client.ts +++ b/packages/provider-webdriver/src/webdriver-client.ts @@ -165,7 +165,7 @@ export class WebDriverClient { ...(timeoutMs === undefined ? {} : { timeoutMs }), }); } catch (error) { - if (isUnimplementedWebDriverRoute(error)) return 'unsupported'; + if (isWebDriverRouteUnsupported(error)) return 'unsupported'; throw error; } return typeof value === 'boolean' ? value : 'unsupported'; @@ -205,7 +205,7 @@ export class WebDriverClient { await this.sessionRequest('GET', '/element/active', undefined, requestBudget(deadline)), ); } catch (error) { - if (isUnimplementedWebDriverRoute(error)) return 'unsupported'; + if (isWebDriverRouteUnsupported(error)) return 'unsupported'; if (isNoSuchElementError(error)) return 'none'; throw error; } @@ -219,7 +219,7 @@ export class WebDriverClient { ); return { id: elementId, rect: readWindowRect(value) }; } catch (error) { - if (isUnimplementedWebDriverRoute(error)) return 'unsupported'; + if (isWebDriverRouteUnsupported(error)) return 'unsupported'; // The focused element went away between the two calls — a stale answer, // not a broken driver. Report it as "nothing focused" so the caller polls // again rather than failing on a race it can simply retry out of. @@ -371,18 +371,6 @@ function readSession(value: unknown): WebDriverSession { return { sessionId, capabilities }; } -/** - * A route this driver does not implement, as opposed to one that failed. - * - * Classified from the W3C error code, NOT from the HTTP status: `unknown - * command` and `invalid session id` are both 404, so a status test would read a - * dead session as a missing feature — exactly the confusion that turns a broken - * session into a blind text entry. Only 405/501 are unambiguous enough to stand - * on their own. A 5xx, an auth rejection, or a timeout is a real failure. - */ -const UNIMPLEMENTED_WEBDRIVER_STATUSES = new Set([405, 501]); -const UNIMPLEMENTED_WEBDRIVER_ERRORS = new Set(['unknown command', 'unknown method']); - /** * The W3C element identifier, whose key is the spec's fixed UUID rather than a * readable name. Appium also echoes the legacy `ELEMENT` key; accept either so @@ -423,13 +411,6 @@ function isNoSuchElementError(error: unknown): boolean { return readWebDriverErrorCode(error).toLowerCase() === 'no such element'; } -function isUnimplementedWebDriverRoute(error: unknown): boolean { - if (!(error instanceof AppError)) return false; - const status = error.details?.status; - if (typeof status === 'number' && UNIMPLEMENTED_WEBDRIVER_STATUSES.has(status)) return true; - return UNIMPLEMENTED_WEBDRIVER_ERRORS.has(readWebDriverErrorCode(error).toLowerCase()); -} - function readWebDriverErrorCode(error: AppError): string { const response = error.details?.response; const value = diff --git a/packages/provider-webdriver/src/webdriver-orientation.test.ts b/packages/provider-webdriver/src/webdriver-orientation.test.ts index 91ea8e41bb..4195857282 100644 --- a/packages/provider-webdriver/src/webdriver-orientation.test.ts +++ b/packages/provider-webdriver/src/webdriver-orientation.test.ts @@ -1,21 +1,49 @@ -import { test } from 'vitest'; +import { afterEach, test } from 'vitest'; import assert from 'node:assert/strict'; import { AppError } from '@agent-device/kernel/errors'; import type { WebDriverClient } from './webdriver-client.ts'; import { setWebDriverOrientation } from './webdriver-orientation.ts'; +import { WebDriverTransport } from './webdriver-transport.ts'; type Call = { method: string; args: unknown[] }; -/** A driver answering "I do not implement this route" — the only case that earns a fallback. */ -function unsupportedEndpointError(): AppError { - return new AppError('COMMAND_FAILED', 'Unknown command', { - status: 404, - response: { value: { error: 'unknown command' } }, +const realFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = realFetch; +}); + +/** The error the real transport raises for this driver answer. */ +async function driverAnswerError(driver: typeof globalThis.fetch): Promise { + const transport = new WebDriverTransport({ + clientVersion: '0.0.0-test', + endpoint: 'http://cloud-webdriver.test/wd/hub/', + requestPolicy: { timeoutMs: 20 }, }); + globalThis.fetch = driver; + try { + await transport.requestValue('POST', '/session/wd-1/rotation', {}); + } catch (error) { + return error; + } finally { + globalThis.fetch = realFetch; + } + throw new Error('the driver answer did not fail'); +} + +function answer(status: number, body: unknown): () => Promise { + return () => driverAnswerError(async () => Response.json(body, { status })); } -function makeClient(options: { reject?: readonly string[]; rejectWith?: () => unknown } = {}): { +/** A driver answering "I do not implement this route" — the only case that earns a fallback. */ +const unsupportedEndpointError = answer(404, { + value: { error: 'unknown command', message: 'Unknown command' }, +}); + +function makeClient( + options: { reject?: readonly string[]; rejectWith?: () => Promise } = {}, +): { client: WebDriverClient; calls: Call[]; } { @@ -25,7 +53,7 @@ function makeClient(options: { reject?: readonly string[]; rejectWith?: () => un const record = (method: string) => { return async (...args: unknown[]): Promise => { calls.push({ method, args }); - if (reject.has(method)) throw rejectWith(); + if (reject.has(method)) throw await rejectWith(); }; }; return { @@ -84,40 +112,44 @@ test('a driver rejecting /rotation degrades to the two-way endpoint', async () = // A transport that fails for any reason other than "not implemented" must surface as itself. The // earlier implementation caught everything, so a timeout or an expired session was reported as an // orientation-support problem with the real cause discarded. -const NON_FALLBACK_FAILURES: readonly { name: string; error: () => unknown }[] = [ +const NON_FALLBACK_FAILURES: readonly { name: string; error: () => Promise }[] = [ { name: 'a request timeout', - error: () => Object.assign(new Error('The operation timed out'), { name: 'TimeoutError' }), + error: () => + driverAnswerError( + async (_input, init) => + await new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(init.signal?.reason as Error)); + }), + ), }, { name: 'an auth rejection', - error: () => new AppError('COMMAND_FAILED', 'Forbidden', { status: 403 }), + error: answer(403, { value: { error: 'unable to set cookie', message: 'Forbidden' } }), }, { name: 'a provider 5xx', - error: () => new AppError('COMMAND_FAILED', 'Bad gateway', { status: 502 }), + error: answer(502, { value: { error: 'unknown error', message: 'Bad gateway' } }), }, { name: 'a dead session', - error: () => new AppError('SESSION_NOT_FOUND', 'WebDriver session has not been created yet.'), + error: async () => + new AppError('SESSION_NOT_FOUND', 'WebDriver session has not been created yet.'), }, { // The status alone says "not found", but the W3C code says the session died. Reading the code // first is what keeps this from being misread as a missing route. name: 'a 404 carrying invalid session id', - error: () => - new AppError('COMMAND_FAILED', 'A session is either terminated or not started', { - status: 404, - response: { value: { error: 'invalid session id' } }, - }), + error: answer(404, { + value: { + error: 'invalid session id', + message: 'A session is either terminated or not started', + }, + }), }, { name: 'a 405 carrying a non-routing error code', - error: () => - new AppError('COMMAND_FAILED', 'Session timed out', { - status: 405, - response: { value: { error: 'timeout' } }, - }), + error: answer(405, { value: { error: 'timeout', message: 'Session timed out' } }), }, ]; diff --git a/packages/provider-webdriver/src/webdriver-orientation.ts b/packages/provider-webdriver/src/webdriver-orientation.ts index ff11196be3..29e29f57db 100644 --- a/packages/provider-webdriver/src/webdriver-orientation.ts +++ b/packages/provider-webdriver/src/webdriver-orientation.ts @@ -5,6 +5,7 @@ import { } from '@agent-device/contracts/device'; import { AppError } from '@agent-device/kernel/errors'; import type { WebDriverClient } from './webdriver-client.ts'; +import { isWebDriverRouteUnsupported } from './webdriver-transport.ts'; export type WebDriverOrientationBackend = 'android' | 'xctest'; @@ -48,7 +49,7 @@ export async function setWebDriverOrientation( // Only a driver saying "I do not implement this endpoint" justifies trying the next one. // A timeout, an auth rejection, a dead session or a 5xx is a real failure, and swallowing it // here would report it as an orientation-support problem and hide the actual cause. - if (!isUnsupportedEndpointError(error)) throw error; + if (!isWebDriverRouteUnsupported(error)) throw error; attempts.push({ transport, error: error instanceof Error ? error.message : String(error) }); } } @@ -64,39 +65,6 @@ export async function setWebDriverOrientation( ); } -/** - * W3C error codes a driver returns for an endpoint it does not implement. Keyed structurally rather - * than by message text, so a driver rewording its prose cannot turn a real failure into a fallback. - */ -const UNSUPPORTED_ENDPOINT_W3C_ERRORS: ReadonlySet = new Set([ - 'unknown command', - 'unknown method', -]); - -/** HTTP statuses that mean "this route does not exist here", as opposed to "it failed". */ -const UNSUPPORTED_ENDPOINT_STATUSES: ReadonlySet = new Set([404, 405]); - -function isUnsupportedEndpointError(error: unknown): boolean { - if (!(error instanceof AppError)) return false; - // A session that never opened, or died, is not an unsupported endpoint. - if (error.code === 'SESSION_NOT_FOUND') return false; - // The W3C code wins whenever the driver sent one: several real failures share the statuses - // below, most importantly a 404 carrying `invalid session id`, which is a dead session rather - // than a missing route. Bare status is only consulted when no code was returned at all. - const code = readW3CErrorCode(error.details?.response); - if (code) return UNSUPPORTED_ENDPOINT_W3C_ERRORS.has(code); - const status = error.details?.status; - return typeof status === 'number' && UNSUPPORTED_ENDPOINT_STATUSES.has(status); -} - -function readW3CErrorCode(response: unknown): string { - if (!response || typeof response !== 'object') return ''; - const value = (response as { value?: unknown }).value; - if (!value || typeof value !== 'object') return ''; - const code = (value as { error?: unknown }).error; - return typeof code === 'string' ? code : ''; -} - async function applyTransport( client: WebDriverClient, transport: OrientationTransport, diff --git a/packages/provider-webdriver/src/webdriver-transport.test.ts b/packages/provider-webdriver/src/webdriver-transport.test.ts index 666c3cc602..c0e2fb0178 100644 --- a/packages/provider-webdriver/src/webdriver-transport.test.ts +++ b/packages/provider-webdriver/src/webdriver-transport.test.ts @@ -7,6 +7,7 @@ import { WebDriverTransport, isWebDriverConnectRefused, isWebDriverRequestTimeout, + isWebDriverRouteUnsupported, } from './webdriver-transport.ts'; const realFetch = globalThis.fetch; @@ -256,3 +257,38 @@ test('a 5xx response to a POST discloses an unresolved outcome and is not resent ); assert.equal(calls, 1); }); + +const ROUTE_ANSWERS: readonly { status: number; body: unknown; unsupported: boolean }[] = [ + { status: 404, body: { value: { error: 'unknown command', message: 'x' } }, unsupported: true }, + { status: 405, body: { value: { error: 'unknown method', message: 'x' } }, unsupported: true }, + { status: 404, body: {}, unsupported: true }, + { status: 405, body: {}, unsupported: true }, + { status: 501, body: {}, unsupported: true }, + { + status: 404, + body: { value: { error: 'invalid session id', message: 'x' } }, + unsupported: false, + }, + { status: 405, body: { value: { error: 'timeout', message: 'x' } }, unsupported: false }, + { status: 501, body: { value: { error: 'unknown error', message: 'x' } }, unsupported: false }, + { status: 500, body: {}, unsupported: false }, + { status: 403, body: {}, unsupported: false }, +]; + +// A W3C error code decides when present; without one, a bare 404, 405, or 501 is the driver +// saying it does not implement the route. +for (const { status, body, unsupported } of ROUTE_ANSWERS) { + test(`HTTP ${status} ${JSON.stringify(body)} is ${unsupported ? '' : 'not '}an unsupported route`, async () => { + const transport = new WebDriverTransport({ + clientVersion: '0.0.0-test', + endpoint: 'http://cloud-webdriver.test/wd/hub/', + }); + globalThis.fetch = async () => Response.json(body, { status }); + + await assert.rejects(transport.requestValue('POST', '/session/wd-1/back'), (error: unknown) => { + assert.equal(isWebDriverRouteUnsupported(error), unsupported); + if (unsupported) assert.equal((error as AppError).details?.dispatched, 'no'); + return true; + }); + }); +} diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index 86c84c1f91..3c8c7b0006 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -320,26 +320,40 @@ function webdriverError(status: number, payload: unknown): AppError { typeof (value as { message?: unknown }).message === 'string' ? (value as { message: string }).message : `WebDriver request failed with HTTP ${status}.`; + if (isUnsupportedRouteAnswer(status, payload)) { + return new AppError('COMMAND_FAILED', message, { + reason: WEBDRIVER_ROUTE_UNSUPPORTED_REASON, + status, + response: payload, + ...dispatchDisclosure('no'), + }); + } return new AppError('COMMAND_FAILED', message, { status, response: payload, // A 5xx means the driver received and processed the request, but not // whether the mutation it described completed before it failed. ...(status >= 500 ? dispatchDisclosure('unknown') : {}), - ...(isUnsupportedRouteAnswer(status, payload) ? dispatchDisclosure('no') : {}), }); } -/** W3C `error` codes a driver answers with when it does not implement the route at all. */ +/** Machine-readable `details.reason` of a driver answer that it does not implement the route. */ +const WEBDRIVER_ROUTE_UNSUPPORTED_REASON = 'webdriver_route_unsupported'; + +/** W3C `error` codes a driver answers with when it does not implement the route. */ const UNSUPPORTED_ROUTE_ERROR_CODES: ReadonlySet = new Set([ 'unknown command', 'unknown method', ]); +/** HTTP statuses that, without a W3C error code, mean the route is not implemented here. */ +const UNSUPPORTED_ROUTE_STATUSES: ReadonlySet = new Set([404, 405, 501]); + /** - * A W3C `unknown command`/`unknown method` code, or a bare 404/405 that carries no W3C error code - * (a server that does not know the path). A 404 that names another W3C code, such as - * `invalid session id` or `no such element`, is a different answer and never counts. + * A W3C error code decides when the driver sent one, because other failures share these statuses + * (a 404 `invalid session id` is a dead session); with no code, a bare 404 (no such route), 405 + * (the route exists but not for this method), or 501 (the driver declares it not implemented) + * means the route is not implemented by this driver. */ function isUnsupportedRouteAnswer(status: number, payload: unknown): boolean { const value = @@ -348,8 +362,8 @@ function isUnsupportedRouteAnswer(status: number, payload: unknown): boolean { : undefined; const code = value && typeof value === 'object' ? (value as { error?: unknown }).error : undefined; - if (typeof code === 'string') return UNSUPPORTED_ROUTE_ERROR_CODES.has(code); - return status === 404 || status === 405; + if (typeof code === 'string') return UNSUPPORTED_ROUTE_ERROR_CODES.has(code.toLowerCase()); + return UNSUPPORTED_ROUTE_STATUSES.has(status); } /** @@ -358,11 +372,7 @@ function isUnsupportedRouteAnswer(status: number, payload: unknown): boolean { * route may already have acted. */ export function isWebDriverRouteUnsupported(error: unknown): error is AppError { - return ( - error instanceof AppError && - error.details?.dispatched === 'no' && - typeof error.details.status === 'number' - ); + return error instanceof AppError && error.details?.reason === WEBDRIVER_ROUTE_UNSUPPORTED_REASON; } function webdriverTimeoutError( From dfd4d274f6e1023dcc337798c45d024abfd35c15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 30 Sep 2026 15:42:02 +0200 Subject: [PATCH 09/13] feat(provider-webdriver): record an unsupported route in request diagnostics When activateApp or terminateApp falls back to its mobile: sibling route, the first route's answer left no trace: the fallback succeeds and nothing in --debug output showed that the Appium route was refused. The transport now emits a webdriver_route_unsupported diagnostic event with the method, path, and HTTP status each time it classifies an answer as an unsupported route. This covers the app-route fallbacks, the orientation fallback, and the keyboard and active-element probes. provider-webdriver now depends on @agent-device/host-kit for emitDiagnostic. capture-kit already brings it into the package's closure. Mutations checked: - remove the emitDiagnostic call: the 5 unsupported classifier rows and the terminate-app fallback test fail (6). --- packages/provider-webdriver/package.json | 1 + .../src/webdriver-mutation-resend.test.ts | 9 +++++++- .../src/webdriver-transport.test.ts | 21 +++++++++++++++---- .../src/webdriver-transport.ts | 9 ++++++-- pnpm-lock.yaml | 3 +++ 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/packages/provider-webdriver/package.json b/packages/provider-webdriver/package.json index 596d8f1c3d..d4d56b8d0e 100644 --- a/packages/provider-webdriver/package.json +++ b/packages/provider-webdriver/package.json @@ -7,6 +7,7 @@ "dependencies": { "@agent-device/capture-kit": "workspace:*", "@agent-device/contracts": "workspace:*", + "@agent-device/host-kit": "workspace:*", "@agent-device/kernel": "workspace:*", "@agent-device/xml": "workspace:*" }, diff --git a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts index fc690cfead..6a4ddce50e 100644 --- a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts +++ b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts @@ -2,6 +2,10 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { afterEach, test } from 'vitest'; +import { + countDiagnosticEventsByPhase, + withDiagnosticsScope, +} from '@agent-device/host-kit/diagnostics'; import { AppError } from '@agent-device/kernel/errors'; import { createCloudWebDriverCapabilities } from './capabilities.ts'; import { WebDriverClient } from './webdriver-client.ts'; @@ -246,7 +250,10 @@ test('an unsupported app-termination route falls back to the sibling route once' unsupportedRoute: 'POST /session/:id/appium/device/terminate_app', }); - await client.terminateApp('com.example.app'); + await withDiagnosticsScope({ command: 'close' }, async () => { + await client.terminateApp('com.example.app'); + assert.equal(countDiagnosticEventsByPhase(['webdriver_route_unsupported']), 1); + }); assert.equal(sendsTo('POST /session/:id/appium/device/terminate_app'), 1); assert.equal(sendsTo('mobile: terminateApp'), 1); diff --git a/packages/provider-webdriver/src/webdriver-transport.test.ts b/packages/provider-webdriver/src/webdriver-transport.test.ts index c0e2fb0178..5b3e6b8538 100644 --- a/packages/provider-webdriver/src/webdriver-transport.test.ts +++ b/packages/provider-webdriver/src/webdriver-transport.test.ts @@ -2,6 +2,10 @@ import assert from 'node:assert/strict'; import http from 'node:http'; import net, { type AddressInfo } from 'node:net'; import { afterEach, test } from 'vitest'; +import { + countDiagnosticEventsByPhase, + withDiagnosticsScope, +} from '@agent-device/host-kit/diagnostics'; import { AppError } from '@agent-device/kernel/errors'; import { WebDriverTransport, @@ -285,10 +289,19 @@ for (const { status, body, unsupported } of ROUTE_ANSWERS) { }); globalThis.fetch = async () => Response.json(body, { status }); - await assert.rejects(transport.requestValue('POST', '/session/wd-1/back'), (error: unknown) => { - assert.equal(isWebDriverRouteUnsupported(error), unsupported); - if (unsupported) assert.equal((error as AppError).details?.dispatched, 'no'); - return true; + await withDiagnosticsScope({ command: 'back' }, async () => { + await assert.rejects( + transport.requestValue('POST', '/session/wd-1/back'), + (error: unknown) => { + assert.equal(isWebDriverRouteUnsupported(error), unsupported); + if (unsupported) assert.equal((error as AppError).details?.dispatched, 'no'); + return true; + }, + ); + assert.equal( + countDiagnosticEventsByPhase(['webdriver_route_unsupported']), + unsupported ? 1 : 0, + ); }); }); } diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index 3c8c7b0006..9c726ba7d9 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -1,4 +1,5 @@ import { setTimeout as sleep } from 'node:timers/promises'; +import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { AppError } from '@agent-device/kernel/errors'; import { agentDeviceRequestHeaders } from './request-headers.ts'; import { basicAuthHeader, trimLeadingSlash, withTrailingSlash } from './webdriver-utils.ts'; @@ -200,8 +201,12 @@ export class WebDriverTransport { requestSignal, ); const payload = text ? parseJsonResponse(text) : {}; - if (!ok) throw webdriverError(status, payload); - return readWebDriverValue(payload); + if (ok) return readWebDriverValue(payload); + const error = webdriverError(status, payload); + if (isWebDriverRouteUnsupported(error)) { + emitDiagnostic({ phase: WEBDRIVER_ROUTE_UNSUPPORTED_REASON, data: { method, path, status } }); + } + throw error; } private async fetchWebDriver( diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9d3122356f..bf1452f083 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -478,6 +478,9 @@ importers: '@agent-device/contracts': specifier: workspace:* version: link:../contracts + '@agent-device/host-kit': + specifier: workspace:* + version: link:../host-kit '@agent-device/kernel': specifier: workspace:* version: link:../kernel From 33e3e2db9223152abed7d01d449a5fcf2cd685eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 30 Sep 2026 15:44:11 +0200 Subject: [PATCH 10/13] chore(gates): admit host-kit as a provider-webdriver dependency The package-boundaries layering test pins provider-webdriver's workspace dependencies; add @agent-device/host-kit, which the diagnostic event now needs. Drop the stale unused-class-member suppression on deleteSession: the mutation-resend table now calls it, and fallow audit reported the suppression as stale. --- packages/provider-webdriver/src/webdriver-client.ts | 1 - scripts/layering/package-boundaries.test.ts | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/provider-webdriver/src/webdriver-client.ts b/packages/provider-webdriver/src/webdriver-client.ts index 60183f91d7..2114561240 100644 --- a/packages/provider-webdriver/src/webdriver-client.ts +++ b/packages/provider-webdriver/src/webdriver-client.ts @@ -100,7 +100,6 @@ export class WebDriverClient { return session; } - // fallow-ignore-next-line unused-class-member async deleteSession(): Promise { const sessionId = this.requireSessionId(); await this.requestValue('DELETE', `/session/${sessionId}`); diff --git a/scripts/layering/package-boundaries.test.ts b/scripts/layering/package-boundaries.test.ts index 222b622719..ea6b4f56ef 100644 --- a/scripts/layering/package-boundaries.test.ts +++ b/scripts/layering/package-boundaries.test.ts @@ -758,6 +758,7 @@ test('the real tree parses, declares, and passes R11', () => { assert.deepEqual([...providerWebDriverPackage.workspaceDependencies].sort(), [ '@agent-device/capture-kit', '@agent-device/contracts', + '@agent-device/host-kit', '@agent-device/kernel', '@agent-device/xml', ]); From 5cd1d5ce99767e24622d84d9f8cf97d1d9dcaee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 30 Sep 2026 15:58:01 +0200 Subject: [PATCH 11/13] fix(provider-webdriver): load the diagnostics emitter on demand so the eager closure holds The static import of @agent-device/host-kit/diagnostics in the transport added two modules to provider-webdriver's eager import closure (51 vs 49 at the merge-base). The unsupported-route diagnostic is a rare path, so reportRouteUnsupported now imports the emitter inside the function; the classifier stays synchronous. The closure walker counts only static and top-level dynamic imports, so the budget holds. The host-kit dependency stays declared because the dynamic import still resolves the package. The diagnostic assertions wait for the event with vi.waitFor. Mutations checked: - remove the reportRouteUnsupported call: 6 tests fail (5 classifier rows and the terminate-app fallback test). --- .../src/webdriver-mutation-resend.test.ts | 6 ++++-- .../src/webdriver-transport.test.ts | 10 ++++++---- .../src/webdriver-transport.ts | 17 +++++++++++++---- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts index 6a4ddce50e..083f8d9527 100644 --- a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts +++ b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; -import { afterEach, test } from 'vitest'; +import { afterEach, test, vi } from 'vitest'; import { countDiagnosticEventsByPhase, withDiagnosticsScope, @@ -252,7 +252,9 @@ test('an unsupported app-termination route falls back to the sibling route once' await withDiagnosticsScope({ command: 'close' }, async () => { await client.terminateApp('com.example.app'); - assert.equal(countDiagnosticEventsByPhase(['webdriver_route_unsupported']), 1); + await vi.waitFor(() => + assert.equal(countDiagnosticEventsByPhase(['webdriver_route_unsupported']), 1), + ); }); assert.equal(sendsTo('POST /session/:id/appium/device/terminate_app'), 1); diff --git a/packages/provider-webdriver/src/webdriver-transport.test.ts b/packages/provider-webdriver/src/webdriver-transport.test.ts index 5b3e6b8538..59b4386540 100644 --- a/packages/provider-webdriver/src/webdriver-transport.test.ts +++ b/packages/provider-webdriver/src/webdriver-transport.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import http from 'node:http'; import net, { type AddressInfo } from 'node:net'; -import { afterEach, test } from 'vitest'; +import { afterEach, test, vi } from 'vitest'; import { countDiagnosticEventsByPhase, withDiagnosticsScope, @@ -298,9 +298,11 @@ for (const { status, body, unsupported } of ROUTE_ANSWERS) { return true; }, ); - assert.equal( - countDiagnosticEventsByPhase(['webdriver_route_unsupported']), - unsupported ? 1 : 0, + await vi.waitFor(() => + assert.equal( + countDiagnosticEventsByPhase(['webdriver_route_unsupported']), + unsupported ? 1 : 0, + ), ); }); }); diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index 9c726ba7d9..a051942a4d 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -1,5 +1,4 @@ import { setTimeout as sleep } from 'node:timers/promises'; -import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { AppError } from '@agent-device/kernel/errors'; import { agentDeviceRequestHeaders } from './request-headers.ts'; import { basicAuthHeader, trimLeadingSlash, withTrailingSlash } from './webdriver-utils.ts'; @@ -203,9 +202,7 @@ export class WebDriverTransport { const payload = text ? parseJsonResponse(text) : {}; if (ok) return readWebDriverValue(payload); const error = webdriverError(status, payload); - if (isWebDriverRouteUnsupported(error)) { - emitDiagnostic({ phase: WEBDRIVER_ROUTE_UNSUPPORTED_REASON, data: { method, path, status } }); - } + if (isWebDriverRouteUnsupported(error)) reportRouteUnsupported({ method, path, status }); throw error; } @@ -371,6 +368,18 @@ function isUnsupportedRouteAnswer(status: number, payload: unknown): boolean { return UNSUPPORTED_ROUTE_STATUSES.has(status); } +/** + * Records the answer in the request diagnostics. The emitter loads on demand because this path is + * rare and the transport sits in the provider's eager import closure. + */ +function reportRouteUnsupported(data: { method: string; path: string; status: number }): void { + void import('@agent-device/host-kit/diagnostics') + .then(({ emitDiagnostic }) => + emitDiagnostic({ phase: WEBDRIVER_ROUTE_UNSUPPORTED_REASON, data }), + ) + .catch(() => undefined); +} + /** * The driver answered that it does not implement the route, so nothing was dispatched and a * sibling route for the same action may be tried. A timeout or 5xx is not this answer: the first From 3690196bcbe4eea4e848fb219e9c52326ad0d9e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 30 Sep 2026 17:21:31 +0200 Subject: [PATCH 12/13] feat(provider-webdriver): report the first route's W3C code and the sibling fallback in diagnostics The webdriver_route_unsupported event now carries the W3C error code of the refused answer (value.error, or null when the body has none) next to method, path, and status. When activateApp or terminateApp falls back, the client emits webdriver_route_fallback with the first route, the mobile: script it ran, and that request's HTTP status, after the sibling request succeeds. Both events are debug level and load the emitter on demand through one transport helper, emitWebDriverDiagnostic, so the eager closure is unchanged. The transport's new request() returns the answer with its status; requestValue() delegates to it. The activateApp and terminateApp fallback tests read the --debug request log a diagnostics scope writes and assert both events with every field. The route-table scan also reads the app routes and their siblings from appRouteWithSibling calls. Mutations checked: - drop the fallback emit: both fallback tests fail (2). - drop the code field: both fallback tests fail (2). - drop the unsupported-route emit: 7 tests fail. - rename a sibling script: the table-coverage test fails. --- .../src/webdriver-client.ts | 33 +++-- .../src/webdriver-mutation-resend.test.ts | 124 +++++++++++++----- .../src/webdriver-transport.ts | 62 +++++---- 3 files changed, 153 insertions(+), 66 deletions(-) diff --git a/packages/provider-webdriver/src/webdriver-client.ts b/packages/provider-webdriver/src/webdriver-client.ts index 2114561240..06276c4cf9 100644 --- a/packages/provider-webdriver/src/webdriver-client.ts +++ b/packages/provider-webdriver/src/webdriver-client.ts @@ -1,6 +1,7 @@ import fs from 'node:fs/promises'; import { AppError } from '@agent-device/kernel/errors'; import { + emitWebDriverDiagnostic, isWebDriverRouteUnsupported, WebDriverTransport, type WebDriverAuth, @@ -111,20 +112,36 @@ export class WebDriverClient { } async activateApp(appId: string): Promise { - try { - await this.sessionRequest('POST', '/appium/device/activate_app', { appId }); - } catch (error) { - if (!isWebDriverRouteUnsupported(error)) throw error; - await this.executeScript('mobile: activateApp', [{ appId, bundleId: appId }]); - } + await this.appRouteWithSibling('/appium/device/activate_app', 'mobile: activateApp', appId); } async terminateApp(appId: string): Promise { + await this.appRouteWithSibling('/appium/device/terminate_app', 'mobile: terminateApp', appId); + } + + /** + * Sends the Appium app route, and the `mobile:` sibling script only when the driver answered + * that it does not implement the first route. + */ + private async appRouteWithSibling( + route: string, + siblingScript: string, + appId: string, + ): Promise { try { - await this.sessionRequest('POST', '/appium/device/terminate_app', { appId }); + await this.sessionRequest('POST', route, { appId }); } catch (error) { if (!isWebDriverRouteUnsupported(error)) throw error; - await this.executeScript('mobile: terminateApp', [{ appId, bundleId: appId }]); + const { status } = await this.transport.request( + 'POST', + `/session/${this.requireSessionId()}/execute/sync`, + { script: siblingScript, args: [{ appId, bundleId: appId }] }, + ); + emitWebDriverDiagnostic('webdriver_route_fallback', { + from: route, + to: siblingScript, + status, + }); } } diff --git a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts index 083f8d9527..3c10e06246 100644 --- a/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts +++ b/packages/provider-webdriver/src/webdriver-mutation-resend.test.ts @@ -2,12 +2,10 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { afterEach, test, vi } from 'vitest'; -import { - countDiagnosticEventsByPhase, - withDiagnosticsScope, -} from '@agent-device/host-kit/diagnostics'; +import { withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; import { AppError } from '@agent-device/kernel/errors'; import { createCloudWebDriverCapabilities } from './capabilities.ts'; +import { mkdtempForTest } from './tmp-dir.fixtures.ts'; import { WebDriverClient } from './webdriver-client.ts'; import { createWebDriverInteractor } from './webdriver-interactor.ts'; @@ -159,29 +157,39 @@ const MUTATING_ROUTES: Record = { }; /** - * Every mutating route the provider sends, read from its source: each non-GET client request, and - * each `mobile:` script passed to `executeScript`. `POST .../execute/sync` itself is the carrier + * Every mutating route the provider sends, read from its source: each non-GET client request, each + * Appium app route and its `mobile:` sibling, and each `mobile:` script passed to `executeScript`. `POST .../execute/sync` itself is the carrier * of the `mobile:` scripts, which are enumerated by name. */ function mutatingRoutesInSource(): string[] { const sourceDir = path.dirname(new URL(import.meta.url).pathname); - const routes = new Set(); - for (const file of fs.readdirSync(sourceDir)) { - if (!file.endsWith('.ts') || file.endsWith('.test.ts')) continue; - const source = fs.readFileSync(path.join(sourceDir, file), 'utf8'); - for (const match of source.matchAll( + const sources = fs + .readdirSync(sourceDir) + .filter((file) => file.endsWith('.ts') && !file.endsWith('.test.ts')) + .map((file) => fs.readFileSync(path.join(sourceDir, file), 'utf8')); + const routes = sources.flatMap((source) => [ + ...requestRoutesIn(source), + ...appRoutesWithSiblingIn(source), + ...[...source.matchAll(/\.executeScript\(\s*'(mobile: \w+)'/g)].map((match) => match[1]!), + ]); + return [...new Set(routes)].filter((route) => route !== 'POST /session/:id/execute/sync').sort(); +} + +function requestRoutesIn(source: string): string[] { + return [ + ...source.matchAll( /(sessionRequest|requestValue)\(\s*'(POST|PUT|PATCH|DELETE)',\s*[`']([^`']+)[`']/g, - )) { - const [, helper, method, routePath] = match; - const absolute = helper === 'sessionRequest' ? `/session/:id${routePath}` : routePath; - const route = `${method} ${absolute!.replace('${sessionId}', ':id')}`; - if (route !== 'POST /session/:id/execute/sync') routes.add(route); - } - for (const match of source.matchAll(/\.executeScript\(\s*'(mobile: \w+)'/g)) { - routes.add(match[1]!); - } - } - return [...routes].sort(); + ), + ].map(([, helper, method, routePath]) => { + const absolute = helper === 'sessionRequest' ? `/session/:id${routePath}` : routePath!; + return `${method} ${absolute.replace('${sessionId}', ':id')}`; + }); +} + +function appRoutesWithSiblingIn(source: string): string[] { + return [...source.matchAll(/appRouteWithSibling\(\s*'([^']+)',\s*'(mobile: \w+)'/g)].flatMap( + ([, route, script]) => [`POST /session/:id${route}`, script!], + ); } test('the mutating-route table covers every mutating route the provider sends', () => { @@ -242,24 +250,68 @@ test('a timed-out app activation does not fall back to the sibling route', async assert.equal(sendsTo('mobile: activateApp'), 0); }); -// A driver that answers the first route with "unknown command" never ran it, so the sibling route -// is the one attempt that reaches the device. -test('an unsupported app-termination route falls back to the sibling route once', async () => { - const { client, sendsTo } = await connectedWebDriverInteractor({ - hangRoute: 'none', - unsupportedRoute: 'POST /session/:id/appium/device/terminate_app', +/** The `--debug` diagnostics log a request writes, one parsed event per line. */ +async function debugEvents( + run: () => Promise, +): Promise<{ phase: string; level: string; data?: Record }[]> { + const logPath = path.join(await mkdtempForTest('webdriver-fallback-'), 'request.ndjson'); + const read = () => + fs.existsSync(logPath) + ? fs + .readFileSync(logPath, 'utf8') + .trim() + .split('\n') + .map( + (line) => + JSON.parse(line) as { phase: string; level: string; data?: Record }, + ) + : []; + await withDiagnosticsScope({ command: 'open', debug: true, logPath }, async () => { + await run(); + await vi.waitFor(() => assert.equal(read().length, 2)); }); + return read(); +} + +// A driver that answers the first route with "unknown command" never ran it, so the sibling route +// is the one attempt that reaches the device. The debug log shows the refused route with its W3C +// code, then the sibling route that ran. +for (const { action, route, script } of [ + { action: 'activateApp', route: '/appium/device/activate_app', script: 'mobile: activateApp' }, + { action: 'terminateApp', route: '/appium/device/terminate_app', script: 'mobile: terminateApp' }, +] as const) { + test(`an unsupported ${action} route falls back to the sibling route once`, async () => { + const { client, sendsTo } = await connectedWebDriverInteractor({ + hangRoute: 'none', + unsupportedRoute: `POST /session/:id${route}`, + }); - await withDiagnosticsScope({ command: 'close' }, async () => { - await client.terminateApp('com.example.app'); - await vi.waitFor(() => - assert.equal(countDiagnosticEventsByPhase(['webdriver_route_unsupported']), 1), + const events = await debugEvents(() => client[action]('com.example.app')); + + assert.deepEqual( + events.map(({ phase, level, data }) => ({ phase, level, data })), + [ + { + phase: 'webdriver_route_unsupported', + level: 'debug', + data: { + method: 'POST', + path: `/session/wd-1${route}`, + status: 404, + code: 'unknown command', + }, + }, + { + phase: 'webdriver_route_fallback', + level: 'debug', + data: { from: route, to: script, status: 200 }, + }, + ], ); + assert.equal(sendsTo(`POST /session/:id${route}`), 1); + assert.equal(sendsTo(script), 1); }); - - assert.equal(sendsTo('POST /session/:id/appium/device/terminate_app'), 1); - assert.equal(sendsTo('mobile: terminateApp'), 1); -}); +} // W3C also answers 404 for a session that no longer exists. That is not an unsupported route, so // the sibling route is not tried and the failure is not classified as never dispatched here. diff --git a/packages/provider-webdriver/src/webdriver-transport.ts b/packages/provider-webdriver/src/webdriver-transport.ts index a051942a4d..aa1e28993c 100644 --- a/packages/provider-webdriver/src/webdriver-transport.ts +++ b/packages/provider-webdriver/src/webdriver-transport.ts @@ -121,6 +121,9 @@ type ResolvedWebDriverRequestPolicy = Required< */ const IDEMPOTENT_METHODS: ReadonlySet = new Set(['GET', 'HEAD']); +/** A successful WebDriver answer: its HTTP status and the unwrapped W3C `value`. */ +export type WebDriverAnswer = { status: number; value: unknown }; + /** Focused HTTP/retry policy for one WebDriver endpoint; session semantics stay in WebDriverClient. */ export class WebDriverTransport { private readonly endpoint: URL; @@ -147,8 +150,18 @@ export class WebDriverTransport { body?: unknown, overrides?: WebDriverRequestOverrides, ): Promise { + return (await this.request(method, path, body, overrides)).value; + } + + /** The request's answer with the HTTP status it arrived with. */ + async request( + method: string, + path: string, + body?: unknown, + overrides?: WebDriverRequestOverrides, + ): Promise { const idempotent = overrides?.idempotent ?? IDEMPOTENT_METHODS.has(method); - return await this.requestValueWithRetries(method, path, body, { + return await this.requestWithRetries(method, path, body, { retryAttempts: overrides?.retryAttempts ?? (idempotent ? this.requestPolicy.retryAttempts : 0), timeoutMs: overrides?.timeoutMs ?? this.requestPolicy.timeoutMs, @@ -156,22 +169,16 @@ export class WebDriverTransport { }); } - private async requestValueWithRetries( + private async requestWithRetries( method: string, path: string, body: unknown, overrides: ResolvedWebDriverRequestOverrides, - ): Promise { + ): Promise { let lastError: unknown; for (let attempt = 0; attempt <= overrides.retryAttempts; attempt += 1) { try { - return await this.requestValueOnce( - method, - path, - body, - overrides.timeoutMs, - overrides.signal, - ); + return await this.requestOnce(method, path, body, overrides.timeoutMs, overrides.signal); } catch (error) { lastError = error; if ( @@ -185,13 +192,13 @@ export class WebDriverTransport { throw lastError; } - private async requestValueOnce( + private async requestOnce( method: string, path: string, body: unknown, timeoutMs: number, requestSignal?: AbortSignal, - ): Promise { + ): Promise { const { ok, status, text } = await this.fetchWebDriver( method, path, @@ -200,9 +207,16 @@ export class WebDriverTransport { requestSignal, ); const payload = text ? parseJsonResponse(text) : {}; - if (ok) return readWebDriverValue(payload); + if (ok) return { status, value: readWebDriverValue(payload) }; const error = webdriverError(status, payload); - if (isWebDriverRouteUnsupported(error)) reportRouteUnsupported({ method, path, status }); + if (isWebDriverRouteUnsupported(error)) { + emitWebDriverDiagnostic(WEBDRIVER_ROUTE_UNSUPPORTED_REASON, { + method, + path, + status, + code: w3cErrorCode(payload) ?? null, + }); + } throw error; } @@ -358,25 +372,29 @@ const UNSUPPORTED_ROUTE_STATUSES: ReadonlySet = new Set([404, 405, 501]) * means the route is not implemented by this driver. */ function isUnsupportedRouteAnswer(status: number, payload: unknown): boolean { + const code = w3cErrorCode(payload); + if (code !== undefined) return UNSUPPORTED_ROUTE_ERROR_CODES.has(code.toLowerCase()); + return UNSUPPORTED_ROUTE_STATUSES.has(status); +} + +/** The W3C `value.error` string of a response body, if it carries one. */ +function w3cErrorCode(payload: unknown): string | undefined { const value = payload && typeof payload === 'object' && 'value' in payload ? (payload as { value?: unknown }).value : undefined; const code = value && typeof value === 'object' ? (value as { error?: unknown }).error : undefined; - if (typeof code === 'string') return UNSUPPORTED_ROUTE_ERROR_CODES.has(code.toLowerCase()); - return UNSUPPORTED_ROUTE_STATUSES.has(status); + return typeof code === 'string' ? code : undefined; } /** - * Records the answer in the request diagnostics. The emitter loads on demand because this path is - * rare and the transport sits in the provider's eager import closure. + * Records a debug event in the request diagnostics. The emitter loads on demand because the + * events are rare and the transport sits in the provider's eager import closure. */ -function reportRouteUnsupported(data: { method: string; path: string; status: number }): void { +export function emitWebDriverDiagnostic(phase: string, data: Record): void { void import('@agent-device/host-kit/diagnostics') - .then(({ emitDiagnostic }) => - emitDiagnostic({ phase: WEBDRIVER_ROUTE_UNSUPPORTED_REASON, data }), - ) + .then(({ emitDiagnostic }) => emitDiagnostic({ level: 'debug', phase, data })) .catch(() => undefined); } From e307316606333fe4ad17b001a77de80ce97cc9de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 30 Sep 2026 17:22:32 +0200 Subject: [PATCH 13/13] test(provider-webdriver): pin the every-code and depth-cap rules An AggregateError discloses no only when every member failed before connecting: all-ECONNREFUSED is no, ECONNREFUSED plus ECONNRESET is unknown. The cause chain is read to depth 4 below the thrown error: a pre-connect code at depth 4 is no, the same code at depth 5 is not read, so the failure is unknown. The doubles throw undici's shape, a TypeError('fetch failed') whose cause holds the socket error. Mutations checked: - every -> some over the codes: the AggregateError test fails. - depth cap 4 -> 5, and 4 -> 3: the depth-cap test fails for each. --- .../src/webdriver-transport.test.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/packages/provider-webdriver/src/webdriver-transport.test.ts b/packages/provider-webdriver/src/webdriver-transport.test.ts index 59b4386540..f003ebff64 100644 --- a/packages/provider-webdriver/src/webdriver-transport.test.ts +++ b/packages/provider-webdriver/src/webdriver-transport.test.ts @@ -307,3 +307,52 @@ for (const { status, body, unsupported } of ROUTE_ANSWERS) { }); }); } + +/** The shape undici rejects with: `TypeError('fetch failed')` whose `cause` holds the socket error. */ +function fetchFailedWith(cause: unknown): TypeError { + return new TypeError('fetch failed', { cause }); +} + +function socketError(code: string): Error { + return Object.assign(new Error(code), { code }); +} + +/** `socketError(code)` wrapped in `layers` code-less errors, so the code sits `layers + 1` causes down. */ +function nestedCause(code: string, layers: number): Error { + let error = socketError(code); + for (let layer = 0; layer < layers; layer += 1) error = new Error('wrapped', { cause: error }); + return error; +} + +async function dispatchedAfter(failure: TypeError): Promise { + const transport = new WebDriverTransport({ + clientVersion: '0.0.0-test', + endpoint: 'http://cloud-webdriver.test/wd/hub/', + }); + globalThis.fetch = async () => { + throw failure; + }; + try { + await transport.requestValue('POST', '/session/wd-1/actions', {}); + } catch (error) { + return (error as AppError).details?.dispatched; + } + throw new Error('the request did not fail'); +} + +// Happy-eyeballs connects surface as an AggregateError over every address tried. One address that +// connected and then reset means the request may have reached the driver. +test('an AggregateError discloses no only when every member failed before connecting', async () => { + const allRefused = new AggregateError([socketError('ECONNREFUSED'), socketError('ECONNREFUSED')]); + const oneReset = new AggregateError([socketError('ECONNREFUSED'), socketError('ECONNRESET')]); + + assert.equal(await dispatchedAfter(fetchFailedWith(allRefused)), 'no'); + assert.equal(await dispatchedAfter(fetchFailedWith(oneReset)), 'unknown'); +}); + +// The cause chain is read to depth 4 below the thrown error. A code further down is not read, and +// a failure with no readable code never proves the request stayed off the wire. +test('a pre-connect code is read to the depth cap and not beyond', async () => { + assert.equal(await dispatchedAfter(fetchFailedWith(nestedCause('ECONNREFUSED', 3))), 'no'); + assert.equal(await dispatchedAfter(fetchFailedWith(nestedCause('ECONNREFUSED', 4))), 'unknown'); +});