grafana / xk6-browser

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

Panic with `we either navigate top level or have old version of the navigated frame` #1341

Closed ankur22 closed 4 months ago

ankur22 commented 5 months ago

Brief summary

When navigating through certain websites, in some cases an error we either navigate top level or have old version of the navigated frame is reported which stops the test iteration. At the moment it's not clear why this occurs, but in the investigation so far we have found that the frame in question that is causing this panic isn't being tracked by k6 browser.

There are two ways in which a new frame is tracked by k6 browser:

  1. Chrome tells us that a frame is attached (this is where we expect a CDP message);
  2. We attach to the frame(s) by getting the existing frame tree.

We have explored the CDP requests that are made between k6 browser and chrome, but we do not see a frameAttached event being sent from Chrome, but we can see that the frameTree does contain a reference to the new frame as a child.

xk6-browser version

v1.5.1

OS

NA

Chrome version

NA

Docker version and image (if applicable)

NA

Steps to reproduce the problem

Run the script that is causing this issue to replicate the issue.

Expected behaviour

Script passes with no issues.

Actual behaviour

Panics with we either navigate top level or have old version of the navigated frame.

### Tasks
- [ ] https://github.com/grafana/xk6-browser/pull/1346
- [ ] Release notes
- [ ] https://github.com/grafana/xk6-browser/issues/1347
ankur22 commented 5 months ago

I think I have a better idea of what is happening.

  1. We have the page, which has a frame and a session.
  2. The page has an iframe, which is first attached with page.frameAttached CDP from Chrome.
  3. Eventually we get a target.attachTarget for the iframe, but a new frameSession is also created.
  4. Now we start to receive CDP requests from both the original session and new session for the iframe.
  5. We finally receive a frameDetached CDP event of type "swap" from chrome for the iframe on the original session (not the new session).
  6. We remove the child frames. navigation error since the child frame was removed.

I'm not 100% sure how to handle the CDP events incoming from different sessions for the same frame, but I think we can check to see if the incoming detach event's sessionId matches the frame's sessionId, and if they don't match we ignore the event.

Playwright does something similar to what i'm proposing, well, they just check to see if the frame exists with a session.

Puppeteer seems to ignore the swap frameDetach events completely, as in they don't seem to remove any frames :shrug:

inancgumus commented 5 months ago

@ankur22

👍 Makes sense.

Puppeteer seems to ignore the swap frameDetach events completely, as in they don't seem to remove any frames 🤷

Side note: When I look into the Puppeteer's code, they seem to be doing some actions for the frame-swapped events. This and this.

ankur22 commented 5 months ago

Side note: When I look into the Puppeteer's code, they seem to be doing some actions for the frame-swapped events. This and this.

Thanks for the links. Yeah, i still don't see them removing the frames, so i feel the solution we're going with (at least for now) is a good one. We might have to change it if we find another edge case with iframe, and bring it more in line with either Playwright or Puppeteer.

inancgumus commented 5 months ago

Yes, our solution is fine. Just wanted to give a heads-up about the future changes.

On Fri, May 24, 2024 at 1:40 PM Ankur @.***> wrote:

Side note: When I look into the Puppeteer's code, they seem to be doing some actions for the frame-swapped events. This https://github.com/puppeteer/puppeteer/blob/d963fcdd80dbcf26fc6e169917ddd18e7a751f4f/packages/puppeteer-core/src/cdp/LifecycleWatcher.ts#L224-L265 and this https://github.com/puppeteer/puppeteer/blob/d963fcdd80dbcf26fc6e169917ddd18e7a751f4f/packages/puppeteer-core/src/cdp/Frame.ts#L73-L77 .

Thanks for the links. Yeah, i still don't see them removing the frames, so i feel the solution we're going with (at least for now) is a good one. We maybe have to change it if we find another edge case with iframe.

— Reply to this email directly, view it on GitHub https://github.com/grafana/xk6-browser/issues/1341#issuecomment-2129215284, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXVMCJNGBLJGVFGQUV2R3ZD4KKPAVCNFSM6AAAAABID3FCAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRZGIYTKMRYGQ . You are receiving this because you commented.Message ID: @.***>