Skip to content

[feat]: add timeout support for locator actions & reads - #3024

Open
seanmcguire12 wants to merge 3 commits into
share-operation-context-in-locator-helpersfrom
share-operation-context-in-locator-actions-reads
Open

seanmcguire12 wants to merge 3 commits into
share-operation-context-in-locator-helpersfrom
share-operation-context-in-locator-actions-reads

Conversation

@seanmcguire12

@seanmcguire12 seanmcguire12 commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

why

pr #3023 made locator resolution respect the caller's deadline. the action or read that follows also needs to use the remaining time. otherwise, a stalled browser command can keep the call waiting after its timeout.

what changed

  • passed the same progress through direct, deep, & frame-scoped locator methods, from finding the target through executing the action/read.
  • bounded browser commands for click(), hover(), selectOption(), scrollTo(), sendClickEvent(), centroid(), backendNodeId(), counts, & element reads.
    • counts also pass progress through their separate selector helpers
  • preserved timeout & closed connection/session errors in catches that previously ignored errors or returned zero. ordinary lookup failures keep their existing handling.
  • used bounded cleanup for temporary browser references. cleanup failures cannot replace the action's error, & cleanup crossing the deadline cannot turn the call into success.
  • kept click events in their existing order, without waiting for each response before sending the next event. expiry prevents further dispatch.

note:

progress remains optional during migration. fill, typing, highlighting, & uploads come in a subsequent PR

test plan

added tests in locatorActions.test.ts using shared setup, controlled time, & held browser responses to verify:

  • delegates, actions, reads, & counting helpers receive the same progress.
  • time spent resolving the target leaves only the remaining budget for execution.
  • stalled commands reject at the deadline; late responses cannot resume actions. an already-expired call sends no action commands.
  • click events retain their order & are sent before earlier responses arrive. dispatch stops if the deadline passes during the sequence.
  • timeout & closure errors propagate through scrolling & counting. valid empty reads still return false, zero, or an empty string.
  • cleanup failures preserve the action error, cleanup crossing the deadline reports a timeout, & stalled cleanup cannot hold the outer caller past expiry.
  • timing out one call leaves a concurrent unlimited call on the same locator free to finish.

Summary by cubic

Makes locator actions and reads respect the caller's timeout so a stalled browser command can no longer hold a call past its deadline.

  • Threads the same progress through direct, deep, and frame-scoped locator methods, from target resolution through execution.
  • Bounds browser commands for click, hover, selectOption, scrollTo, sendClickEvent, centroid, backendNodeId, count, and element reads; counts also pass progress through their selector helpers.
  • Preserves timeout and closed connection/session errors in catches that previously ignored them or returned zero.
  • Bounds cleanup of temporary browser references so cleanup failures can't replace the action's error, and cleanup crossing the deadline can't turn the call into success; cleanup is no longer awaited when an action is expiring.
  • Keeps click events in their existing order without waiting for each response; expiry stops further dispatch.
  • Progress remains optional during migration; fill, typing, highlighting, and uploads come in a later PR.

Written for commit a938b81. Summary will update on new commits.

Review in cubic

@changeset-bot

changeset-bot Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a938b81

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@seanmcguire12
seanmcguire12 changed the base branch from add-locator-operation-context to share-operation-context-in-locator-helpers September 23, 2026 20:57

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 14 files

