[Regression]: Cooperative Intercept Handlers should execute even when a handler throws an exception

[benallfree]

Bug description

@jschfflr I believe https://github.com/puppeteer/puppeteer/pull/7722/files#diff-2fedf4b9325c81a1652c274473daa03e99b58f353e1bba42da48d43f15a4f527R232 introduces two regressions with respect to Cooperative Intercept Mode:

1. Uncaught/unhandled exception when handler throws
2. Handler exception stops subsequent handlers from executing

In the below TS playground example, you can see the importance of .catch(handleError). Without it, handler3 is never executed. This is undesirable in any cooperative scenario because it means your handler is not guaranteed to run. See https://github.com/puppeteer/puppeteer/blob/v11.0.0/docs/api.md#cooperative-intercept-mode-and-legacy-intercept-mode which states In Cooperative Mode, all intercept handlers are guaranteed to run and all async handlers are awaited.

const handler1 = async () => {
  console.log('handler 1 success')
}

const handler2 = async () => {
  throw new Error(`Handler 2 failed`)
}

const handler3 = async () => {
  console.log('handler 3 success')
}

const handleError = (e:Error)=>{
  console.log(`Caught error ${e}`)
}
const interceptActions = [handler1, handler2, handler3]

const finalizeInterceptions = async () => {

  await interceptActions.reduce(
    (promiseChain, interceptAction) => promiseChain.then(interceptAction).catch(handleError),
    Promise.resolve()
  );
}

finalizeInterceptions()

Puppeteer version

master

Node.js version

14

npm version

8

What operating system are you seeing the problem on?

macOS

Relevant log output

No response

[benallfree]

I think https://github.com/puppeteer/puppeteer/pull/7020/files#diff-2fedf4b9325c81a1652c274473daa03e99b58f353e1bba42da48d43f15a4f527L233 contributed to this regression because the comment about the reasoning for catching the error was removed.

[benallfree]

Tagging:
#7020
#7722
@MikkelSnitker

[benallfree]

While we're on the topic, I think removing the catch here (https://github.com/puppeteer/puppeteer/pull/7020/files#diff-3280fbae5b6189c9b87a76ea21a5da3e10140467e29532e0b7b72608036f5070L391) might be risky. In theory it should never execute, but since the promise is not returned it could result in an unhandled exception at some point in the future. I remember something about future versions of nodejs exiting for unhandled exceptions rather than just reporting them. Not sure what the Puppeteer team's policy is regarding unhandled exceptions, but I thought I'd mention it.

[stale]

We're marking this issue as unconfirmed because it has not had recent activity and we weren't able to confirm it yet. It will be closed if no further activity occurs within the next 30 days.

[stale]

We are closing this issue. If the issue still persists in the latest version of Puppeteer, please reopen the issue and update the description. We will try our best to accomodate it!