Add retry to newAction-based locator APIs by janHildebrandt98 · Pull Request #5657 · grafana/k6

janHildebrandt98 · 2026-02-12T19:56:18Z

What?

Wrapping newAction() on ElementHandle with a retry function, similar to what was done in #5111.
Changed frame.BoundingBox() to call public handle.BoundingBox() because it neatly translates the errors in retryable errors.

Why?

Parallel actions on a target element may lead to it being hidden/detached after initial actioanbility checks were performed but before/during our call to the injected script. This can lead to transient errors.
This PR introduces a retry mechanic to succeed in case of transient errors because of the retry and else fail with a timeout.

Checklist

I have performed a self-review of my code.
I have commented on my code, particularly in hard-to-understand areas.
I have added tests for my changes.
I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

I have added the correct milestone and labels to the PR.
I have updated the release notes: link
I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

closes #5174

janHildebrandt98 · 2026-02-12T20:07:17Z

@ankur22
I have some questions left:

Do we need an option to not retry like it is available in retryPointerAction()?
I used the public .BoundingBox() because it neatly translates the errors. Don't know if thats a smell?
Should we retry force = true calls?
Something to consider: I think we are performing actionability checks in many places right now: frame.NewAction(), handle.NewAction() and sometimes also in the injected script. I wonder if it would make sense to centralize it in the injected script since thats the latest and hence it minimizes the chance for a change on the element between check and logic execution.

If you have an idea for a non-flaky test, that would also be appreciated!

ankur22 · 2026-02-17T09:26:33Z

@janHildebrandt98 thanks for the PR!

Do we need an option to not retry like it is available in retryPointerAction()?

If you are referring to the following:

		// Only locator based APIs should retry.
		if !opts.retry {
			return res, err
		}

Then yes, i think we need the same. The code from locator and non-locator based APIs use the same newAction. I would like the locator based APIs to retry, but no the non-locator based APIs. Does that make sense? I might be wrong in assuming that non-locator based APIs do not also need to retry, basically i think we should match the Playwright behaviour.

I used the public .BoundingBox() because it neatly translates the errors. Don't know if thats a smell?

No strong feeling here, happy to leave it in, unless the other reviewer spots something.

Should we retry force = true calls?

Good question, i'm not sure. What does Playwright do?

Something to consider: I think we are performing actionability checks in many places right now: frame.NewAction(), handle.NewAction() and sometimes also in the injected script. I wonder if it would make sense to centralize it in the injected script since thats the latest and hence it minimizes the chance for a change on the element between check and logic execution.

Yeah, this is a good observation. If you're interested in working out a better way then please go ahead, but in a new PR. I'm not sure why this is setup in this way, there could be a good reason for having multiple actionability checks, I wasn't around for when that was initially implemented and there's no clue in the git history.

ankur22 · 2026-02-17T09:30:01Z

+		if strings.Contains(err.Error(), "element is not attached to the DOM") {
+			return ErrElementNotAttachedToDOM
+		}


What's the reason for this change?

I misunderstood how errors come out of eval(),,,
Thanks for pointing it out.
Removed.

janHildebrandt98 · 2026-02-20T20:03:45Z

Hi @ankur22,

I conducted some research into playwrights codebase and I found that matching it exactly will be difficult.

The retry logic from this PR mainly affects focus() and methods calling it—retrying when the target is detached, hoping it gets re-attached.

Playwright handles this differently: it re-runs the entire selector, potentially resolving to a new element. It cleanly separates locator retries (does the element exist?) from actionability retries, with locator retries wrapping actionability retries wrapping the actual logic.

Matching this in our codebase is difficult because selector execution happens before newAction(), so retrying the selector would require some restructuring.

What we can do with minimal impact to the code:
Match Playwright as close as possible:

Retry as implemented in this PR on the same element without re-running the selector. Force = true is not retried by playwright.
Playwright does not care from where the function is called: locator or elsewhere, does not influence the retry behavior
Ill also adjust some more details to match Playwrioght more closely.

