Skip to content

✨ Improve network tracking resilience#469

Merged
wwilsman merged 2 commits intomasterfrom
ww/network-tracking-resilience
Aug 2, 2021
Merged

✨ Improve network tracking resilience#469
wwilsman merged 2 commits intomasterfrom
ww/network-tracking-resilience

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

@wwilsman wwilsman commented Aug 2, 2021

What is this?

With network tracking, there are a number of events which need to be handled to manage requests properly. While some of these events are intended to happen in a specific order, and handlers are written in a way to handle an unexpected order of events, there aren't actually guards in place to prevent handling seemingly duplicate events.

The changes in this PR were found in Puppeteer after discovering a font-related issue. We have a history of font-related issues, so I thought to give these changes a try here. The specific issue that prompted these safeguards are a result of this Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1196004

Our network handling isn't an exact 1:1 of Puppeteer's, so I wasn't able to reproduce the issue with a test for the same. One likely reason is because we always have browser caches disabled because we have our own cache. Regardless, after breaking down the changes I decided they would be a good addition to have anyway.

  • When requests are handled or forgotten, they should be removed from all trackers.
  • Intercepted request mapping was updated to track the entire request event.
  • During pending requests, already intercepted requests are always handled.
  • During request interception, already pending requests are only handled when it is not actually a redirect.
  • During request handling, redirects clean up other existing trackers besides auth.

When requests are handled or forgotten, they should be removed from all trackers.

Intercepted request mapping was updated to track the entire request event.

During pending requests, already intercepted requests are always handled.

During request interception, already pending requests are only handled when said request is actually
the same request and not a redirect request.

During request handling, redirects clean up other existing trackers besides auths.
@wwilsman wwilsman added the ✨ enhancement New feature or request label Aug 2, 2021
@wwilsman wwilsman requested a review from Robdel12 August 2, 2021 22:52
@wwilsman wwilsman enabled auto-merge (squash) August 2, 2021 22:58
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 was a reallly good find!

@wwilsman wwilsman merged commit 1db67ec into master Aug 2, 2021
@wwilsman wwilsman deleted the ww/network-tracking-resilience branch August 2, 2021 23:17
samarsault pushed a commit that referenced this pull request Mar 3, 2023
* ⬆️ Bump @percy/core from 1.0.0-beta.75 to 1.0.0-beta.76

Bumps [@percy/core](https://github.com/percy/cli/tree/HEAD/packages/core) from 1.0.0-beta.75 to 1.0.0-beta.76.
- [Release notes](https://github.com/percy/cli/releases)
- [Commits](https://github.com/percy/cli/commits/v1.0.0-beta.76/packages/core)

---
updated-dependencies:
- dependency-name: "@percy/core"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

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

* ⬆️ Bump @percy/sdk-utils from 1.0.0-beta.75 to 1.0.0-beta.76 (#469)

Bumps [@percy/sdk-utils](https://github.com/percy/cli/tree/HEAD/packages/sdk-utils) from 1.0.0-beta.75 to 1.0.0-beta.76.
- [Release notes](https://github.com/percy/cli/releases)
- [Commits](https://github.com/percy/cli/commits/v1.0.0-beta.76/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>

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

✨ enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants