fix(route): catch all Route methods when page closes (#10140)

This fixes a common scenario where you setup a route,
and the page closes (e.g. test ends) while we are aborting/continuing
some requests that are not instrumental to the test itself.
This commit is contained in:
Dmitry Gozman 2021-11-08 15:13:15 -08:00 committed by GitHub
parent 4b14c466d4
commit 3cc839e013
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 40 additions and 5 deletions

View File

@ -228,9 +228,20 @@ export class Route extends ChannelOwner<channels.RouteChannel, channels.RouteIni
return Request.from(this._initializer.request);
}
private _raceWithPageClose(promise: Promise<any>): Promise<void> {
const page = this.request().frame()._page;
// When page closes or crashes, we catch any potential rejects from this Route.
// Note that page could be missing when routing popup's initial request that
// does not have a Page initialized just yet.
return Promise.race([
promise,
page ? page._closedOrCrashedPromise : Promise.resolve(),
]);
}
async abort(errorCode?: string) {
return this._wrapApiCall(async (channel: channels.RouteChannel) => {
await channel.abort({ errorCode });
await this._raceWithPageClose(channel.abort({ errorCode }));
});
}
@ -271,13 +282,13 @@ export class Route extends ChannelOwner<channels.RouteChannel, channels.RouteIni
if (length && !('content-length' in headers))
headers['content-length'] = String(length);
await channel.fulfill({
await this._raceWithPageClose(channel.fulfill({
status: statusOption || 200,
headers: headersObjectToArray(headers),
body,
isBase64,
fetchResponseUid
});
}));
});
}
@ -292,12 +303,12 @@ export class Route extends ChannelOwner<channels.RouteChannel, channels.RouteIni
private async _continue(options: { url?: string, method?: string, headers?: Headers, postData?: string | Buffer }, isInternal?: boolean) {
return await this._wrapApiCall(async (channel: channels.RouteChannel) => {
const postDataBuffer = isString(options.postData) ? Buffer.from(options.postData, 'utf8') : options.postData;
await channel.continue({
await this._raceWithPageClose(channel.continue({
url: options.url,
method: options.method,
headers: options.headers ? headersObjectToArray(options.headers) : undefined,
postData: postDataBuffer ? postDataBuffer.toString('base64') : undefined,
});
}));
}, undefined, isInternal);
}
}

View File

@ -77,6 +77,30 @@ it('should not allow changing protocol when overriding url', async ({ page, serv
expect(error.message).toContain('New URL must have same protocol as overridden URL');
});
it('should not throw when continuing while page is closing', async ({ page, server }) => {
let done;
await page.route('**/*', async route => {
done = Promise.all([
route.continue(),
page.close(),
]);
});
const error = await page.goto(server.EMPTY_PAGE).catch(e => e);
await done;
expect(error).toBeInstanceOf(Error);
});
it('should not throw when continuing after page is closed', async ({ page, server }) => {
let done;
await page.route('**/*', async route => {
await page.close();
done = route.continue();
});
const error = await page.goto(server.EMPTY_PAGE).catch(e => e);
await done;
expect(error).toBeInstanceOf(Error);
});
it('should override method along with url', async ({ page, server }) => {
const request = server.waitForRequest('/empty.html');
await page.route('**/foo', route => {