Skip to content

Commit

Permalink
chore: migrate NetworkManager events (#6174)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jackfranklin committed Jul 7, 2020
1 parent 022495b commit 12ee70f
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 15 deletions.
13 changes: 13 additions & 0 deletions src/common/Events.ts
Expand Up @@ -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',
Expand Down
3 changes: 2 additions & 1 deletion src/common/LifecycleWatcher.ts
Expand Up @@ -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'
Expand Down Expand Up @@ -117,7 +118,7 @@ export class LifecycleWatcher {
),
helper.addEventListener(
this._frameManager.networkManager(),
Events.NetworkManager.Request,
NetworkManagerEmittedEvents.Request,
this._onRequest.bind(this)
),
];
Expand Down
26 changes: 19 additions & 7 deletions src/common/NetworkManager.ts
Expand Up @@ -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';
Expand All @@ -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
*/
Expand Down Expand Up @@ -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(
Expand All @@ -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(Events.NetworkManager.RequestFinished, request);
this.emit(NetworkManagerEmittedEvents.Response, response);
this.emit(NetworkManagerEmittedEvents.RequestFinished, request);
}

_onResponseReceived(event: Protocol.Network.responseReceivedPayload): void {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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);
}
}
14 changes: 7 additions & 7 deletions src/common/Page.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down

0 comments on commit 12ee70f

Please sign in to comment.