Architecture diagram
sequenceDiagram
    participant Client
    participant Delegate as Locator Delegate (Direct/Deep/Frame)
    participant Locator as Locator
    participant Progress as Progress Context
    participant Registry as ExecutionContextRegistry
    participant Resolver as SelectorResolver
    participant CDP as CDP Session

    Note over Client,CDP: Locator Action/Read with Timeout Boundary

    Client->>Delegate: click()/hover()/count()/etc.(..., progress)
    Delegate->>Delegate: real(progress)
    Delegate->>Progress: throwIfStopped()

    alt Deep or Frame Locator
        Delegate->>Locator: resolveLocatorWithHops(page, root, selector, progress)
        Locator->>Registry: waitForLocatorWorld(session, frameId, 1000, progress)
        Registry->>CDP: Runtime.enable bounded
        Registry->>CDP: Runtime.evaluate inspector probe
        CDP-->>Registry: Locator world ready or fallback
        Registry-->>Locator: Context ID
        Locator-->>Delegate: Real Locator
    end

    Delegate->>Locator: action/read(..., progress)
    Locator->>Progress: throwIfStopped()
    Locator->>Resolver: resolveNode/resolveAll/count(query, progress)
    Resolver->>Registry: waitForLocatorWorld(..., progress)
    Registry-->>Resolver: Context ID

    loop Per element index resolve
        Resolver->>CDP: Runtime.evaluate bounded via runLocatorStep
        CDP-->>Resolver: objectId
    end

    alt Element found
        Resolver-->>Locator: ResolvedNode
    else Timeout during resolve
        Resolver->>Progress: throwIfStopped()
        Progress-->>Resolver: TimeoutError
        Resolver->>CDP: Runtime.releaseObject bounded cleanup
        Resolver-->>Locator: Reject with timeout
    end

    Locator->>CDP: DOM.scrollIntoViewIfNeeded / DOM.getBoxModel / Input.dispatchMouseEvent each bounded

    alt Command succeeds within budget
        CDP-->>Locator: Result
        Locator->>CDP: Runtime.releaseObject bounded cleanup
        Locator-->>Delegate: Result
        Delegate-->>Client: Action/read result
    else Command stalls past deadline
        Progress->>CDP: Abort signal fires
        CDP-->>Locator: Late response ignored
        Locator->>Progress: throwIfStopped throws TimeoutError
        Locator->>CDP: Runtime.releaseObject cleanup bounded
        Locator-->>Delegate: Reject
        Delegate-->>Client: TimeoutError
    end

    Note over Progress,CDP: Closing CDP errors propagate and ordinary lookup failures keep prior behavior
    Note over Progress,CDP: Cleanup failures never replace action errors and timeout during cleanup reports timeout
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/extension/understudy/selectorResolver.ts
Comment thread packages/extension/understudy/selectorResolver.ts
@seanmcguire12
seanmcguire12 force-pushed the share-operation-context-in-locator-actions-reads branch from b6a1ee8 to 77b5510 Compare September 23, 2026 21:11
@seanmcguire12
seanmcguire12 added this pull request to stack #3025 September 23, 2026 21:12

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 7 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.
Architecture diagram
sequenceDiagram
    participant Caller as Caller Code
    participant Delegate as Locator Delegate (Deep/Frame)
    participant Locator as Locator
    participant Progress as Progress Context
    participant Resolver as Selector Resolver
    participant Session as CDP Session
    participant Browser as Browser Target

    Note over Caller,Browser: Locator Action with Timeout (Happy Path)

    Caller->>Delegate: click(options, progress)
    Delegate->>Delegate: real(progress)
    Delegate->>Locator: resolveNode(progress)
    Locator->>Progress: throwIfStopped()
    Progress-->>Locator: OK

    Locator->>Session: Runtime.callFunctionOn (resolve selector)
    Session-->>Locator: objectId

    Locator->>Progress: remainingMs (check budget)

    alt Budget remaining
        Locator->>Session: DOM.scrollIntoViewIfNeeded
        Session-->>Locator: OK
        Locator->>Session: DOM.getBoxModel
        Session-->>Locator: box model
        Locator->>Progress: remainingMs (check budget)

        loop For each click event
            Locator->>Progress: throwIfStopped (before dispatch)
            alt Still within deadline
                Locator->>Session: Input.dispatchMouseEvent (fire-and-forget)
            else Expired
                Locator-->>Delegate: throw TimeoutError
                Delegate-->>Caller: throw TimeoutError
            end
        end

        Locator->>Progress: cleanup(releaseObject)
        Progress->>Session: Runtime.releaseObject (bounded)
        Session-->>Locator: OK
        Locator-->>Delegate: Success
        Delegate-->>Caller: Success
    else Expired
        Locator-->>Delegate: throw TimeoutError
        Delegate-->>Caller: throw TimeoutError
    end

    Note over Caller,Browser: Stalled Command Path (Unhappy Path)

    Caller->>Locator: count(progress)
    Locator->>Progress: throwIfStopped()
    Locator->>Session: Runtime.enable
    Locator->>Session: DOM.enable
    Locator->>Resolver: count(query, progress)
    Resolver->>Resolver: waitForLocatorWorld(session, frameId, 1000, progress)

    alt Stalled command (never responds)
        Resolver->>Session: Runtime.evaluate (held)
        Note over Resolver,Progress: Deadline reached while waiting
        Progress-->>Resolver: TimeoutError
        Resolver-->>Locator: throw TimeoutError
        Locator-->>Caller: throw TimeoutError (late response ignored)
    else CDP connection closed
        Session-->>Resolver: CDP closed error
        Resolver->>Resolver: throwIfStopped(), isCdpClosedError()
        Resolver-->>Locator: throw original error
    end

    Note over Caller,Browser: Cleanup Path with Expired Deadline

    Caller->>Locator: hover(progress)
    Locator->>Session: resolveNode (completes)
    Locator->>Session: DOM.getBoxModel (completes)
    Locator->>Progress: cleanup(releaseObject)
    alt Cleanup stale (deadline reached)
        Progress->>Progress: Mark as expiring
        Progress-->>Locator: throw TimeoutError (not awaiting release)
        Locator-->>Caller: throw TimeoutError
    else Cleanup fails
        Session-->>Progress: release failure
        Locator->>Progress: throwIfStopped()
        Locator-->>Caller: throw original action error (cleanup error swallowed)
    end

    Note over Caller,Browser: Deep Locator with Shared Budget

    Caller->>Delegate: selectOption(values, progress)
    Delegate->>Delegate: real(progress) (takes time)
    Delegate->>Locator: selectOption(values, progress)
    Note over Locator,Progress: Remaining budget = original timeout - resolution time
    Locator->>Progress: remainingMs (reduced budget)
    alt Enough budget
        Locator->>Session: execute action commands
        Locator-->>Delegate: Success
    else Budget exhausted
        Locator-->>Delegate: throw TimeoutError
        Delegate-->>Caller: throw TimeoutError
    end
Loading

Re-trigger cubic

@seanmcguire12
seanmcguire12 force-pushed the share-operation-context-in-locator-actions-reads branch from 9c695a4 to a938b81 Compare September 25, 2026 22:08

This branch has not been deployed

No deployments
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.

1 participant