mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
Michał Pierzchała
a853734f0c
* fix(webdriver): give cloud session creation its own budget and stop leaking billed sessions Cloud lease allocation ran under the generic 30s/1-retry request policy, so BrowserStack iOS real-device session creation (45-90s) aborted client-side at ~60s on most runs. Each timed-out POST /session still completed server-side and, being non-idempotent, was retried — leaving two billed provider sessions per failed open with no id to release them. - POST /session is its own phase: a 180s create budget (default), zero retries, and no request-bound abort, so the daemon always learns the session id. - lease_allocate carries a 300s allocation budget surfaced to providers as LeaseLifecycleContext.deadline, and a matching 330s client envelope that preserves the daemon on timeout (a reset would SIGKILL mid-create and orphan every billed session the daemon held). - The request's cancellation signal is ownership evidence: a session that completes after the requester left is released, not registered; a create that the transport gives up on surfaces typed evidence (provider + lease) so an operator can find and stop the maybe-orphaned session. Closes #1774 * refactor: one canceled-request error, and tighten the #1774 shapes Review pass over the session-create fix: - The canceled-request error had nine hand-rolled copies (src/request/cancel, maestro shared, exec, retry, install-source x2, and the new provider one). It now has one definition in @agent-device/kernel/errors: createRequestCanceledError(details?, cause?) + isRequestCanceledError + REQUEST_CANCELED_REASON. Callers add evidence or a sharper hint; the reason itself is not overridable, so nothing can build one the predicate misses. - lease_allocate's timeout bundle moves beside INSTALL_TIMEOUT_POLICY in the registry (same {...DEFAULT, envelopeMs, onTimeout} shape); the request timeout constant stays exported from timeout-policy like its siblings. - Transport: fetch helper returns Response's own ok/status; the timeout reason const is private behind isWebDriverRequestTimeout. - Client: one-use options type inlined; the two deadline helpers share one floor. - Session-manager tests: shared makeRuntime/jsonResponse/afterEach restore. Net -29 lines with the feature in. * chore: keep the canceled-request reason private to the kernel * fix: typed cancellation everywhere + own the AWS remote-access ARN through startup Second-order follow-ups the #1774 refactor made cheap: - markRequestCanceled aborts the request signal WITH the kernel's typed canceled error as its reason. Every signal.throwIfAborted(), aborted fetch, and 'throw signal.reason' in the daemon (20+ sites) now surfaces a canceled request as such instead of a bare DOMException that normalized to UNKNOWN — and no site has to know the factory exists. - AWS Device Farm prepareSession owns the remote-access ARN from the moment create-remote-access-session answers: a startup timeout, the allocation deadline, or a canceled request now stops it before the failure surfaces (previously a timed-out startup left a RUNNING billed session behind — the same leak class as the WebDriver session, one phase earlier). The startup wait is capped by LeaseLifecycleContext.deadline and wakes on cancellation. - BrowserStack's pre-session local app upload honors the request signal (an upload is not billed, so plain abort is right there). - lease_heartbeat/lease_release share lease_allocate's preserve-daemon policy: the rationale — the daemon owns billed sessions; a reset orphans them all — applies verbatim. Each AWS ownership test proven red without the guard (3/3). * refactor: dedupe billed-resource cleanup and lease-signal wiring Shrink pass — same behavior, less duplication: - releaseOnFailure(primaryError, release) in webdriver-utils replaces the two identical 'best-effort stop the billed resource, attach cleanupError to the primary AppError' helpers (WebDriver session + AWS remote-access ARN); shared errorMessage too. - The lease handler pulls the request signal from getRequestSignal(requestId) like every sibling handler, instead of threading a requestSignal arg through LeaseHandlerArgs and the request-handler chain. Drops the field, the wiring, and five mechanical test edits; the handler test now proves the request-bound signal (abort it, watch the provider's signal flip) rather than arg identity. - Inlined the one-use requestHeaders back into fetchWebDriver. Handler-signal test proven red without the wiring. * fix(lease): the daemon releases a lease allocated for a gone requester; honest release evidence Review follow-up. The provider was doing the daemon's job: it treated the request signal as 'ownership evidence, not an interrupt' and needed three paragraphs to say so. The daemon owns the request, so it now decides — generically, for every provider — what happens to a lease that finished allocating after its requester left: release it (provider + registry) and answer with the canceled error. - lease.ts: after allocate returns, isRequestCanceled(requestId) → releaseAllocationForGoneRequester(). Release evidence is claimed ONLY on a clean release (no warnings, no throw); a WEBDRIVER_SESSION_DELETE_FAILED release is reported released:false with providerSessionId + a stop-by-hand hint (thymikee's finding: the previous evidence was success-shaped even when DELETE failed). - WebDriverSessionManager: the createOwnedSession/releaseCanceledSession trio is gone; allocate is plain 'create with a budget; on failure clean up' again. - LeaseLifecycleContext.signal is just cancellation, like everywhere else; the ownership-semantics comments on the contract, client, registry, AWS prepare and utils shrink to what the code no longer says itself. - Tests: the two provider-level cancellation tests move to the daemon handler (where the logic now lives), plus the failing-DELETE regression; both proven red without the post-allocate check. * fix(aws): the allocation deadline bounds remote-access startup, not the 120s default Live iOS real-device run: startup needed ~128s and hit the standalone 120s default while the daemon's 300s allocation budget still had room — the new ownership guard correctly stopped the ARN, but the open failed for no reason. When the daemon supplies a deadline it is the bound; the default only applies standalone. Rerun: open in 112s, snapshot, clean close, session STOPPING. * test(aws): pin that the allocation deadline outlives the 120s startup default; drop empty import Review follow-ups on 7f9d1481a: a virtual-clock test (Date.now advanced 10s per poll, RUNNING at 150s, deadline 300s) that fails on the old min(default, deadline) logic and passes now; and the empty 'import {} from kernel/errors' left in maestro/shared.ts is removed. * refactor: finish the dedupe — one release path, kernel errorMessage, AWS on releaseOnFailure Code-quality review at 7f9d1481a: 1. aws-device-farm.ts still carried its own copy of releaseOnFailure (the dedupe commit's script aborted before reaching it and I mis-verified). Now uses the shared helper; private copy deleted. 2. Empty 'import {} from kernel/errors' in maestro/shared.ts removed (273870099). 3. errorMessage() lives in @agent-device/kernel/errors; the two copies this PR had added (lease.ts, webdriver-utils.ts) import it. Sweeping the pre-existing copies is a follow-up. 4. lease.ts has ONE release path: releaseLease(registry, provider, lease, request, ctx) → { released (registry), provider } used by both the lease_release case (wire shape unchanged) and the gone-requester branch, which folds a throwing provider release into releaseError. 'released' now means the same thing in both; the provider verdict is a separate 'providerReleased' (warnings-free, no throw) that drives the stop-by-hand hint. -~35 lines. 5. sessionCreateTimeoutMs is Omit-ed at the WebDriverTransportOptions boundary instead of Pick-ed back out internally. * fix(lease): 'released' on a canceled allocation means the billed session is confirmed gone Re-review at 3665ea06: unifying the release path had made the cancellation error report released:true from the daemon's registry record while the provider DELETE had failed — success-shaped again, with the operator verdict demoted to a second key. Fixed at the source of the ambiguity: - LeaseReleaseOutcome names its bookkeeping field registryReleased. - On the canceled error, 'released' is true only when registryReleased AND the provider released without warnings AND without throwing; the registry record is exposed as 'registryReleased'. The stop-by-hand hint keys on 'released'. - lease_release keeps its existing wire field ('released' = registry; provider cleanup rides in 'provider'), unchanged. - Regressions: failed DELETE and throwing release both pin released:false / registryReleased:true (+ providerSessionId, warnings|releaseError, hint); both proven red on registry-only semantics. * ci: retrigger default-setup CodeQL Run 32051017472 is wedged on GitHub's side: status=completed with Analyze (python) still queued and Analyze (java-kotlin) failed only at SARIF upload (503, 'No server is currently available'). It can be neither cancelled nor rerun, and default-setup CodeQL has no dispatchable workflow, so a new push is the only way to get a fresh run. No source change. * test(webdriver): assert the typed timeout contract on the shared-budget probe main's #1790 tightened this test to expect the raw TimeoutError DOMException, which this PR intentionally normalizes into AppError{reason: webdriver_request_timeout}. On the merge ref the two met and Coverage went red. The regression now asserts the structured contract and that the second request's budget is the shared remainder (~118ms of 200 after an 80ms first call).