grafana / xk6-browser

k6 extension that adds support for browser automation and end-to-end web testing via the Chrome Devtools Protocol
https://grafana.com/docs/k6/latest/javascript-api/k6-experimental/browser/
GNU Affero General Public License v3.0
327 stars 41 forks source link

Initiate internal goroutines after a component tree is ready #1247

Open inancgumus opened 3 months ago

inancgumus commented 3 months ago

What?

The browser module might have race conditions because it sets the internal components to receive events before their parent fully completes their initialization. In that case, the event system listens to incoming CDP events and starts reacting to those events with internal components that haven't completed the initialization step.

For example:

Here, NewFrameSession calls NewNetworkManager, which calls NetworkManager.initEvents:

func (m *NetworkManager) initEvents() {
    chHandler := make(chan Event)
    m.session.on(m.ctx, []string{
        cdproto.EventNetworkLoadingFailed,
        cdproto.EventNetworkLoadingFinished,
        cdproto.EventNetworkRequestWillBeSent,
        cdproto.EventNetworkRequestServedFromCache,
        cdproto.EventNetworkResponseReceived,
        cdproto.EventFetchRequestPaused,
        cdproto.EventFetchAuthRequired,
    }, chHandler)

    go func() {
        for m.handleEvents(chHandler) {
        }
    }()
}

However, NewNetworkManager returns before the FrameSession initialization has finished. This might answer the question here. Before the inits are completed, we might start to receive events.

So how does a page end up with a nil mainFrame, or how does the mainFrame end up with a nil networkManager?

Why?

To prevent race conditions.

How?

A better approach might be to start event loops (internal goroutines (e.g., NetworkManager.handleEvents)) once we initialize all the components in the same tree (parent-child relationship).

Tasks

### Tasks
- [ ] Subissue
- [ ] PR
- [ ] Update the k6 release notes

Related PR(s)/Issue(s)

No response

ankur22 commented 1 month ago

This comment links to an issue which is related to the refactoring of internals to better schedule them to avoid race conditions and other issues.

While investigating https://github.com/grafana/xk6-browser/issues/827, I eventually found that the issue was related to the ordering of the CDP requests we make when a new tab is opened after clicking on a link on the original page.

The following are the CDP requests that are made when a new page is created:

-> {"id":5,"method":"Target.createTarget","params":{"url":"about:blank","browserContextId":"F673CD0CD1376255A44A54650C2B2226"}}
-> {"id":6,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Target.setAutoAttach","params":{"autoAttach":true,"waitForDebuggerOnStart":true,"flatten":true}}
-> {"id":7,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Browser.getWindowForTarget","params":{"targetId":"A8D91F407737AEA6DB82462AAE0FD169"}}
-> {"id":8,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.enable"}
-> {"id":9,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.getFrameTree"}
-> {"id":10,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Runtime.evaluate","params":{"expression":"window.k6SpanId = '0000000000000000';","awaitPromise":true}}
-> {"id":11,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Log.enable"}
-> {"id":12,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Runtime.enable"}
-> {"id":13,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Target.setAutoAttach","params":{"autoAttach":true,"waitForDebuggerOnStart":true,"flatten":true}}
-> {"id":14,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.setLifecycleEventsEnabled","params":{"enabled":true}}
-> {"id":15,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.createIsolatedWorld","params":{"frameId":"A8D91F407737AEA6DB82462AAE0FD169","worldName":"__k6_browser_utility_world__","grantUniveralAccess":true}}
-> {"id":16,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Page.addScriptToEvaluateOnNewDocument","params":{"source":"//# sourceURL=__xk6_browser_evaluation_script__","worldName":"__k6_browser_utility_world__"}}
-> {"id":17,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Network.enable","params":{}}
-> {"id":18,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setDeviceMetricsOverride","params":{"width":1280,"height":720,"deviceScaleFactor":1,"mobile":false,"screenWidth":1280,"screenHeight":720,"screenOrientation":{"type":"landscapePrimary","angle":90}}}
-> {"id":19,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Browser.setWindowBounds","params":{"windowId":1994774730,"bounds":{"width":1280,"height":799}}}
-> {"id":20,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setLocaleOverride","params":{"locale":"en-US"}}
-> {"id":21,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setEmulatedMedia","params":{"media":"screen","features":[{"name":"prefers-color-scheme","value":"light"},{"name":"prefers-reduced-motion","value":""}]}}
-> {"id":22,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setFocusEmulationEnabled","params":{"enabled":true}}
-> {"id":23,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Emulation.setUserAgentOverride","params":{"userAgent":"","acceptLanguage":"en-US"}}
-> {"id":24,"sessionId":"A7BD3279F92854D7C831096EFC2A3003","method":"Runtime.runIfWaitingForDebugger"}

They are ran sequentially one after the other. When a new page is opened after clicking a link (with _blank as the target) k6 browser will attempt to run these requests. Unfortunately some of these CDP requests block indefinitely (such as Network.enable) until Runtime.runIfWaitingForDebugger is called. So there are two issues with our current approach:

  1. We're calling Runtime.runIfWaitingForDebugger too late.
  2. We're performing this sequentially.

Playwright on the other hand run these CDP requests concurrently, which queues the CDP requests up in Chrome which it will handle once it receives the Runtime.runIfWaitingForDebugger.

It might be a good idea to refactor how we create a new page so that we work with errgroup to perform all the necessary CDP requests to mimic PW's create page behaviour.