Skip to content

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

Open
Winify wants to merge 3 commits into
webdriverio:mainfrom
Winify:fix/redundant-trace-capture
Open

Winify wants to merge 3 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: 4/5

The PR should not merge until screencast teardown can complete when an unbounded startup handshake or first screenshot wedges.

Findings

  1. P1 Startup can block teardown
  2. P1 CDP startup leaks after stop

Summary

This PR restores one pre-action trace capture, adds a final-action capture and navigation-gated settling, changes frame selection to latest-at-or-before, and prevents overlapping polling screenshots. It also serializes screencast startup and teardown to address the previously reported CDP lifecycle race.

  • Removes redundant post-action capture and settling work.
  • Preserves final-action and cross-test snapshot attribution.
  • Improves trace-frame selection and native screencast polling behavior.
  • Fixes the previously reported mid-handshake CDP resource leak, but introduces an unbounded teardown wait when startup wedges.

Diagram

sequenceDiagram
    participant Caller
    participant Queue as Recorder queue
    participant Start as startInner
    participant Driver
    participant Stop as stopInner

    Caller->>Queue: start()
    Queue->>Start: begin startup
    Start->>Driver: CDP handshake / first screenshot
    Caller->>Queue: stop()
    Note over Queue,Driver: stop waits behind startup
    alt startup settles
        Driver-->>Start: result
        Start-->>Queue: complete
        Queue->>Stop: release resources
    else driver operation never settles
        Note over Queue,Stop: stop never executes
    end
Loading

Reviews (3) · Last reviewed commit: "chore(changeset): introduce 2 changesets..."

@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

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