feat: Cooperative request intercepts #6733
Closed
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.
Hello, I wanted to open a discussion via this PR about a solution for cooperative request intercepts. I needed this for a recent project and thought it might help to share my fork.
If this PR seems too aggressive to be a core change, my other thought is #6734, which is just the modification to allow Puppeteer to accept pluggable
NetworkManager
andHTTPResponse
classes. Then I can maintain a separate package with these more extensive changes.The Problems
Puppeteer's core expects only one
page.on('request')
handlerPuppeteer anticipates only one request intercept handler (
page.on('request', ...)
) to performcontinue()
,abort()
, orrespond()
. As the community has pointed out in multiple issues, there are often cases where multiple request handlers make sense. For example,puppeteer-extra
attempts a plugin architecture. The current core design makes it impossible for multiple request handlers to cooperatively determine what to do with a request.Here is a sample of issues for background:
berstend/puppeteer-extra#364
berstend/puppeteer-extra#156
argos-ci/jest-puppeteer#308
#5334
#3853 (comment)
#2687
#4524
Puppeteer's request intercept handler does not wait for async handlers to finish
Request intercept handling currently does not support async intercept handlers. The underlying
mitt
event system does not await async events, andNetworkManager
just fires the events and moves on:The solutions
The solution this PR proposes a cooperative interception strategy while also making
NetworkManager
wait for async handlers to be fulfilled before finalizing the request interception.One caveat to a cooperative interception strategy is deciding which directive should 'win'. I decided upon
abort > respond > continue
Possible breaking changes
HTTPRequest.abort()
,HTTPRequest.continue()
andHTTPRequest.respond()
are no longer return promises. Their behavior has changed because they no longer await resolution of the underlying the CDP Fetch events.I don't think they should anymore, because they are merely cooperative 'recommendations' about what to do. We could make these still return a promise, but then we'd have to decide what to do if the request interception ultimately resolved differently. For example, if you did
await continue()
but the request actually aborted, what should happen? Probably an exception? I think it is cleaner to make these return nothing, and if needed, introduce a separateHTTPRequest.fulfilled()=>Promise<ResolutionType>
signal.