chore: remove installAsyncStackHooks helper (#6186)

* chore: remove `installAsyncStackHooks` helper

This code was written when browsers/Node didn't support errors in async
functions very well. They now do a much better job of this, so we can
lose the additonal complexity from our codebase and leave it to the host
environment :)

* lazy launcher is private

* remove async stack test

Author: jackfranklin

Diff for: new-docs/puppeteer.puppeteer._lazylauncher.md

@@ -40,7 +40,6 @@ const puppeteer = require('puppeteer');
 |  [\_\_productName](./puppeteer.puppeteer.__productname.md) |  | string |  |
 |  [\_changedProduct](./puppeteer.puppeteer._changedproduct.md) |  | boolean |  |
 |  [\_isPuppeteerCore](./puppeteer.puppeteer._ispuppeteercore.md) |  | boolean |  |
-|  [\_lazyLauncher](./puppeteer.puppeteer._lazylauncher.md) |  | [ProductLauncher](./puppeteer.productlauncher.md) |  |
 |  [\_preferredRevision](./puppeteer.puppeteer._preferredrevision.md) |  | string |  |
 |  [devices](./puppeteer.puppeteer.devices.md) |  | [DevicesMap](./puppeteer.devicesmap.md) |  |
 |  [errors](./puppeteer.puppeteer.errors.md) |  | [PuppeteerErrors](./puppeteer.puppeteererrors.md) |  |

Diff for: new-docs/puppeteer.puppeteer.md

@@ -13,13 +13,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-// @ts-nocheck
 
-/* This file is used in two places:
+/* This file is used in one place:
  * 1) the coverage-utils use it to gain a list of all methods we check for test
  *    coverage on
- * 2) index.js uses it to iterate through all methods and call
- *    helper.installAsyncStackHooks on
  */
 module.exports = {
   Accessibility: require('./common/Accessibility').Accessibility,

Diff for: src/api.ts

@@ -61,7 +61,7 @@ export class Puppeteer {
   _isPuppeteerCore: boolean;
   _changedProduct = false;
   __productName: string;
-  _lazyLauncher: ProductLauncher;
+  private _lazyLauncher: ProductLauncher;
 
   /**
    * @internal

Diff for: src/common/Puppeteer.ts

@@ -28,10 +28,6 @@ const closeAsync = promisify(fs.close);
 
 export const debugError = debug('puppeteer:error');
 
-interface AnyClass {
-  prototype: object;
-}
-
 function getExceptionMessage(
   exceptionDetails: Protocol.Runtime.ExceptionDetails
 ): string {
@@ -95,39 +91,6 @@ async function releaseObject(
     });
 }
 
-function installAsyncStackHooks(classType: AnyClass): void {
-  for (const methodName of Reflect.ownKeys(classType.prototype)) {
-    const method = Reflect.get(classType.prototype, methodName);
-    if (
-      methodName === 'constructor' ||
-      typeof methodName !== 'string' ||
-      methodName.startsWith('_') ||
-      typeof method !== 'function' ||
-      method.constructor.name !== 'AsyncFunction'
-    )
-      continue;
-    Reflect.set(classType.prototype, methodName, function (...args) {
-      const syncStack = {
-        stack: '',
-      };
-      Error.captureStackTrace(syncStack);
-      return method.call(this, ...args).catch((error) => {
-        const stack = syncStack.stack.substring(
-          syncStack.stack.indexOf('\n') + 1
-        );
-        const clientStack = stack.substring(stack.indexOf('\n'));
-        if (
-          error instanceof Error &&
-          error.stack &&
-          !error.stack.includes(clientStack)
-        )
-          error.stack += '\n  -- ASYNC --\n' + stack;
-        throw error;
-      });
-    });
-  }
-}
-
 export interface PuppeteerEventListener {
   emitter: CommonEventEmitter;
   eventName: string | symbol;
@@ -277,7 +240,6 @@ export const helper = {
   addEventListener,
   removeEventListeners,
   valueFromRemoteObject,
-  installAsyncStackHooks,
   getExceptionMessage,
   releaseObject,
 };

Diff for: src/common/helper.ts

@@ -14,24 +14,13 @@
  * limitations under the License.
  */
 
-// api.ts has to use module.exports as it's also consumed by DocLint which runs
-// on Node.
-// eslint-disable-next-line @typescript-eslint/no-var-requires
-const api = require('./api');
-
-import { helper } from './common/helper';
 import { Puppeteer } from './common/Puppeteer';
 import { PUPPETEER_REVISIONS } from './revisions';
 import pkgDir from 'pkg-dir';
 
 export const initializePuppeteer = (packageName: string): Puppeteer => {
   const puppeteerRootDirectory = pkgDir.sync(__dirname);
 
-  for (const className in api) {
-    if (typeof api[className] === 'function')
-      helper.installAsyncStackHooks(api[className]);
-  }
-
   let preferredRevision = PUPPETEER_REVISIONS.chromium;
   const isPuppeteerCore = packageName === 'puppeteer-core';
   // puppeteer-core ignores environment variables
@@ -43,16 +32,10 @@ export const initializePuppeteer = (packageName: string): Puppeteer => {
   if (!isPuppeteerCore && product === 'firefox')
     preferredRevision = PUPPETEER_REVISIONS.firefox;
 
-  const puppeteer = new Puppeteer(
+  return new Puppeteer(
     puppeteerRootDirectory,
     preferredRevision,
     isPuppeteerCore,
     product
   );
-
-  // The introspection in `Helper.installAsyncStackHooks` references
-  // `Puppeteer._launcher` before the Puppeteer ctor is called, such that an
-  // invalid Launcher is selected at import, so we reset it.
-  puppeteer._lazyLauncher = undefined;
-  return puppeteer;
 };

Diff for: src/initialize.ts

@@ -441,6 +441,11 @@ describe('Launcher specs', function () {
 
       after(async () => {
         const { puppeteer } = getTestState();
+        /* launcher is a private property so we don't want our users doing this
+         * but we need to reset the state fully here for testing different
+         * browser launchers
+         */
+        // @ts-expect-error
         puppeteer._lazyLauncher = undefined;
         puppeteer._productName = productName;
       });

Diff for: test/launcher.spec.ts

@@ -119,21 +119,6 @@ describe('Page', function () {
     });
   });
 
-  describeFailsFirefox('Async stacks', () => {
-    it('should work', async () => {
-      const { page, server } = getTestState();
-
-      server.setRoute('/empty.html', (req, res) => {
-        res.statusCode = 204;
-        res.end();
-      });
-      let error = null;
-      await page.goto(server.EMPTY_PAGE).catch((error_) => (error = error_));
-      expect(error).not.toBe(null);
-      expect(error.stack).toContain(__filename);
-    });
-  });
-
   // This test fails on Firefox on CI consistently but cannot be replicated
   // locally. Skipping for now to unblock the Mitt release and given FF support
   // isn't fully done yet but raising an issue to ask the FF folks to have a