SInce I'll be gone for the next week, you can expect the remaining changes to this PR on the following week.

janHildebrandt98 · 2026-03-11T15:36:08Z

Hi @ankur22,
please have a look at the changes! I tried to match Playwright as close as possible without making too many changes.
Thank you for your time.

ankur22

LGTM 🚀

ankur22 · 2026-03-11T16:04:24Z

+		var res any
+		var err error
+
+		if retry && !force {


Why !force?

Playwright won't retry actionability when force = true.
It is not entirely comparable to our implementation because of what I tried to explain in my earlier comment in this PR but i thought doing it this way is as close as we can get right now.

janHildebrandt98 · 2026-03-12T07:32:13Z

Hi @ankur22,
I adjusted the returned error, feel free to approve again 👍
Thanks a lot for your time.

inancgumus

LGTM, @janHildebrandt98, except for two minor issues.

Thanks for your contribution 🙇

inancgumus · 2026-03-12T22:08:48Z


 func (h *ElementHandle) newAction(
-	states []string, fn elementHandleActionFunc, force, noWaitAfter bool, timeout time.Duration,
+	states []string, fn elementHandleActionFunc, force bool, retry bool, noWaitAfter bool, timeout time.Duration,


Successive primitive types make it easier to introduce bugs (i.e., force bool, retry bool, noWaitAfter bool). They also make it harder for the caller to understand. Could you add and use Retry, NoRetry (these names are suggestions, and you can pick whatever name you want) constants?

Great idea. Will adjust it soon.

@inancgumus I added the constants. Please have another look.

inancgumus

Good stuff. Thanks! LGTM.

janHildebrandt98 · 2026-03-14T12:20:36Z

Hi @ankur22,
sorry to bother, but you need to approve a third time :)
Thanks a lot!

janHildebrandt98 · 2026-03-18T09:36:53Z

@ankur22
Quick ping to have another look at this so we can close the PR :)
Have a nice day!

janHildebrandt98 · 2026-03-23T15:23:55Z

@ankur22
Would really appreciate your approval!
Have a nice one

janHildebrandt98 · 2026-03-26T17:55:01Z

Just wanted to say I really enjoyed collaborating with you on this one.
Thank you for your time.

* Add retry func and test * Refactor retry methods to reuse shared logic * Cleanup * more cleanup * make linter happy * Fixx error masking * remove unnecessary error check * Dont retry for BoundingBox(), add test * Return errElementNotVisible and NotAttachedToDom from ErrFromDOMError * Return correct error * Introduce constants for retry switch

ankur22 reviewed Feb 17, 2026

View reviewed changes

janHildebrandt98 force-pushed the fix/retry-locator-apis branch 2 times, most recently from 507768a to f039b47 Compare March 10, 2026 17:11

janHildebrandt98 commented Mar 10, 2026

View reviewed changes

Comment thread internal/js/modules/k6/browser/common/element_handle.go

janHildebrandt98 commented Mar 10, 2026

View reviewed changes

Comment thread internal/js/modules/k6/browser/common/frame.go Outdated

janHildebrandt98 commented Mar 10, 2026

View reviewed changes

Comment thread internal/js/modules/k6/browser/browser/element_handle_mapping.go

janHildebrandt98 marked this pull request as ready for review March 11, 2026 15:34

janHildebrandt98 requested a review from a team as a code owner March 11, 2026 15:34

janHildebrandt98 requested review from inancgumus and joanlopez and removed request for a team March 11, 2026 15:34

ankur22 previously approved these changes Mar 11, 2026

View reviewed changes

janHildebrandt98 dismissed ankur22’s stale review via f7474fb March 12, 2026 07:30

janHildebrandt98 force-pushed the fix/retry-locator-apis branch from f7474fb to 4f00eeb Compare March 12, 2026 07:36

