Skip to content

fix(tracing): Address redundant state captures per action - #377

Merged
Winify merged 6 commits into
webdriverio:mainfrom
Winify:fix/redundant-trace-capture
Sep 22, 2026
Merged

Winify merged 6 commits into
webdriverio:mainfrom
Winify:fix/redundant-trace-capture

Conversation

@Winify

@Winify Winify commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

What & why

mode: 'trace' took two DOM captures per action plus a readyState poll hiding the second one's motion. On native Appium each capture is two serial round trips (GET /screenshot ~1.2 s at 1.86 MB, GET /source ~0.09s at 40 KB) — ~1.2 s per action, bracketing #351's 40–60 s/run. This restores the original one-capture design and removes the two patches built on it.

Closes #351

The original approach

One capture per action in beforeCommand, taken before the command is issued — the one moment the driver is idle, so an action's result is the next action's "before" and no row resolves to a state in motion.

Stamped at the previous action's end (Date.now() for the session's first, which makes it the initial frame); afterCommand captures nothing in trace mode, it only drains the collector.

Nothing is waited for, because the gap it needs is the test's own: measured on Appium, a capture at a 0 s gap is 359–476 KB mid-transition against 1,871,924 B settled, and already settled at 0.1 s and 0.25 s. Cost: 1.19 s/action vs 2.41 s; end-to-end 12.4 s vs 19.1 s, live 5.9 s. Only the last action has no successor to hand its result to — hence a settle in exactly one place.

How it regressed

  • 44477f7 — per-action capture born: one capture in afterCommand, fire-and-forget.
  • b3ed046 — getPageSource enters the mobile element path.
  • 6f8cd47 — native guard: skips execute/getUrl/getTitle, so native ≈ one screenshot.
  • d924a02 — capture moved into beforeCommand, stamped at the previous action's end.
  • 06d0ee1 — the doubling: an eager post-action capture added beside the pre-capture, both stamped to one slot, the second discarded by the richer-screenshot merge.
  • 86f5b10 — the patch: waitForActionResult, a readyState poll + 250 ms pause, because that eager capture lands while the screen still moves.

The doubling came first; the wait hid the consequence of the capture it added. The beforeCommand design was never the problem, and the native guard survived throughout — just paid twice. Nuance for review: the second capture was not only waste — the merge keeps the larger screenshot, so it supplied the settled frame whenever a test had a gap.

That accident is why a settle is needed for the last action and nowhere else.

What changed

  • One capture per action; captureActionResult, waitForActionResult, the __wdioSnapMark tag deleted. The last action's capture comes from #finalizePerScenario, named after that action; FINAL_SNAPSHOT_COMMAND (final) is now a shared const reserved for a session that ran no action — skipping it by name would otherwise have dropped the only capture of the last action, i.e. the failing row's own screenshot.
  • Settle gated, not timed: the drain before it anchors each document once, so SessionCapturer.replacedDocumentInLastDrain says whether the last action navigated to an unseen document. No → return; yes → waitUntil(readyState === 'complete'), with the body.childElementCount > 0 clause dropped (empty-bodied is then correct, not a guaranteed 8 s timeout). Native pauses 250 ms.
  • backend nearestFrame preferred minimum absolute distance, so a row without its own capture could replay its successor's state; now latest at-or-before, matching the app's rule.
  • Cross-test stamp borrow: the log is run-long, so the next test's first pre-capture landed on the previous test's last-action slot, where the richer merge could replace it (under reloadSession, the post-reload page). It now stamps Date.now() when the scanned timestamp predates #currentTestStartWallTime (0 without per-test hooks, so standalone is unchanged).
  • Screencast: #pollInFlight keeps one shot outstanding (native 1.2 s against a 200 ms interval stacked ~6 deep; a 15 ms command measured 4.5–7.8 s); #pollGeneration invalidates a shot orphaned by stop(); start() claims the generation before its first await so a mid-first-screenshot stop() cannot have the loop armed underneath it.
  • Native example (wdio.native.conf.ts, pnpm demo:wdio:native) — the platform was previously unmeasurable. No APK; APPIUM_HOST/_PORT/_DEVICE.
  • Cleanups: the capture gate's predicate was written twice, now #isActionCommand; ActionSnapshot.command documented as a label, not a key.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Polish (an improvement to an existing feature)
  • Breaking change (existing behavior changes for users)
  • Documentation
  • Internal (build, CI, dependencies, tooling)

Packages touched

  • shared (types and contracts)
  • core (framework-agnostic capture/reporting)
  • elements (published element/snapshot API — @wdio/elements)
  • service (WebdriverIO adapter)
  • nightwatch-devtools (Nightwatch adapter)
  • selenium-devtools (Selenium adapter)
  • selenium-devtools-py (Selenium Python adapter)
  • backend (server)
  • app (UI)
  • script (page-injected runtime)
  • trace (Trace mode)

