From 824242246de9e158aacb85f71350a79cb386ed92 Mon Sep 17 00:00:00 2001 From: Ben Allfree Date: Thu, 9 Dec 2021 06:57:40 -0800 Subject: [PATCH] fix!: typo in 'already-handled' constant of the request interception API (#7813) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issues: #7745, #7747, #7780 Co-authored-by: Rodrigo Fernández --- docs/api.md | 37 +++++++-------- src/common/HTTPRequest.ts | 47 ++++++++++++------- test/requestinterception-experimental.spec.ts | 5 +- 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/docs/api.md b/docs/api.md index 50d882b457a70..eec5b8c4eaa90 100644 --- a/docs/api.md +++ b/docs/api.md @@ -2458,13 +2458,11 @@ Here is the example above rewritten using `request.interceptResolutionState` ```js /* This first handler will succeed in calling request.continue because the request interception has never been resolved. - -Note: `alreay-handled` is misspelled but likely won't be fixed until v13. https://github.com/puppeteer/puppeteer/pull/7780 */ page.on('request', (interceptedRequest) => { // The interception has not been handled yet. Control will pass through this guard. const { action } = interceptedRequest.interceptResolutionState(); - if (action === 'alreay-handled') return; + if (action === InterceptResolutionAction.AlreadyHandled) return; // It is not strictly necessary to return a promise, but doing so will allow Puppeteer to await this handler. return new Promise(resolve => { @@ -2473,7 +2471,7 @@ page.on('request', (interceptedRequest) => { // Inside, check synchronously to verify that the intercept wasn't handled already. // It might have been handled during the 500ms while the other handler awaited an async op of its own. const { action } = interceptedRequest.interceptResolutionState(); - if (action === 'alreay-handled') { + if (action === InterceptResolutionAction.AlreadyHandled) { resolve(); return; }; @@ -2484,12 +2482,12 @@ page.on('request', (interceptedRequest) => { }); page.on('request', async (interceptedRequest) => { // The interception has not been handled yet. Control will pass through this guard. - if (interceptedRequest.interceptResolutionState().action === 'alreay-handled') return; + if (interceptedRequest.interceptResolutionState().action === InterceptResolutionAction.AlreadyHandled) return; await someLongAsyncOperation() // The interception *MIGHT* have been handled by the first handler, we can't be sure. // Therefore, we must check again before calling continue() or we risk Puppeteer raising an exception. - if (interceptedRequest.interceptResolutionState().action === 'alreay-handled') return; + if (interceptedRequest.interceptResolutionState().action === InterceptResolutionAction.AlreadyHandled) return; interceptedRequest.continue(); }); ``` @@ -2547,14 +2545,14 @@ page.on('request', (request) => { // Control reaches this point because the request was cooperatively aborted which postpones resolution. - // { action: 'abort', priority: 0 }, because abort @ 0 is the current winning resolution + // { action: InterceptResolutionAction.Abort, priority: 0 }, because abort @ 0 is the current winning resolution console.log(request.interceptResolutionState()); // Legacy Mode: intercept continues immediately. request.continue({}); }); page.on('request', (request) => { - // { action: 'alreay-handled' }, because continue in Legacy Mode was called + // { action: InterceptResolutionAction.AlreadyHandled }, because continue in Legacy Mode was called console.log(request.interceptResolutionState()); }); @@ -2578,7 +2576,7 @@ page.on('request', (request) => { request.continue(request.continueRequestOverrides(), 5); }); page.on('request', (request) => { - // { action: 'continue', priority: 5 }, because continue @ 5 > abort @ 0 + // { action: InterceptResolutionAction.Continue, priority: 5 }, because continue @ 5 > abort @ 0 console.log(request.interceptResolutionState()); }); ``` @@ -2613,24 +2611,24 @@ page.on('request', (request) => { request.respond(request.responseForRequest(), 12); }); page.on('request', (request) => { - // { action: 'respond', priority: 15 }, because respond @ 15 > continue @ 15 > respond @ 12 > abort @ 10 + // { action: InterceptResolutionAction.Respond, priority: 15 }, because respond @ 15 > continue @ 15 > respond @ 12 > abort @ 10 console.log(request.interceptResolutionState()); }); ``` ##### Cooperative Request Continuation -Puppeteer requires `request.continue` to be called explicitly or the request will hang. Even if -your handler means to take no special action, or 'opt out', `request.continue` must still be called. +Puppeteer requires `request.continue()` to be called explicitly or the request will hang. Even if +your handler means to take no special action, or 'opt out', `request.continue()` must still be called. With the introduction of Cooperative Intercept Mode, two use cases arise for cooperative request continuations: Unopinionated and Opinionated. -The first case (common) is that your handler means to opt out of doing anything special the request. It has no opinion on further action and simply intends to continue by default and/or defer to other handlers that might have an opinion. But in case there are no other handlers, we must call `request.continue` to ensure that the request doesn't hang. +The first case (common) is that your handler means to opt out of doing anything special the request. It has no opinion on further action and simply intends to continue by default and/or defer to other handlers that might have an opinion. But in case there are no other handlers, we must call `request.continue()` to ensure that the request doesn't hang. We call this an **Unopinionated continuation** because the intent is to continue the request if nobody else has a better idea. Use `request.continue({...}, DEFAULT_INTERCEPT_RESOLUTION_PRIORITY)` (or `0`) for this type of continuation. -The second case (uncommon) is that your handler actually does have an opinion and means to force continuation by overriding a lower-priority `abort` or `respond` issued elsewhere. We call this an **Opinionated continuation**. In these rare cases where you mean to specify an overriding continuation priority, use a custom priority. +The second case (uncommon) is that your handler actually does have an opinion and means to force continuation by overriding a lower-priority `abort()` or `respond()` issued elsewhere. We call this an **Opinionated continuation**. In these rare cases where you mean to specify an overriding continuation priority, use a custom priority. To summarize, reason through whether your use of `request.continue` is just meant to be default/bypass behavior vs falling within the intended use case of your handler. Consider using a custom priority for in-scope use cases, and a default priority otherwise. Be aware that your handler may have both Opinionated and Unopinionated cases. @@ -5018,8 +5016,7 @@ When in Cooperative Intercept Mode, awaits pending interception handlers and the #### httpRequest.interceptResolutionState() - returns: <[InterceptResolutionState]> - - `action` <[InterceptResolutionAction]> Current resolution action. Possible values: `abort`, `respond`, `continue`, - `disabled`, `none`, and `alreay-handled` + - `action` <[InterceptResolutionAction]> Current resolution action. - `priority` The current priority of the winning action. `InterceptResolutionAction` is one of: @@ -5029,17 +5026,17 @@ When in Cooperative Intercept Mode, awaits pending interception handlers and the - `continue` - The request will be continued if no higher priority arises. - `disabled` - Request interception is not currently enabled (see `page.setRequestInterception`). - `none` - `abort/continue/respond` have not been called yet. -- `alreay-handled` - The interception has already been handled in Legacy Mode by a call to `abort/continue/respond` with +- `already-handled` - The interception has already been handled in Legacy Mode by a call to `abort/continue/respond` with a `priority` of `undefined`. Subsequent calls to `abort/continue/respond` will throw an exception. -This example will `continue` a request at a slightly higher priority than the current action if the interception has not +This example will `continue()` a request at a slightly higher priority than the current action if the interception has not already handled and is not already being continued. ```js page.on('request', (interceptedRequest) => { const { action, priority } = interceptedRequest.interceptResolutionState(); - if (action === 'alreay-handled') return; - if (action === 'continue') return; + if (action === InterceptResolutionAction.AlreadyHandled) return; + if (action === InterceptResolutionAction.Continue) return; // Change the action to `continue` and bump the priority so `continue` becomes the new winner interceptedRequest.continue( diff --git a/src/common/HTTPRequest.ts b/src/common/HTTPRequest.ts index 6fa8928506623..a9e0f2c419e62 100644 --- a/src/common/HTTPRequest.ts +++ b/src/common/HTTPRequest.ts @@ -180,7 +180,7 @@ export class HTTPRequest { this._frame = frame; this._redirectChain = redirectChain; this._continueRequestOverrides = {}; - this._interceptResolutionState = { action: 'none' }; + this._interceptResolutionState = { action: InterceptResolutionAction.None }; this._interceptHandlers = []; this._initiator = event.initiator; @@ -231,11 +231,13 @@ export class HTTPRequest { * priority?: number * * InterceptResolutionAction is one of: `abort`, `respond`, `continue`, - * `disabled`, `none`, or `alreay-handled`. + * `disabled`, `none`, or `already-handled`. */ interceptResolutionState(): InterceptResolutionState { - if (!this._allowInterception) return { action: 'disabled' }; - if (this._interceptionHandled) return { action: 'alreay-handled' }; + if (!this._allowInterception) + return { action: InterceptResolutionAction.Disabled }; + if (this._interceptionHandled) + return { action: InterceptResolutionAction.AlreadyHandled }; return { ...this._interceptResolutionState }; } @@ -441,7 +443,10 @@ export class HTTPRequest { priority > this._interceptResolutionState.priority || this._interceptResolutionState.priority === undefined ) { - this._interceptResolutionState = { action: 'continue', priority }; + this._interceptResolutionState = { + action: InterceptResolutionAction.Continue, + priority, + }; return; } if (priority === this._interceptResolutionState.priority) { @@ -451,7 +456,8 @@ export class HTTPRequest { ) { return; } - this._interceptResolutionState.action = 'continue'; + this._interceptResolutionState.action = + InterceptResolutionAction.Continue; } return; } @@ -527,14 +533,17 @@ export class HTTPRequest { priority > this._interceptResolutionState.priority || this._interceptResolutionState.priority === undefined ) { - this._interceptResolutionState = { action: 'respond', priority }; + this._interceptResolutionState = { + action: InterceptResolutionAction.Respond, + priority, + }; return; } if (priority === this._interceptResolutionState.priority) { if (this._interceptResolutionState.action === 'abort') { return; } - this._interceptResolutionState.action = 'respond'; + this._interceptResolutionState.action = InterceptResolutionAction.Respond; } } @@ -605,7 +614,10 @@ export class HTTPRequest { priority >= this._interceptResolutionState.priority || this._interceptResolutionState.priority === undefined ) { - this._interceptResolutionState = { action: 'abort', priority }; + this._interceptResolutionState = { + action: InterceptResolutionAction.Abort, + priority, + }; return; } } @@ -626,18 +638,19 @@ export class HTTPRequest { /** * @public */ -export type InterceptResolutionAction = - | 'abort' - | 'respond' - | 'continue' - | 'disabled' - | 'none' - | 'alreay-handled'; +export enum InterceptResolutionAction { + Abort = 'abort', + Respond = 'respond', + Continue = 'continue', + Disabled = 'disabled', + None = 'none', + AlreadyHandled = 'already-handled', +} /** * @public * - * Deprecate ASAP + * @deprecated please use {@link InterceptResolutionAction} instead. */ export type InterceptResolutionStrategy = InterceptResolutionAction; diff --git a/test/requestinterception-experimental.spec.ts b/test/requestinterception-experimental.spec.ts index 9f0203ee301d9..e16a715c0ea52 100644 --- a/test/requestinterception-experimental.spec.ts +++ b/test/requestinterception-experimental.spec.ts @@ -25,6 +25,7 @@ import { describeFailsFirefox, } from './mocha-utils'; // eslint-disable-line import/extensions import { ActionResult } from '../lib/cjs/puppeteer/api-docs-entry.js'; +import { InterceptResolutionAction } from '../lib/cjs/puppeteer/common/HTTPRequest.js'; describe('request interception', function () { setupTestBrowserHooks(); @@ -845,7 +846,7 @@ describe('request interception', function () { 'Yo, page!' ); }); - it('should indicate alreay-handled if an intercept has been handled', async () => { + it('should indicate already-handled if an intercept has been handled', async () => { const { page, server } = getTestState(); await page.setRequestInterception(true); @@ -857,7 +858,7 @@ describe('request interception', function () { }); page.on('request', (request) => { const { action } = request.interceptResolutionState(); - expect(action).toBe('alreay-handled'); + expect(action).toBe(InterceptResolutionAction.AlreadyHandled); }); await page.goto(server.EMPTY_PAGE); });