janHildebrandt98 requested a review from ankur22 March 12, 2026 07:37

ankur22 previously approved these changes Mar 12, 2026

View reviewed changes

janHildebrandt98 temporarily deployed to azure-trusted-signing March 12, 2026 09:27 — with GitHub Actions Inactive

janHildebrandt98 temporarily deployed to azure-trusted-signing March 12, 2026 09:29 — with GitHub Actions Inactive

inancgumus requested changes Mar 12, 2026

View reviewed changes

janHildebrandt98 dismissed ankur22’s stale review via da1b127 March 13, 2026 16:42

janHildebrandt98 added 5 commits March 13, 2026 17:42

Add retry func and test

448b5a5

Refactor retry methods to reuse shared logic

3562c65

Cleanup

c099a2d

more cleanup

eae7c5a

make linter happy

5e9b6f6

janHildebrandt98 and others added 6 commits March 13, 2026 17:42

Fixx error masking

4fcc4fe

remove unnecessary error check

e1cb471

Dont retry for BoundingBox(), add test

0565293

Return errElementNotVisible and NotAttachedToDom from ErrFromDOMError

406b6a3

Return correct error

1a929b2

Introduce constants for retry switch

71dff07

janHildebrandt98 force-pushed the fix/retry-locator-apis branch from da1b127 to 71dff07 Compare March 13, 2026 16:43

janHildebrandt98 requested a review from inancgumus March 13, 2026 16:53

inancgumus approved these changes Mar 14, 2026

View reviewed changes

janHildebrandt98 temporarily deployed to azure-trusted-signing March 14, 2026 10:15 — with GitHub Actions Inactive

janHildebrandt98 temporarily deployed to azure-trusted-signing March 14, 2026 10:16 — with GitHub Actions Inactive

janHildebrandt98 mentioned this pull request Mar 14, 2026

Align selector retry semantics with Playwright when element becomes detached #5739

Open

janHildebrandt98 requested a review from ankur22 March 18, 2026 09:36

ankur22 approved these changes Mar 26, 2026

View reviewed changes

ankur22 merged commit 9a09a9a into grafana:master Mar 26, 2026
51 of 52 checks passed

ankur22 added this to the v2.0.0 milestone Mar 26, 2026

janHildebrandt98 deleted the fix/retry-locator-apis branch April 2, 2026 17:25

BrewTestBot mentioned this pull request May 11, 2026

k6 2.0.0 Homebrew/homebrew-core#281992

Merged

mstoykov mentioned this pull request May 15, 2026

[v1.8.0 backport] Add retry to newAction-based locator APIs (browser) #5994

Merged

8 tasks

Uh oh!

Conversation

janHildebrandt98 commented Feb 12, 2026 • edited by ankur22 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

Checklist

Checklist: Documentation (only for k6 maintainers and if relevant)

Related PR(s)/Issue(s)

Uh oh!

janHildebrandt98 commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ankur22 commented Feb 17, 2026

Uh oh!

ankur22 Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

janHildebrandt98 Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

janHildebrandt98 commented Feb 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

janHildebrandt98 commented Mar 11, 2026

Uh oh!

ankur22 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ankur22 Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

janHildebrandt98 Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

janHildebrandt98 commented Mar 12, 2026

Uh oh!

inancgumus left a comment

Choose a reason for hiding this comment

Uh oh!

inancgumus Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

janHildebrandt98 Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

janHildebrandt98 Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

inancgumus left a comment

Choose a reason for hiding this comment

Uh oh!

janHildebrandt98 commented Mar 14, 2026

Uh oh!

janHildebrandt98 commented Mar 18, 2026

Uh oh!

janHildebrandt98 commented Mar 23, 2026

Uh oh!

Uh oh!

janHildebrandt98 commented Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

janHildebrandt98 commented Feb 12, 2026 •

edited by ankur22

Loading

janHildebrandt98 commented Feb 12, 2026 •

edited

Loading