From 4f7d53ac66fab9b435f0a278aae0c24f07eea5f5 Mon Sep 17 00:00:00 2001
From: Yury Semikhatsky <yurys@chromium.org>
Date: Mon, 18 Oct 2021 19:41:56 -0700
Subject: [PATCH] fix(fetch): abort requests when context is disposed (#9601)

---
 packages/playwright-core/src/server/fetch.ts | 15 ++++++---
 tests/browsercontext-fetch.spec.ts           | 17 ++++++++++
 tests/global-fetch.spec.ts                   | 33 ++++++++++++++++++++
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts
index 942dd1eac0..b4c93ebc12 100644
--- a/packages/playwright-core/src/server/fetch.ts
+++ b/packages/playwright-core/src/server/fetch.ts
@@ -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) {
diff --git a/tests/browsercontext-fetch.spec.ts b/tests/browsercontext-fetch.spec.ts
index c8902dd46d..d89ef74f73 100644
--- a/tests/browsercontext-fetch.spec.ts
+++ b/tests/browsercontext-fetch.spec.ts
@@ -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;
+});
diff --git a/tests/global-fetch.spec.ts b/tests/global-fetch.spec.ts
index f32eb9bc98..a2b6999026 100644
--- a/tests/global-fetch.spec.ts
+++ b/tests/global-fetch.spec.ts
@@ -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;
+});