Notes for reviewers

Screenshots / recordings

@greptile-apps

greptile-apps Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with no new findings and all three previous screencast lifecycle findings fixed.

Findings

  1. P1 CDP startup leaks after stop ▶
  2. P1 Startup can block teardown ▶
  3. P1 Late CDP sessions leak ▶

Summary

This PR restores one pre-action trace capture per action, finalizes the last action separately, corrects frame selection and cross-test timestamp handling, and strengthens screencast lifecycle behavior across the shared core, WebdriverIO, and Selenium integrations.

  • Serializes screencast start and stop while bounding driver handshake operations.
  • Cleans up CDP sessions or sockets that resolve after startup timeouts.
  • Prevents overlapping screenshot polls on serialized native drivers.
  • Adds native Appium demonstration coverage and updates trace-capture documentation.
  • The three earlier screencast findings are addressed: startup and teardown are serialized, awaited startup operations are bounded, and late CDP resources are explicitly detached or closed.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Start[start recording] --> Queue[serialized lifecycle queue]
  Queue --> CDP{CDP available within timeout?}
  CDP -->|yes| Push[collect pushed frames]
  CDP -->|late resource| Cleanup[detach session or close socket]
  CDP -->|no| First[bounded first screenshot]
  First --> Poll[poll with one request in flight]
  Push --> Stop[queued stop]
  Poll --> Stop
  Stop --> Release[stop stream and release resources]
Loading

Reviews (6) · Last reviewed commit: "fix: clean up CDP sessions and sockets t..."

@vishnuv688

Copy link
Copy Markdown
Member

Both PRs it overlapped with are now merged into main (#375 and #378), so it needs a rebase. Two things to watch when you do:

isNativeAppSession moved from service/mobile.ts into shared and now takes capabilities instead of the browser to keep the shared version or the rebase will revert it.
#378 touched the same three service files. It routes the beforeCommand probes straight at the driver to avoid a deadlock on mobile-web Appium. Your PR deletes #markDocument, which it reroutes, so that part just goes away.

@Winify
Winify force-pushed the fix/redundant-trace-capture branch from ecc35da to 22964d6 Compare September 20, 2026 12:46
Comment thread packages/core/src/screencast.ts Outdated
Comment on lines +55 to +57
const cdpOk = await this.tryStartCdp()
if (generation !== this.#pollGeneration) {
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 CDP startup leaks after stop

If stop() runs while tryStartCdp() is awaiting after creating a CDP session or starting the screencast, stop() returns because #isRecording is still false. The generation check then exits without calling tryStopCdp(). This leaves the CDP stream and frame listener active after teardown, so capture work can leak and frames can be appended after stopping or during a later recording.

Knowledge Base Used: Visual artifacts and screencasts

Comment on lines +71 to +77
#enqueue(op: () => Promise<void>): Promise<void> {
const run = this.#queue.then(op, op)
this.#queue = run.then(
() => undefined,
() => undefined
)
return run

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Startup can block teardown

If the service CDP handshake or the polling path's first screenshot never settles, the new serialized queue keeps stop() waiting behind start() indefinitely. Those startup operations have no timeout, so recorder finalization—and potentially the test process—cannot complete. Bound or cancel queued startup so teardown can still finish when the driver wedges.

Knowledge Base Used: Visual artifacts and screencasts

- every driver primitive the handshake awaits is ceilinged at SCREENCAST_HANDSHAKE_TIMEOUT_MS, so a wedged driver falls back to polling instead of parking teardown
- The final capture's alreadyCaptured check tracks the slot it captured instead of scanning by timestamp
- Selenium's CDP ceiling gets its regression tests
@Winify
Winify force-pushed the fix/redundant-trace-capture branch from 52e0217 to 1fb78c4 Compare September 22, 2026 14:46
Comment on lines +84 to +90
const session = await withTimeout<CdpSessionLike | undefined>(
pages[0].createCDPSession(),
SCREENCAST_HANDSHAKE_TIMEOUT_MS,
undefined
)
if (!session) {
return undefined

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Late CDP sessions leak

If createCDPSession() takes longer than five seconds but later succeeds, withTimeout does not cancel it. This code has already returned and fallen back to polling, so the late CDP session is never stored or detached. The Selenium createCDPConnection() path has the same ownership gap. These orphaned attachments remain for the browser session and can interfere with later recording starts. Please retain ownership of the original promise and clean up resources that resolve after the timeout.

@Winify
Winify force-pushed the fix/redundant-trace-capture branch from 25e6bf0 to 3ae55e9 Compare September 22, 2026 15:39
@Winify
Winify force-pushed the fix/redundant-trace-capture branch from 3ae55e9 to 1387285 Compare September 22, 2026 15:56
@Winify
Winify merged commit afc07ca into webdriverio:main Sep 22, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native mobile trace mode costs 40-60s per run in per-action probes

2 participants