fix(fetch): abort requests when context is disposed (#9601)

This commit is contained in:
Yury Semikhatsky 2021-10-18 19:41:56 -07:00 committed by GitHub
parent ff16ac0351
commit 4f7d53ac66
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 61 additions and 4 deletions

View file

@ -211,7 +211,7 @@ export abstract class FetchRequest extends SdkObject {
if (redirectStatus.includes(response.statusCode!)) {
if (!options.maxRedirects) {
reject(new Error('Max redirect count exceeded'));
request.abort();
request.destroy();
return;
}
const headers = { ...options.headers };
@ -243,7 +243,7 @@ export abstract class FetchRequest extends SdkObject {
if (response.headers.location) {
const locationURL = new URL(response.headers.location, url);
fulfill(this._sendRequest(locationURL, redirectOptions, postData));
request.abort();
request.destroy();
return;
}
}
@ -255,7 +255,7 @@ export abstract class FetchRequest extends SdkObject {
const encoded = Buffer.from(`${username || ''}:${password || ''}`).toString('base64');
options.headers!['authorization'] = `Basic ${encoded}`;
fulfill(this._sendRequest(url, options, postData));
request.abort();
request.destroy();
return;
}
}
@ -297,6 +297,13 @@ export abstract class FetchRequest extends SdkObject {
});
request.on('error', reject);
const disposeListener = () => {
reject(new Error('Request context disposed.'));
request.destroy();
};
this.on(FetchRequest.Events.Dispose, disposeListener);
request.on('close', () => this.off(FetchRequest.Events.Dispose, disposeListener));
if (debugLogger.isEnabled('api')) {
debugLogger.log('api', `${options.method} ${url.toString()}`);
if (options.headers) {
@ -308,7 +315,7 @@ export abstract class FetchRequest extends SdkObject {
if (options.deadline) {
const rejectOnTimeout = () => {
reject(new Error(`Request timed out after ${options.timeout}ms`));
request.abort();
request.destroy();
};
const remaining = options.deadline - monotonicTime();
if (remaining <= 0) {

View file

@ -929,3 +929,20 @@ it('should accept bool and numeric params', async ({ page, server }) => {
expect(params.get('bool')).toEqual('true');
expect(params.get('bool2')).toEqual('false');
});
it('should abort requests when browser context closes', async ({ contextFactory, server }) => {
const connectionClosed = new Promise(resolve => {
server.setRoute('/empty.html', (req, res) => {
req.socket.on('close', resolve);
});
});
const context = await contextFactory();
const [ error ] = await Promise.all([
context.request.get(server.EMPTY_PAGE).catch(e => e),
context.request.post(server.EMPTY_PAGE).catch(e => e),
server.waitForRequest('/empty.html').then(() => context.close())
]);
expect(error instanceof Error).toBeTruthy();
expect(error.message).toContain('Request context disposed');
await connectionClosed;
});

View file

@ -177,3 +177,36 @@ it('should return empty body', async ({ playwright, server }) => {
const error = await response.body().catch(e => e);
expect(error.message).toContain('Response has been disposed');
});
it('should abort requests when context is disposed', async ({ playwright, server }) => {
const connectionClosed = new Promise(resolve => {
server.setRoute('/empty.html', req => req.socket.on('close', resolve));
});
const request = await playwright.request.newContext();
const results = await Promise.all([
request.get(server.EMPTY_PAGE).catch(e => e),
request.post(server.EMPTY_PAGE).catch(e => e),
request.delete(server.EMPTY_PAGE).catch(e => e),
server.waitForRequest('/empty.html').then(() => request.dispose())
]);
for (const result of results.slice(0, -1)) {
expect(result instanceof Error).toBeTruthy();
expect(result.message).toContain('Request context disposed');
}
await connectionClosed;
});
it('should abort redirected requests when context is disposed', async ({ playwright, server }) => {
server.setRedirect('/redirect', '/test');
const connectionClosed = new Promise(resolve => {
server.setRoute('/test', req => req.socket.on('close', resolve));
});
const request = await playwright.request.newContext();
const [result] = await Promise.all([
request.get(server.PREFIX + '/redirect').catch(e => e),
server.waitForRequest('/test').then(() => request.dispose())
]);
expect(result instanceof Error).toBeTruthy();
expect(result.message).toContain('Request context disposed');
await connectionClosed;
});