From 17d9d27b935ea75f4c57d36fd289661e163129c9 Mon Sep 17 00:00:00 2001 From: Bulat Galeev Date: Tue, 29 Sep 2026 13:18:10 +0400 Subject: [PATCH] fix(wda): keep enough connections open for four requests at once 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). --- CHANGELOG.md | 3 + pkg/driver/wda/client.go | 23 ++++- pkg/driver/wda/kept_connections_test.go | 115 ++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 pkg/driver/wda/kept_connections_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e9aca020..7862da16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **WDA keeps enough idle connections for its own parallel reads.** The driver reads an element's name, rect, text and displayed at once, and a tap looks an element up four ways at once, but Go's default transport keeps two idle connections per host, so every burst closed two connections and opened two new ones. Through a forwarded port to a physical iPhone, new connections opened together fail with EOF and are sent again, which costs time on every step. The WDA client now keeps up to eight. + ## [1.1.28] - 2026-09-30 This release is about **running a Maestro suite and getting Maestro's answer**. Most of it came from running real Maestro suites (DuckDuckGo Android and iOS, React Native's RNTester, React Navigation) side by side with Maestro and fixing each place the two disagreed. The largest of those is selector matching: text and id selectors now match the whole value, as Maestro does, which is a behaviour change — see below. `--driver devicelab` on iOS now runs a new agent, on simulators and on real iPhones. DeviceLab becomes the default Android driver. Every flow gets `DEVICE_UDID`, `-e` stops splitting values at commas, and the Appium driver closes a long list of gaps with the native drivers. diff --git a/pkg/driver/wda/client.go b/pkg/driver/wda/client.go index 56a0d35b..5078bc4e 100644 --- a/pkg/driver/wda/client.go +++ b/pkg/driver/wda/client.go @@ -34,11 +34,32 @@ func NewClient(port uint16) *Client { return &Client{ baseURL: fmt.Sprintf("http://127.0.0.1:%d", port), httpClient: &http.Client{ - Timeout: 60 * time.Second, + Timeout: 60 * time.Second, + Transport: newWDATransport(), }, } } +// wdaIdleConnsPerHost is how many idle connections the client keeps open to WebDriverAgent. +// The driver sends up to four requests at once (an element's name, rect, text and displayed; +// a tap's four lookups), and Go keeps two idle connections per host by default, so every such +// burst closed two connections and opened two new ones. On a real iPhone reached through a +// forward (SSH, then iproxy), a new connection took about 300 ms, and the two opened together +// failed at once with EOF in 297 of 996 bursts of one 44-flow run, always both of them and +// never a kept connection. Kept for every request of a burst, no new ones are needed. +const wdaIdleConnsPerHost = 8 + +// newWDATransport is Go's default transport with room to keep a whole burst's connections. +func newWDATransport() *http.Transport { + base, ok := http.DefaultTransport.(*http.Transport) + if !ok { + base = &http.Transport{Proxy: http.ProxyFromEnvironment} + } + t := base.Clone() + t.MaxIdleConnsPerHost = wdaIdleConnsPerHost + return t +} + // Session management // CreateSession creates a new WDA session. diff --git a/pkg/driver/wda/kept_connections_test.go b/pkg/driver/wda/kept_connections_test.go new file mode 100644 index 00000000..ea0b8652 --- /dev/null +++ b/pkg/driver/wda/kept_connections_test.go @@ -0,0 +1,115 @@ +package wda + +import ( + "fmt" + "net" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/devicelab-dev/maestro-runner/pkg/core" +) + +// keptConnServer answers an element's four property reads, each after a short delay so the +// four of a burst are in flight together, as they are on a phone. It counts the connections +// it accepted, and with dropNewAfter > 0 closes, without an answer, every new connection past +// that many: a forward whose new connections fail, while kept ones work. +type keptConnServer struct { + server *httptest.Server + accepted int32 + dropped int32 + dropNewAfter int32 +} + +func newKeptConnServer(t *testing.T, dropNewAfter int32) *keptConnServer { + t.Helper() + es := &keptConnServer{dropNewAfter: dropNewAfter} + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(20 * time.Millisecond) + switch { + case strings.HasSuffix(r.URL.Path, "/rect"): + jsonResponse(w, map[string]interface{}{"value": map[string]interface{}{"x": 10, "y": 20, "width": 100, "height": 40}}) + case strings.HasSuffix(r.URL.Path, "/displayed"): + jsonResponse(w, map[string]interface{}{"value": true}) + default: + jsonResponse(w, map[string]interface{}{"value": "Continue"}) + } + }) + es.server = httptest.NewUnstartedServer(handler) + es.server.Config.ConnState = func(c net.Conn, state http.ConnState) { + if state != http.StateNew { + return + } + n := atomic.AddInt32(&es.accepted, 1) + if es.dropNewAfter > 0 && n > es.dropNewAfter { + atomic.AddInt32(&es.dropped, 1) + _ = c.Close() + } + } + es.server.Start() + t.Cleanup(es.server.Close) + return es +} + +// driverFor is a driver whose client is built the way the runner builds it. +func (es *keptConnServer) driverFor(t *testing.T) *Driver { + t.Helper() + _, portText, err := net.SplitHostPort(strings.TrimPrefix(es.server.URL, "http://")) + if err != nil { + t.Fatalf("server address %q: %v", es.server.URL, err) + } + port, _ := strconv.Atoi(portText) + client := NewClient(uint16(port)) + client.baseURL = es.server.URL // the listener is on 127.0.0.1, not localhost's first address + client.sessionID = "s1" + return &Driver{client: client, info: &core.PlatformInfo{Platform: "ios", ScreenWidth: 390, ScreenHeight: 844}} +} + +// Fifty elements' reads, four at a time, open four connections and then keep them, instead of +// closing two and opening two for every element. +func TestElementReadsKeepTheirConnections(t *testing.T) { + es := newKeptConnServer(t, 0) + d := es.driverFor(t) + for i := 0; i < 50; i++ { + if _, err := d.getElementInfo(fmt.Sprintf("E%d", i)); err != nil { + t.Fatalf("element %d: %v", i, err) + } + } + if n := atomic.LoadInt32(&es.accepted); n > 4 { + t.Errorf("50 bursts of four reads opened %d connections, want at most 4", n) + } +} + +// Where a new connection fails (the phone's forward), only the first burst opens any, so the +// reads after it never meet a failing one. +func TestElementReadsSurviveAForwardWhoseNewConnectionsFail(t *testing.T) { + es := newKeptConnServer(t, 4) + d := es.driverFor(t) + var mu sync.Mutex + failed := 0 + for i := 0; i < 50; i++ { + if _, err := d.getElementInfo(fmt.Sprintf("E%d", i)); err != nil { + mu.Lock() + failed++ + mu.Unlock() + } + } + if dropped := atomic.LoadInt32(&es.dropped); dropped != 0 || failed != 0 { + t.Errorf("after the first burst, %d new connections were needed and dropped, and %d reads failed; want none", dropped, failed) + } +} + +func TestTheWDATransportKeepsABurstsConnections(t *testing.T) { + tr := newWDATransport() + if tr.MaxIdleConnsPerHost < 4 { + t.Errorf("MaxIdleConnsPerHost = %d, fewer than the driver's four requests at once", tr.MaxIdleConnsPerHost) + } + if tr.Proxy == nil || tr.DialContext == nil { + t.Error("the WDA transport lost the default transport's proxy or dialer") + } +}