Solving request interception chain

[XavierGeerinck]

Hi Everyone!

Currently I am having the problem that when you have 2 plugins that try to intercept and handle requests, that the dependencies start to fail. (E.g. when I have the adblocker plugin and a personal plugin that both try to block resources, then it starts stating that a request has already been handled).

I agree that this is how it is with Puppeteer and that it will be hard to fix it (sadly enough). But seeing the plugin interface, I think it might be worthwhile to find a solution to this.

Some thoughts that I had:

• Create page hooks (such as the onPageCreated) but then for onPageRequest, onPageRequestFinished, onPageRequestFailed and make them check if a request has been handled already (it should be possible to keep working with requests until an .abort has been called or the request finished?)
• Add a weight system to allow certain plugins to come before others (e.g. adblocker to block requests before other plugins).

I'd love to hear some thoughts from the community to see if this is feasible or not? Looking forward to the responses!

[berstend]

@XavierGeerinck thanks for the well thought-out comment 👍

It's been a while since I worked on that aspect, but I remember the main annoyance being that when request interception is enabled someone needs to decide if the request shall be continued or aborted (but that decision can only happen once).

That makes it a bit annoying to work with from a plugin framework perspective 😄 If I remember correctly puppeteer is holding an internal state for this.

If all code happens within plugins this would be relatively simple to fix, it get's more tricky when we need to take care of code in the user's scripts as well (by patching .enableRequestInterception) and the like.

I would need to refresh my memory on the issues I ran into while working on a solution to this a while back, happy for other input on this though.

[XavierGeerinck]

Well I am personally fine if plugins have to be written for this to work inside the core, but I also believe a better solution could be found. The solution I personally was thinking was to indeed handle this within the plugin framework and:

• add the appropriate hooks, then when they exist make the hooks return abort or continue and finally decide in the framework to continue or not :)
• Add a specific flag to show what was decided (so that in the code you can check if the request was indeed handled or not)

Is this something that could be added? Would love to see this upstream!

In the meantime I also opened a request on puppeteer for official plugin support as well as solving this use case: puppeteer/puppeteer#6567

let me know what you think!

[knuppe]

A simple solution would be.

page.on('request', async (req) => {
    if (req._interceptionHandled) return;
    /* ... */
}

[XavierGeerinck]

_interceptionHandled

I am not sure tbh.. this is what I used before and it will then crash further on in the code if you have multiple listeners, since the request was already handled then.

[berstend]

I'm lacking the bandwidth at the moment to look into this properly, but I'm pretty sure there's a way to fix this so multiple plugins as well as external scripts can use request interception at the same time. :-)

I will revisit this once playwright support and the new stealth plugin has landed, if anyone else wants to take a stab at this in the meantime that'd be great as well.

[berstend]

I guess one "transparent" way to handle this could be to patch request.continue() to not finalize the interception but have all other listeners be called as well. In case the following handlers don't issue either continue() or abort() then a fallback handler would do the final continue(). In that scenario abort() would be final (so if the adblocker plugin aborts a request it wouldn't show up in other handlers).

In terms of order of execution: First come first served? That would mean plugins get the first pick (in order they're added), followed by handlers the user might've defined.

[benallfree]

edit: @berstend I had a similar idea, PR in puppeteer/puppeteer#6735

I'm here because I hit this same issue. My use case is I want to intercept pptr requests and potentially pull them from a Redis cache rather than repeating the request or relying on the pptr cache. Unfortunately, whoever calls request.continue() or request.abort() first, wins.

To me, this means pptr can have at most one request interceptor in page.on('request'). Since that's the case, puppeteer-extra should add one listener there and then all the plugins should hook into a puppeteer-extra event system. That way all the plugins can run, each saying whether or not to continue or abort.

page.on('request', req=> {
   const votes = await Promise.all(px.onRequestListeners.map( (e)=>e(req) ))
   const shouldContinue = votes.reduce( (a, e)=>a&&e, true)
   if(shouldContinue) req.continue() else req.abort()
});

[benallfree]

Note: I published a package to fix this https://www.npmjs.com/package/enchant-puppeteer

[berstend]

@benallfree any specific reason your upstream PR is closed but not merged? 🤔
puppeteer/puppeteer#6733

[benallfree]

@berstend Sorry for the confusion, it was moved to puppeteer/puppeteer#6735

[FdezRomero]

@benallfree Your PR puppeteer/puppeteer#6735 was already merged. What would need to be done to completely fix the issue in puppeteer-extra? Thanks!

[benallfree]

@FdezRomero Yes, here are the upgrade instructions: https://github.com/puppeteer/puppeteer/blob/v11.0.0/docs/api.md#upgrading-to-cooperative-mode-for-package-maintainers

Basically it entails adding a 2nd arg to each .abort(), .continue(), and .respond() you call.

Also take note of the setInterceptResolutionStrategy() suggestion in the upgrade notes. puppeteer-extra could really benefit from this.

If you'd like hands-on help implementing this upgrade, please let me know. puppeteer-extra and this ticket were the original inspiration for what eventually became a core contrib to Puppeteer 👍

[FdezRomero]

@benallfree Thanks a lot for the details! I'll try adding support for Cooperative Mode for the stealth, adblocker and resource blocker plugins and confirm that they get along well 👍🏻

I agree that it should be made possible to define the priorities, so I'll add setInterceptResolutionStrategy() as suggested there. I'll let you know if I need some help 😉

[FdezRomero]

@berstend @benallfree I created a PR to fix this issue for both plugins in #592, feedback appreciated 😄

[dekelev]

@berstend Looks like PR is stuck waiting for review for few months now. Can we please merge this?