-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
♻️ Refactor: added defaultmap + locking mechanism for intercepts queue #1441
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
3a81d2d
feat: added defaultmap + locking mechanism for intercepts queue
nilshah98 2a12528
feat: added checkResponseSent lock
nilshah98 90b6085
chore: add comments + rename variables
nilshah98 1cda91c
refactor: move DefaultMap to core/utils
nilshah98 63ec621
feat: track lifecycle for ALL requests, does not matter if nterwork i…
nilshah98 8fb51fc
feat: add locking mechanism for Network.loadingFailed and Network.eve…
nilshah98 f70b9ea
refactor: lock names in requestLifeCycle
nilshah98 c1ec3d6
chore: lint fix
nilshah98 98ff139
fix: coverage issue with single if blocks
nilshah98 f3c6511
test: added defaultMap specs
nilshah98 ebd0542
chore: remove unused intercepts map
nilshah98 924a32f
Merge branch 'master' of github.com:percy/cli into refactor-network-js
nilshah98 3582916
Capture service worker intercepted requests (#1443)
nilshah98 e617894
Merge branch 'master' of github.com:percy/cli into refactor-network-js
nilshah98 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,23 +1,34 @@ | ||||||
import { request as makeRequest } from '@percy/client/utils'; | ||||||
import logger from '@percy/logger'; | ||||||
import mime from 'mime-types'; | ||||||
import { createResource, hostnameMatches, normalizeURL, waitFor } from './utils.js'; | ||||||
import { DefaultMap, createResource, hostnameMatches, normalizeURL, waitFor } from './utils.js'; | ||||||
|
||||||
const MAX_RESOURCE_SIZE = 25 * (1024 ** 2); // 25MB | ||||||
const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308]; | ||||||
const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', 'Other']; | ||||||
const ABORTED_MESSAGE = 'Request was aborted by browser'; | ||||||
|
||||||
// RequestLifeCycleHandler handles life cycle of a requestId | ||||||
// Ideal flow: requestWillBeSent -> requestPaused -> responseReceived -> loadingFinished / loadingFailed | ||||||
// ServiceWorker flow: requestWillBeSent -> responseReceived -> loadingFinished / loadingFailed | ||||||
class RequestLifeCycleHandler { | ||||||
constructor() { | ||||||
this.resolveRequestWillBeSent = null; | ||||||
this.resolveResponseReceived = null; | ||||||
this.requestWillBeSent = new Promise((resolve) => (this.resolveRequestWillBeSent = resolve)); | ||||||
this.responseReceived = new Promise((resolve) => (this.resolveResponseReceived = resolve)); | ||||||
} | ||||||
} | ||||||
// The Interceptor class creates common handlers for dealing with intercepting asset requests | ||||||
// for a given page using various devtools protocol events and commands. | ||||||
export class Network { | ||||||
static TIMEOUT = undefined; | ||||||
|
||||||
log = logger('core:discovery'); | ||||||
|
||||||
#requestsLifeCycleHandler = new DefaultMap(() => new RequestLifeCycleHandler()); | ||||||
#pending = new Map(); | ||||||
#requests = new Map(); | ||||||
#intercepts = new Map(); | ||||||
#authentications = new Set(); | ||||||
#aborted = new Set(); | ||||||
|
||||||
|
@@ -26,6 +37,7 @@ | |||||
this.timeout = options.networkIdleTimeout ?? 100; | ||||||
this.authorization = options.authorization; | ||||||
this.requestHeaders = options.requestHeaders ?? {}; | ||||||
this.captureMockedServiceWorker = options.captureMockedServiceWorker ?? false; | ||||||
this.userAgent = options.userAgent ?? | ||||||
// by default, emulate a non-headless browser | ||||||
page.session.browser.version.userAgent.replace('Headless', ''); | ||||||
|
@@ -43,7 +55,7 @@ | |||||
|
||||||
let commands = [ | ||||||
session.send('Network.enable'), | ||||||
session.send('Network.setBypassServiceWorker', { bypass: true }), | ||||||
session.send('Network.setBypassServiceWorker', { bypass: !this.captureMockedServiceWorker }), | ||||||
session.send('Network.setCacheDisabled', { cacheDisabled: true }), | ||||||
session.send('Network.setUserAgentOverride', { userAgent: this.userAgent }), | ||||||
session.send('Network.setExtraHTTPHeaders', { headers: this.requestHeaders }) | ||||||
|
@@ -125,7 +137,6 @@ | |||||
|
||||||
if (!keepPending) { | ||||||
this.#pending.delete(requestId); | ||||||
this.#intercepts.delete(requestId); | ||||||
} | ||||||
} | ||||||
|
||||||
|
@@ -153,44 +164,42 @@ | |||||
// aborted. If the request is already pending, handle it; otherwise set it to be intercepted. | ||||||
_handleRequestPaused = async (session, event) => { | ||||||
let { networkId: requestId, requestId: interceptId, resourceType } = event; | ||||||
|
||||||
// wait for request to be sent | ||||||
await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent; | ||||||
let pending = this.#pending.get(requestId); | ||||||
this.#pending.delete(requestId); | ||||||
|
||||||
// guard against redirects with the same requestId | ||||||
if (pending?.request.url === event.request.url && | ||||||
pending.request.method === event.request.method) { | ||||||
await this._handleRequest(session, { ...pending, resourceType, interceptId }); | ||||||
} else { | ||||||
// track the session that intercepted the request | ||||||
this.#intercepts.set(requestId, { ...event, session }); | ||||||
} | ||||||
pending?.request.url === event.request.url && | ||||||
pending.request.method === event.request.method && | ||||||
await this._handleRequest(session, { ...pending, resourceType, interceptId }); | ||||||
} | ||||||
|
||||||
// Called when a request will be sent. If the request has already been intercepted, handle it; | ||||||
// otherwise set it to be pending until it is paused. | ||||||
_handleRequestWillBeSent = async event => { | ||||||
let { requestId, request } = event; | ||||||
let { requestId, request, type } = event; | ||||||
|
||||||
// do not handle data urls | ||||||
if (request.url.startsWith('data:')) return; | ||||||
|
||||||
if (this.intercept) { | ||||||
let intercept = this.#intercepts.get(requestId); | ||||||
this.#pending.set(requestId, event); | ||||||
|
||||||
if (intercept) { | ||||||
// handle the request with the session that intercepted it | ||||||
let { session, requestId: interceptId, resourceType } = intercept; | ||||||
await this._handleRequest(session, { ...event, resourceType, interceptId }); | ||||||
this.#intercepts.delete(requestId); | ||||||
if (this.captureMockedServiceWorker) { | ||||||
await this._handleRequest(undefined, { ...event, resourceType: type, interceptId: requestId }, true); | ||||||
} | ||||||
} | ||||||
// release request | ||||||
// note: we are releasing this, even if intercept is not set for network.js | ||||||
// since, we want to process all-requests in-order doesn't matter if it should be intercepted or not | ||||||
this.#requestsLifeCycleHandler.get(requestId).resolveRequestWillBeSent(); | ||||||
} | ||||||
|
||||||
// Called when a pending request is paused. Handles associating redirected requests with | ||||||
// responses and calls this.onrequest with request info and callbacks to continue, respond, | ||||||
// or abort a request. One of the callbacks is required to be called and only one. | ||||||
_handleRequest = async (session, event) => { | ||||||
_handleRequest = async (session, event, serviceWorker = false) => { | ||||||
let { request, requestId, interceptId, resourceType } = event; | ||||||
let redirectChain = []; | ||||||
|
||||||
|
@@ -208,13 +217,18 @@ | |||||
request.redirectChain = redirectChain; | ||||||
this.#requests.set(requestId, request); | ||||||
|
||||||
await sendResponseResource(this, request, session); | ||||||
if (!serviceWorker) { | ||||||
await sendResponseResource(this, request, session); | ||||||
} | ||||||
} | ||||||
|
||||||
// Called when a response has been received for a specific request. Associates the response with | ||||||
// the request data and adds a buffer method to fetch the response body when needed. | ||||||
_handleResponseReceived = (session, event) => { | ||||||
_handleResponseReceived = async (session, event) => { | ||||||
let { requestId, response } = event; | ||||||
// await on requestWillBeSent | ||||||
// no explicitly wait on requestWillBePaused as we implictly wait on it, since it manipulates the lifeCycle of request using Fetch module | ||||||
await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
[optional] add a method |
||||||
let request = this.#requests.get(requestId); | ||||||
/* istanbul ignore if: race condition paranioa */ | ||||||
if (!request) return; | ||||||
|
@@ -224,20 +238,28 @@ | |||||
let result = await this.send(session, 'Network.getResponseBody', { requestId }); | ||||||
return Buffer.from(result.body, result.base64Encoded ? 'base64' : 'utf-8'); | ||||||
}; | ||||||
// release response | ||||||
this.#requestsLifeCycleHandler.get(requestId).resolveResponseReceived(); | ||||||
} | ||||||
|
||||||
// Called when a request streams events. These types of requests break asset discovery because | ||||||
// they never finish loading, so we untrack them to signal idle after the first event. | ||||||
_handleEventSourceMessageReceived = event => { | ||||||
let request = this.#requests.get(event.requestId); | ||||||
_handleEventSourceMessageReceived = async event => { | ||||||
let { requestId } = event; | ||||||
// wait for request to be sent | ||||||
await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent; | ||||||
let request = this.#requests.get(requestId); | ||||||
/* istanbul ignore else: race condition paranioa */ | ||||||
if (request) this._forgetRequest(request); | ||||||
} | ||||||
|
||||||
// Called when a request has finished loading which triggers the this.onrequestfinished | ||||||
// callback. The request should have an associated response and be finished with any redirects. | ||||||
_handleLoadingFinished = async event => { | ||||||
let request = this.#requests.get(event.requestId); | ||||||
let { requestId } = event; | ||||||
// wait for upto 2 seconds or check if response has been sent | ||||||
await this.#requestsLifeCycleHandler.get(requestId).responseReceived; | ||||||
let request = this.#requests.get(requestId); | ||||||
/* istanbul ignore if: race condition paranioa */ | ||||||
if (!request) return; | ||||||
|
||||||
|
@@ -246,7 +268,13 @@ | |||||
} | ||||||
|
||||||
// Called when a request has failed loading and triggers the this.onrequestfailed callback. | ||||||
_handleLoadingFailed = event => { | ||||||
_handleLoadingFailed = async event => { | ||||||
let { requestId } = event; | ||||||
// wait for request to be sent | ||||||
// note: we are waiting on requestWillBeSent and NOT responseReceived | ||||||
// since, requests can be cancelled in-flight without Network.responseReceived having been triggered | ||||||
// and in any case, order of processing for responseReceived and loadingFailed does not matter, as response capturing is done in loadingFinished | ||||||
await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent; | ||||||
let request = this.#requests.get(event.requestId); | ||||||
/* istanbul ignore if: race condition paranioa */ | ||||||
if (!request) return; | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when is something deleted from this map ?
also considering we have this, we dont need any state sets right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing is ever deleted from
#requestsLifeCycleHandler
since it is only used to manage lifecycles.We can ideally delete
requestId
once all the locks are released from it's object, ie. it has reached end of lifecycle.But, I don't see the point of it, as this is a very light map.
Need
#pending
to communicate fromrequestWillBeSent
torequestPaused
states.Can remove
#intercepts
completely.Need
#requests
as it keeps mapping of all active requests and is whatidle
depends on.Need
#authentication
as this keeps track of all requests that have already been authenticated.Need
#aborted
to keep track of aborted requests.In the current PR, I am planning to only remove
#intercepts
queue as it was used for communicating fromrequestPaused
torequestWillBeSent
but that should never happen now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as this is a very light map.
do not fully agree with this as we could have thousands of requests in a single page load [ in fact we do for many storybook builds ] [ even though not blocking as it would be still few kbs max ]_requestPaused
couldawait requestWillBeSent
, so it doesnt need to read from#pending
as far as i understand. Because we are also storing event in pending with request id, you can justresolve
withevent
for request will be sent#requests
agree we need it for now at least#authentication
could be done by checking if auth promise is already resolved [ but i dont mind using set either ]#aborted
is not necessary as you could just setaborted
on#request
object [ or in lifecycle like lifecyle.aborted ] it was done the way it is because we could processaborted
event beforerequestWillBeSent
and it wont exist in requests at all but asloadingFailed
would await onrequestWillBeSent
we can be sure that request already exists in requestsAlso other refactors are optional, not necessary in current pr