Endless frameattached events [BUG] #4624
Assignees
Comments
|
@aldeed awesome report, thank you. Investigating |
My best guess here would be an excessive I/O. We fire the event regardless of presence event listeners, so not much changes on the Playwright side of things.
This does look like a real bug - I failed to reproduce it though on tip-of-tree playwright.
At the time However, you can subscribe to the
This one is interesting - we'll discuss it later today on the team meeting. Can you please share more information on your usecase? What exactly do you do with frames? |
|
@aslushnikov thanks!
Where's the Playwright code responsible for this? Might see if I can see anything that could be the cause because it's strange to me that the website responds so much more slowly just based on one listener with a logging statement on the Node side.
I implemented something like this, as well as filtering out based on Playwright's own visibility logic (many junk frames are invisible, at least initially). It prevents us from adding to the problem, but even just listening for the frames and doing that check on each one was slowing down the page and sometimes crashing it. We are talking about many hundreds of frames per minute being created and disposed, and a baseline of 200 or so frames are never disposed.
I'd say 100% of the time I get at least one of those errors with the posted code file, if I add
Essentially we are trying to track each frame created immediately in order to get info from it later, at which point it sometimes is already disposed or navigated away. But we only care about "real" frames. We may be switching to another way of doing things partially because of this issue, but I still thought it important to bring up a reproduction for you in case others might experience it. |
Here it is: playwright/src/server/frames.ts Line 121 in c5bb08c
Out of curiosity - are you doing some sort of page performance analysis? Why would you differentiate between "real" frames and Ad frames? If you're in control of the website you run against - can't you label "real" frames somehow - e.g. with an attribute?
We discussed this on a team meeting and decided it's too far-fetched for a testing library - looks like there's no good test-related usecase. |
|
Thanks. We may label real frames, but it may not always be possible. Frames that we do care about may be created by other libraries or functions that don't support adding attributes to them. I agree that built-in detection of google frames doesn't make much sense unless a lot of people start having this problem. I'll take a peek at that code. |
|
After looking at the code I have one line of thinking that will need more investigation. In terms of the playwright code, the I would need to do a lot more debugging to determine if that's true and if that's why performance degrades as soon as a listener is added. I also don't know what the solution is, and whether it could be solved in playwright or if the reference would need to be dropped by the listener code somehow. |
Do you mean that userland code would keep a reference on frame? In your example, I don't think there's any reference retained. |
|
I mean that something in the Node EventEmitter class might keep a reference or somehow prevent GC when there are listeners, though it does seem unlikely. |
|
Also, @aslushnikov I can confirm that the frameCommittedNewDocumentNavigation(frameId, url, name, documentId, initial) {
const frame = this._frames.get(frameId);
+ if (!frame)
+ return;
this.removeChildFramesRecursively(frame);
this.clearWebSockets(frame);
frame._url = url;
frame._name = name;
let keepPending;
if (frame._pendingDocument) {
if (frame._pendingDocument.documentId === undefined) {
// Pending with unknown documentId - assume it is the one being committed.
frame._pendingDocument.documentId = documentId;
}
if (frame._pendingDocument.documentId === documentId) {
// Committing a pending document.
frame._currentDocument = frame._pendingDocument;
}
else {
// Sometimes, we already have a new pending when the old one commits.
// An example would be Chromium error page followed by a new navigation request,
// where the error page commit arrives after Network.requestWillBeSent for the
// new navigation.
// We commit, but keep the pending request since it's not done yet.
keepPending = frame._pendingDocument;
frame._currentDocument = { documentId, request: undefined };
}
frame._pendingDocument = undefined;
}
else {
// No pending - just commit a new document.
frame._currentDocument = { documentId, request: undefined };
}
frame._onClearLifecycle();
const navigationEvent = { url, name, newDocument: frame._currentDocument };
frame.emit(Frame.Events.Navigation, navigationEvent);
if (!initial) {
debugLogger_1.debugLogger.log('api', ` navigated to "${url}"`);
this._page.emit(page_1.Page.Events.FrameNavigated, frame);
}
// Restore pending if any - see comments above about keepPending.
frame._pendingDocument = keepPending;
} |
|
@aslushnikov Did you use I suppose it could also be related to which ads its showing in the iframes, which might differ by IP region and such. Edit: If it's helpful I can screen record it happening. |
@aldeed Actually, could you please run your script with a (sorry for the long reply - holiday season!) |
|
@aslushnikov Here is a segment of the logs where the error happens twice: I let it run for maybe 30-40 seconds and there were dozens of the same error logged in that time. If you don't see it on a particular loaded w3schools page, you can click Next or navigate to a different page while the script is running and that usually triggers it. (Seems to reload different ads each time you switch pages, and maybe only certain ads cause it.) |
|
I am optimistic that #5320 should fix this issue. Please comment/reopen if this happens again with the next 1.9 release. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Context:
Code Snippet
(Replace "chromium" above with any browser. Happens the same for all.)
Describe the bug
Paste the example code in a file, install latest Playwright package, and run it with Node. You will see something like this:
frame 363 attached frame 364 attached frame 365 attached frame 366 attached frame 367 attached frame 368 attached frame 369 attached frame 370 attached frame 371 attached frame 372 attached frame 373 attached frame 374 attached frame 375 attached frame 376 attached frame 377 attached frame 378 attached frame 379 attached frame 380 attached frame 381 attached frame 382 attached frame 383 attached frame 384 attached frame 385 attached frame 386 attached frame 387 attached frame 388 attached frame 389 attached frame 390 attached frame 391 attached frame 392 attached frame 393 attached frame 394 attached frame 395 attached frame 396 attached frame 397 attached frame 398 attached frame 399 attached frame 400 attached frame 401 attached frame 402 attached frame 403 attached frame 404 attached frame 405 attached frame 406 attached frame 407 attached frame 408 attached frame 409 attached frame 410 attached frame 411 attached frame 412 attached frame detached. 314 remain frame detached. 313 remain frame detached. 312 remain frame detached. 311 remain frame detached. 310 remain frame detached. 309 remain frame 413 attached frame 414 attached frame 415 attached frame 416 attached frame 417 attached frame 418 attached frame 419 attached frame 420 attached frame 421 attached frame 422 attached frame detached. 318 remain frame detached. 317 remain frame detached. 316 remain frame detached. 315 remain frame detached. 314 remain frame detached. 313 remain frame detached. 312 remain frame detached. 311 remain frame detached. 310 remain frame detached. 309 remain frame detached. 308 remain frame detached. 307 remain frame detached. 306 remain frame detached. 305 remain # ... and so on forever until it crashes or CTRL+CThe w3schools page is just one example. It happens on every w3schools page that has ads, as well as other websites. One other example is html.com.
Through debugging, I believe this is due to iframes created by Google code, either tag manager or ads or analytics or some combination. I can try to filter out some iframes based on whether they are visible, etc. but ultimately it is pretty impossible to get any use out of the "frameattached" event when on a page such as these.
I'm considering this a bug for a few reasons:
With a "frameattached" listener, the page (in non-headless) is much slower and sometimes unresponsive. Sometimes it eventually crashes. Gets my CPU humming, too. Without a "frameattached" listener, things are fine. So it seems at the very least that Playwright is not doing a good job of managing frame references on the browser side or is somehow mucking things up.
You will also see an error in the Node console logs now and then as this runs (I think usually only in non-headless runs):
Memory issues aside, this makes "frameattached" kind of useless when you really just want "real" iframes. It might be more a feature than a bug fix, but is there some way Playwright can help here? Maybe pass more frame information like
isVisibleorisEphemeral(no idea how to detect that), or even just an option to ignore frames generated by ad libraries since it's a very common situation.The text was updated successfully, but these errors were encountered: