TestMultiConnectToSingleBrowser is flaky #861
Comments
|
While working on a PR, I found that this test was flakey. Initially i thought it was due to the changes in the PR, but it turns out that this is a known flakey test. I found that not closing the diff --git a/tests/browser_test.go b/tests/browser_test.go
index 9ec03f8..400f7f6 100644
--- a/tests/browser_test.go
+++ b/tests/browser_test.go
@@ -251,7 +251,7 @@ func TestMultiConnectToSingleBrowser(t *testing.T) {
err = p1.Close(nil)
require.NoError(t, err, "failed to close page #1")
- bctx1.Close()
+ // bctx1.Close()
p2, err := bctx2.NewPage()
require.NoError(t, err, "failed to create page #2")@inancgumus did some debugging and found that adding a sleep helped prevent the issue, so it's likely to be a race condition: "This works all the time ( // bctx1.Close()
time.Sleep(time.Second) // <---
p2, err := bctx2.NewPage()"I made more checks. It's probably because of the browser or we killed the connection before attaching the blank page. And we're late updating/removing our internal contexts."
"I guess the solution is we may want to include some mechanism to ensure the session with the target has not been detached before sending the get frame tree command (or others). Or, alternatively, we could handle the error and create a new session if necessary." |
|
A workaround is here in case we need it: #930 |
|
Update I have an inkling that the issue resides in chrome. The error we get is from chrome which reads "Session with given id not found.". When creating a new page we work with the Sessions are created by chrome on It's also plausible that the Finally, maybe we have missed an important step when we're opening multiple websocket connections against chrome, maybe a flag of some kind. Having looked at puppeteer and playwright, I'm unable to see an obvious setup that would help prevent such an issue. In fact it seems that both playwright hasn't solved it and puppeteer have encountered similar problems with unclear solutions. I've opened an issue with the chromium team to see if they can help us out: https://bugs.chromium.org/p/chromium/issues/detail?id=1463459. Possible workaround When the browser module sees such an error, it retries up to n times before giving up. diff --git a/common/browser_context.go b/common/browser_context.go
index 4322d5a..f09c39b 100644
--- a/common/browser_context.go
+++ b/common/browser_context.go
@@ -4,6 +4,7 @@ import (
"context"
"fmt"
"reflect"
+ "strings"
"time"
"github.com/grafana/xk6-browser/api"
@@ -247,9 +248,20 @@ func (b *BrowserContext) NewCDPSession() api.CDPSession {
func (b *BrowserContext) NewPage() (api.Page, error) {
b.logger.Infof("BrowserContext:NewPage", "bctxid:%v", b.id)
- p, err := b.browser.newPageInContext(b.id)
+ var p *Page
+ var err error
+ p, err = b.browser.newPageInContext(b.id)
if err != nil {
- return nil, fmt.Errorf("creating new page in browser context %s: %w", b.id, err)
+ if !strings.Contains(err.Error(), "Session with given id not found") {
+ return nil, fmt.Errorf("creating new page in browser context %s: %w", b.id, err)
+ }
+
+ b.logger.Infof("BrowserContext:NewPage", "retrying page creation bctxid:%v", b.id)
+
+ p, err = b.browser.newPageInContext(b.id)
+ if err != nil {
+ return nil, fmt.Errorf("creating new page in browser context %s: %w", b.id, err)
+ }
}
var (Branch https://github.com/grafana/xk6-browser/tree/investigate/TestMultiConnectToSingleBrowser |
|
After receiving a response from the chromium team with the issue I raised a few weeks ago, they have come back with a response. TL;DR The msg='-> {"id":4,"sessionId":"169957DD1CBB5704DBC6798D891A6099","method":"Page.getFrameTree"}'
msg='<- {"method":"Target.detachedFromTarget","params":{"sessionId":"A10EDFD7D1B1CD6CFC1C8BBFCCFA2160","targetId":"6A497BD7D0C068592869FEEDE3235124"}}'
msg='<- {"method":"Target.detachedFromTarget","params":{"sessionId":"169957DD1CBB5704DBC6798D891A6099","targetId":"6A497BD7D0C068592869FEEDE3235124"}}'
msg='<- {"method":"Target.detachedFromTarget","params":{"sessionId":"60BE8201E7C7B6136367BB624399D03E","targetId":"6A497BD7D0C068592869FEEDE3235124"}}'
msg='-> {"id":4,"method":"Target.createTarget","params":{"url":"about:blank","browserContextId":"A42B57D4FDDF0A5E3928DC583BAD8A9E"}}'
msg='<- {"id":4,"error":{"code":-32001,"message":"Session with given id not found."}}'There's the request for The next steps are:
At the moment when we make a CDP request (i'm going to work with NewPage as the example here):
Now, when we get As a side, I'm also hoping that this will help resolve the deadlock issue since it's a deadlock in a handler waiting for a response from chrome which is never received, so is that due to an error that is being redirected to the wrong place? |
|
Resolved in #984. |
inancgumus
added
the
next
|
Closed by #1219. |
Investigate why the test fails to create the second page.
There is a data race:
The text was updated successfully, but these errors were encountered: