puppeteer: added support for cooperative mode.

[akornatskyy]

This is related to #2277.

[remusao]

Looks great, thank you!

[Kikobeats]

Hey, any idea how I can update this block of code to be compatible with Cooperative mode?

if (abortTypes.length > 0) {
      await page.setRequestInterception(true)
      page.on('request', req => {
        const resourceType = req.resourceType()
        if (!abortTypes.includes(resourceType)) return req.continue({})
        debug('abort', { url: req.url(), resourceType })
        return req.abort('blockedbyclient')
      })
    }

From my understanding, the request should be aborted before the adblocker take care of it

[remusao]

@Kikobeats Did you check this: https://github.com/puppeteer/puppeteer/blob/v10.2.0/docs/api.md#upgrading-to-cooperative-mode-for-package-maintainers? Maybe you can apply the suggested change to your code to make it cooperative. You also need to change the usage of the adblocker by adding a call to: blocker.setRequestInterceptionPriority() (otherwise you get the legacy behavior)

[akornatskyy]

you need something like the following:

// somewhere earlier
blocker.setRequestInterceptionPriority(0);

// ...
if (abortTypes.length > 0) {
  await page.setRequestInterception(true)
  page.on('request', req => {
    const resourceType = req.resourceType()
    if (!abortTypes.includes(resourceType)) return req.continue(req.continueRequestOverrides(), 0)
    debug('abort', { url: req.url(), resourceType })
    return req.abort('blockedbyclient', 0)
  })
}

[benallfree]

Very sorry to meddle in the comments of a merged PR. If your handler has no opinion on further action and the intent is to bypass/defer to other handlers in cooperative mode, use details.continue(details.continueRequestOverrides(), 0) because 0 is the de-facto default priority.

[remusao]

Shouldn't this be already the case, though? The default value is undefined which should preserve the legacy mode (non-cooperative), then if setRequestInterceptionPriority is called without argument the priority defaults to 0. And only if the user decides to use an explicit value different than 0 will this.priority be different from 0. Does this seem correct?

[benallfree]

I think there are two cases. In one case the handler might just mean to provide a default continuation action so the request doesn't hang. In the other case, it might actually mean to override a lower-priority action.

I added a detailed discussion about this, please tell me what you think: https://github.com/puppeteer/puppeteer/pull/7796/files#diff-9eddf4dc51bf6a0125ca7fb094468ad284112270385c41cebce4f8f0a29620abR2617

[benallfree]

Here is an example:

Handler A calls .continue({},1) in certain cases
Handler B calls .abort('..',0) in other non-overlapping cases

So we use setRequestInterceptionPriority(2) to make sure ads are blocked and not continued by Handler A. But now, we have broken Handler B because adBlocker will call .continue({},2). What we really meant was .abort('..',2) and ,continue({},0).

[remusao]

I see, that makes sense. Then we should probably fix this.

[benallfree]

See https://github.com/puppeteer/puppeteer/pull/7796/files#diff-9eddf4dc51bf6a0125ca7fb094468ad284112270385c41cebce4f8f0a29620abR2741 for a flexible solution that addresses both cases.

[akornatskyy]

abort > respond > continue (for the same priority). it probably makes sense to split priority for each case, e.g. continue with 0 and block with 2.

interface ResolutionStrategy {
  abortPriority: number;
  continuePriority: number;
}

const DEFAULT_STRATEGY: ResolutionStrategy = {
  abortPriority: 0,
  continuePriority: 0,
};

[benallfree]

@akornatskyy Also allow for undefined to preserve legacy, and we moved away from the strategy phrasing https://github.com/puppeteer/puppeteer/pull/7796/files#diff-9eddf4dc51bf6a0125ca7fb094468ad284112270385c41cebce4f8f0a29620abR2714.

interface InterceptResolutionConfig {
  abortPriority?: number;
  continuePriority?: number;
}

// This approach supports multiple priorities based on situational
// differences. You could, for example, create a config that
// allowed separate priorities for PNG vs JPG.
const DEFAULT_CONFIG: InterceptResolutionConfig = {
  abortPriority: undefined,     // Default to Legacy Mode
  continuePriority: undefined,  // Default to Legacy Mode
};

// Defaults to undefined which preserves Legacy Mode behavior
let _config: Partial<InterceptResolutionConfig> = {};

export const setInterceptResolutionConfig = (config: InterceptResolutionConfig) =>
  (_config = { ...DEFAULT_CONFIG, ...config });

[FdezRomero]

@akornatskyy The new methods are already released in Puppeteer v13 🎉

In addition to the check that @benallfree recommended, I'd update the details.continueRequestOverrides() to details.continueRequestOverrides?.() as this method didn't exist before Puppeteer 10.2.0 and it currently breaks for older versions that are still supported by this package (cc/ @remusao).

[benallfree]

@FdezRomero That's a great backward compatibility point, I might add that to the official docs/samples.

[benallfree]

If you do update this, you might consider waiting for this upcoming PR https://github.com/puppeteer/puppeteer/pull/7796/files#diff-9eddf4dc51bf6a0125ca7fb094468ad284112270385c41cebce4f8f0a29620abR5035 because it introduces a legacy mode guard too:

if (details.isInterceptResolutionHandled()) return

[benallfree]

Note: here also is a backward compatibility check that is safer:

if (details.isInterceptResolutionHandled?.()) return

[FdezRomero]

@benallfree I created #2343 to gather all the changes in one PR (I couldn't add you as reviewer).

[benallfree]

puppeteer/puppeteer#7796 has been accepted by the Puppeteer team 👍