chore: migrate NetworkManager events (#6174)

This is part of the effort to remove `Events.ts` in favour of defining
events next to the class that emits them. In this case these events are
internal, so there's no docs changes, but it's still worth doing such
that we can remove the Events.ts file in the long term once all the
different events are migrated.

src/common/Events.ts

@@ -14,6 +14,19 @@
  * limitations under the License.
  */
 
+/**
+ * IMPORTANT: we are mid-way through migrating away from this Events.ts file
+ * in favour of defining events next to the class that emits them.
+ *
+ * However we need to maintain this file for now because the legacy DocLint
+ * system relies on them. Be aware in the mean time if you make a change here
+ * you probably need to replicate it in the relevant class. For example if you
+ * add a new Page event, you should update the PageEmittedEvents enum in
+ * src/common/Page.ts.
+ *
+ * Chat to @jackfranklin if you're unsure.
+ */
+
 export const Events = {
   Page: {
     Close: 'close',

src/common/LifecycleWatcher.ts

@@ -21,6 +21,7 @@ import { TimeoutError } from './Errors';
 import { FrameManager, Frame } from './FrameManager';
 import { HTTPRequest } from './HTTPRequest';
 import { HTTPResponse } from './HTTPResponse';
+import { NetworkManagerEmittedEvents } from './NetworkManager';
 
 export type PuppeteerLifeCycleEvent =
   | 'load'
@@ -117,7 +118,7 @@ export class LifecycleWatcher {
       ),
       helper.addEventListener(
         this._frameManager.networkManager(),
-        Events.NetworkManager.Request,
+        NetworkManagerEmittedEvents.Request,
         this._onRequest.bind(this)
       ),
     ];

src/common/NetworkManager.ts

@@ -17,7 +17,6 @@ import { EventEmitter } from './EventEmitter';
 import { assert } from './assert';
 import { helper, debugError } from './helper';
 import Protocol from '../protocol';
-import { Events } from './Events';
 import { CDPSession } from './Connection';
 import { FrameManager } from './FrameManager';
 import { HTTPRequest } from './HTTPRequest';
@@ -31,6 +30,19 @@ export interface Credentials {
   password: string;
 }
 
+/**
+ * We use symbols to prevent any external parties listening to these events.
+ * They are internal to Puppeteer.
+ *
+ * @internal
+ */
+export const NetworkManagerEmittedEvents = {
+  Request: Symbol('NetworkManager.Request'),
+  Response: Symbol('NetworkManager.Response'),
+  RequestFailed: Symbol('NetworkManager.RequestFailed'),
+  RequestFinished: Symbol('NetworkManager.RequestFinished'),
+} as const;
+
 /**
  * @internal
  */
@@ -264,7 +276,7 @@ export class NetworkManager extends EventEmitter {
       redirectChain
     );
     this._requestIdToRequest.set(event.requestId, request);
-    this.emit(Events.NetworkManager.Request, request);
+    this.emit(NetworkManagerEmittedEvents.Request, request);
   }
 
   _onRequestServedFromCache(
@@ -286,8 +298,8 @@ export class NetworkManager extends EventEmitter {
     );
     this._requestIdToRequest.delete(request._requestId);
     this._attemptedAuthentications.delete(request._interceptionId);
-    this.emit(Events.NetworkManager.Response, response);
+    this.emit(NetworkManagerEmittedEvents.Response, response);
-    this.emit(Events.NetworkManager.RequestFinished, request);
+    this.emit(NetworkManagerEmittedEvents.RequestFinished, request);
   }
 
   _onResponseReceived(event: Protocol.Network.responseReceivedPayload): void {
@@ -296,7 +308,7 @@ export class NetworkManager extends EventEmitter {
     if (!request) return;
     const response = new HTTPResponse(this._client, request, event.response);
     request._response = response;
-    this.emit(Events.NetworkManager.Response, response);
+    this.emit(NetworkManagerEmittedEvents.Response, response);
   }
 
   _onLoadingFinished(event: Protocol.Network.loadingFinishedPayload): void {
@@ -310,7 +322,7 @@ export class NetworkManager extends EventEmitter {
     if (request.response()) request.response()._resolveBody(null);
     this._requestIdToRequest.delete(request._requestId);
     this._attemptedAuthentications.delete(request._interceptionId);
-    this.emit(Events.NetworkManager.RequestFinished, request);
+    this.emit(NetworkManagerEmittedEvents.RequestFinished, request);
   }
 
   _onLoadingFailed(event: Protocol.Network.loadingFailedPayload): void {
@@ -323,6 +335,6 @@ export class NetworkManager extends EventEmitter {
     if (response) response._resolveBody(null);
     this._requestIdToRequest.delete(request._requestId);
     this._attemptedAuthentications.delete(request._interceptionId);
-    this.emit(Events.NetworkManager.RequestFailed, request);
+    this.emit(NetworkManagerEmittedEvents.RequestFailed, request);
   }
 }

src/common/Page.ts

@@ -33,7 +33,7 @@ import { Browser, BrowserContext } from './Browser';
 import { Target } from './Target';
 import { createJSHandle, JSHandle, ElementHandle } from './JSHandle';
 import { Viewport } from './PuppeteerViewport';
-import { Credentials } from './NetworkManager';
+import { Credentials, NetworkManagerEmittedEvents } from './NetworkManager';
 import { HTTPRequest } from './HTTPRequest';
 import { HTTPResponse } from './HTTPResponse';
 import { Accessibility } from './Accessibility';
@@ -486,16 +486,16 @@ export class Page extends EventEmitter {
     );
 
     const networkManager = this._frameManager.networkManager();
-    networkManager.on(Events.NetworkManager.Request, (event) =>
+    networkManager.on(NetworkManagerEmittedEvents.Request, (event) =>
       this.emit(PageEmittedEvents.Request, event)
     );
-    networkManager.on(Events.NetworkManager.Response, (event) =>
+    networkManager.on(NetworkManagerEmittedEvents.Response, (event) =>
       this.emit(PageEmittedEvents.Response, event)
     );
-    networkManager.on(Events.NetworkManager.RequestFailed, (event) =>
+    networkManager.on(NetworkManagerEmittedEvents.RequestFailed, (event) =>
       this.emit(PageEmittedEvents.RequestFailed, event)
     );
-    networkManager.on(Events.NetworkManager.RequestFinished, (event) =>
+    networkManager.on(NetworkManagerEmittedEvents.RequestFinished, (event) =>
       this.emit(PageEmittedEvents.RequestFinished, event)
     );
     this._fileChooserInterceptors = new Set();
@@ -1339,7 +1339,7 @@ export class Page extends EventEmitter {
     const { timeout = this._timeoutSettings.timeout() } = options;
     return helper.waitForEvent(
       this._frameManager.networkManager(),
-      Events.NetworkManager.Request,
+      NetworkManagerEmittedEvents.Request,
       (request) => {
         if (helper.isString(urlOrPredicate))
           return urlOrPredicate === request.url();
@@ -1359,7 +1359,7 @@ export class Page extends EventEmitter {
     const { timeout = this._timeoutSettings.timeout() } = options;
     return helper.waitForEvent(
       this._frameManager.networkManager(),
-      Events.NetworkManager.Response,
+      NetworkManagerEmittedEvents.Response,
       (response) => {
         if (helper.isString(urlOrPredicate))
           return urlOrPredicate === response.url();