Skip to content

🐛 Wait for isolated networks to idle#491

Merged
wwilsman merged 3 commits intomasterfrom
ww/wait-for-frame-idle
Aug 12, 2021
Merged

🐛 Wait for isolated networks to idle#491
wwilsman merged 3 commits intomasterfrom
ww/wait-for-frame-idle

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

What is this?

Thanks to #490, it now makes it possible to properly capture resources from isolated pages. However, during the network idle check, only the top-level network method is used which only references its own requests. This PR adds tracking isolated pages to the parent page to create a inverse relationship. And also adds a new network method to retrieve all nested page requests during the idle check. These changes make it so network idle will check for nested requests in addition to its own requests.

While here, I also came across a familiar race condition in the page constructor as a result of the page closing during an async call within the constructor. This same race condition exists in the network constructor, however with a check to prevent intentional page closed errors from bubbling. Rather than repeat the pattern, it was improved slightly to make debug logs less confusing. Now, when one of these async constructor calls happen with the page closing, the error is disregarded. All other errors will be logged rather than let bubble since an error from these locations usually stems from an error elsewhere.

@wwilsman wwilsman added the 🐛 bug Something isn't working label Aug 12, 2021
@wwilsman wwilsman requested a review from Robdel12 August 12, 2021 19:56
async close() {
if (!this.browser) return;

this.log.debug('Page closing', this.meta);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also moved this from the close handler since the handler will be called on every automatically created page while this method is only called when intentionally closing a page. Having it remain in the former location would show several close logs in a row for pages with many frames.

Copy link
Copy Markdown
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏁

This value is too low for CI and some assets are missed
@wwilsman wwilsman enabled auto-merge (squash) August 12, 2021 20:17
@wwilsman wwilsman disabled auto-merge August 12, 2021 20:30
@wwilsman wwilsman enabled auto-merge (squash) August 12, 2021 20:31
@wwilsman wwilsman merged commit d47f6b1 into master Aug 12, 2021
@wwilsman wwilsman deleted the ww/wait-for-frame-idle branch August 12, 2021 20:36
samarsault pushed a commit that referenced this pull request Mar 3, 2023
Bumps [@percy/sdk-utils](https://github.com/percy/cli/tree/HEAD/packages/sdk-utils) from 1.0.5 to 1.0.8.
- [Release notes](https://github.com/percy/cli/releases)
- [Commits](https://github.com/percy/cli/commits/v1.0.8/packages/sdk-utils)

---
updated-dependencies:
- dependency-name: "@percy/sdk-utils"
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants