fix(executor): retry's maxRetries counts retries, not attempts - #182
Merged
omnarayan merged 2 commits intoOct 1, 2026
Merged
Conversation
10 of 16 tasks
bulatgaleev
force-pushed
the
upstream-pr/retry-counts-retries
branch
from
September 29, 2026 18:31
68fce14 to
612fb24
Compare
This was referenced Sep 29, 2026
bulatgaleev
force-pushed
the
upstream-pr/retry-counts-retries
branch
from
September 30, 2026 01:41
612fb24 to
102dfe3
Compare
The WDA driver reads an element's name, rect, text and displayed in parallel, and a tap looks an element up four ways at once. The client used Go's default transport, which keeps two idle connections per host, so every such burst closed two connections and opened two new ones. On a real iPhone reached through a forward (SSH, then iproxy over USB) a new connection costs about 300 ms, and new ones opened together fail at once with EOF. In one 44-flow run, 297 of 996 element bursts lost exactly two reads (the two new connections), 2 lost one, and none lost a read on a kept connection: 976 EOFs, each sent again by the dropped-connection retry. WebDriverAgent does not close an idle keep-alive connection (FBHTTPServer exempts them from its reaper), so a stale reuse was not the cause. The client now keeps up to eight idle connections per host. In the new tests, 50 bursts of four reads open 4 connections instead of 102, and through a server that drops every new connection past the first four, no read fails (the old client lost 23 reads there, after 196 dropped connections).
Maestro runs a retry's commands once and then up to maxRetries more times:
`(maxRetries?.toIntOrNull() ?: 1).coerceAtMost(3)`, then
`while (attempt <= maxRetries)` from 0 (Orchestra.kt:934-955, with
MAX_RETRIES_ALLOWED at 1844 and the "1" default at YamlFluentCommand.kt:614).
So `maxRetries: 1` is two attempts, the default is two, and no retry runs
more than four times. The value is evaluated first, so ${...} works
(Commands.kt:986), and anything that is not an integer counts as 1.
The runner ran exactly maxRetries attempts, three when unset, with no cap,
so a flow written with `maxRetries: 1` never retried at all. It also failed
the step when maxRetries was not an integer. The inline and the file form
of retry now count as Maestro does. A negative maxRetries runs no attempt
and passes, since Maestro's loop never starts. A value that is not an
integer is logged and read as 1.
bulatgaleev
force-pushed
the
upstream-pr/retry-counts-retries
branch
from
September 30, 2026 14:49
102dfe3 to
48922eb
Compare
Contributor
|
Thank you @bulatgaleev. We checked this against Maestro's source too ( This changes behaviour on every driver, not only WDA. A flow with no Merging into |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Maestro runs a
retryblock's commands once, then up tomaxRetriesmore times:(maxRetries?.toIntOrNull() ?: 1).coerceAtMost(3), thenwhile (attempt <= maxRetries)from 0 (Orchestra.kt). SomaxRetries: 1is two attempts, the default is two, and no retry runs more than four times. maestro-runner ran exactlymaxRetriesattempts, three when unset and with no cap, so a flow written withmaxRetries: 1never retried, and it failed the step whenmaxRetrieswas not an integer. Both forms ofretrynow count as Maestro does.Type of Change
Changes Made
retryrun the commands once and then up tomaxRetriesmore times, capped at 3, with a default of 1.maxRetriesruns no attempt and passes, since Maestro's loop never starts.TestExecuteRetry_InvalidMaxRetries, which expected that to fail the step, is replaced by tests of the new reading.pkg/executor/retry_attempts_test.go, and a CHANGELOG.md entry.Related Issues
No existing issue found.
Testing
go test ./pkg/executor/go vet ./...andmake fmt-checkpass (run on the top of the stack, fix(wda): checked selectors work on iOS #185, which contains all five changes)make testas a whole: not run, because its device tests drive whatever device is attached to the machinemake lint: the Makefile has nolinttarget, and the lintersmake checkruns (staticcheck, revive, errcheck, nilaway, gosec) are not installed hereChecklist
Additional Notes
A flow that relied on the old count now retries as it does under Maestro:
maxRetries: 1gets its second attempt, and an unsetmaxRetriesgets two attempts instead of three.Stack 2 of 5: #181 → #182 → #183 → #184 → #185. Merge in that order. This branch is built on #181, so it also contains its commit. This PR's own change is the top commit,
48922eb. Once the PRs below it merge, the rest of the diff disappears, and nothing conflicts. All five sit onmainatfe3dd53(the 1.1.28 release).