cherry-pick(release-1.18): fix: do not mute route handler exceptions (#…

…11572) PR: #11555
microsoft · Jan 24, 2022 · 7dcd171 · 7dcd171
1 parent 2717e61
commit 7dcd171
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 14 deletions.
diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts
@@ -146,10 +146,14 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
   _onRoute(route: network.Route, request: network.Request) {
     for (const routeHandler of this._routes) {
       if (routeHandler.matches(request.url())) {
-        if (routeHandler.handle(route, request)) {
-          this._routes.splice(this._routes.indexOf(routeHandler), 1);
-          if (!this._routes.length)
-            this._wrapApiCall(() => this._disableInterception(), true).catch(() => {});
+        try {
+          routeHandler.handle(route, request);
+        } finally {
+          if (!routeHandler.isActive()) {
+            this._routes.splice(this._routes.indexOf(routeHandler), 1);
+            if (!this._routes.length)
+              this._wrapApiCall(() => this._disableInterception(), true).catch(() => {});
+          }
         }
         return;
       }

diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts
@@ -514,12 +514,13 @@ export class RouteHandler {
     return urlMatches(this._baseURL, requestURL, this.url);
   }
 
-  public handle(route: Route, request: Request): boolean {
-    try {
-      this.handler(route, request);
-    } finally {
-      return ++this.handledCount >= this._times;
-    }
+  public handle(route: Route, request: Request): void {
+    ++this.handledCount;
+    this.handler(route, request);
+  }
+
+  public isActive(): boolean {
+    return this.handledCount < this._times;
   }
 }
 

diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts
@@ -170,10 +170,14 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
   private _onRoute(route: Route, request: Request) {
     for (const routeHandler of this._routes) {
       if (routeHandler.matches(request.url())) {
-        if (routeHandler.handle(route, request)) {
-          this._routes.splice(this._routes.indexOf(routeHandler), 1);
-          if (!this._routes.length)
-            this._wrapApiCall(() => this._disableInterception(), true).catch(() => {});
+        try {
+          routeHandler.handle(route, request);
+        } finally {
+          if (!routeHandler.isActive()) {
+            this._routes.splice(this._routes.indexOf(routeHandler), 1);
+            if (!this._routes.length)
+              this._wrapApiCall(() => this._disableInterception(), true).catch(() => {});
+          }
         }
         return;
       }

diff --git a/tests/playwright-test/playwright.unhandled.spec.ts b/tests/playwright-test/playwright.unhandled.spec.ts
@@ -0,0 +1,85 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { test, expect } from './playwright-test-fixtures';
+
+test('should lead in uncaughtException when page.route raises', async ({ runInlineTest, server }) => {
+  const result = await runInlineTest({
+    'a.test.ts': `
+      const { test } = pwt;
+      test('fail', async ({ page }) => {
+        test.fail();
+        await page.route('**/empty.html', route => {
+          throw new Error('foobar');
+        });
+        await page.goto('${server.EMPTY_PAGE}');
+      });
+    `,
+  }, { workers: 1 });
+  expect(result.skipped).toBe(1);
+  expect(result.output).toContain('foobar');
+});
+
+test('should lead in unhandledRejection when page.route raises', async ({ runInlineTest, server }) => {
+  const result = await runInlineTest({
+    'a.test.ts': `
+      const { test } = pwt;
+      test('fail', async ({ page }) => {
+        test.fail();
+        await page.route('**/empty.html', async route => {
+          throw new Error('foobar');
+        });
+        await page.goto('${server.EMPTY_PAGE}');
+      });
+    `,
+  }, { workers: 1 });
+  expect(result.skipped).toBe(1);
+  expect(result.output).toContain('foobar');
+});
+
+test('should lead in uncaughtException when context.route raises', async ({ runInlineTest, server }) => {
+  const result = await runInlineTest({
+    'a.test.ts': `
+      const { test } = pwt;
+      test('fail', async ({ context, page }) => {
+        test.fail();
+        await context.route('**/empty.html', route => {
+          throw new Error('foobar');
+        });
+        await page.goto('${server.EMPTY_PAGE}');
+      });
+    `,
+  }, { workers: 1 });
+  expect(result.skipped).toBe(1);
+  expect(result.output).toContain('foobar');
+});
+
+test('should lead in unhandledRejection when context.route raises', async ({ runInlineTest, server }) => {
+  const result = await runInlineTest({
+    'a.test.ts': `
+      const { test } = pwt;
+      test('fail', async ({ context, page }) => {
+        test.fail();
+        await context.route('**/empty.html', async route => {
+          throw new Error('foobar');
+        });
+        await page.goto('${server.EMPTY_PAGE}');
+      });
+    `,
+  }, { workers: 1 });
+  expect(result.skipped).toBe(1);
+  expect(result.output).toContain('foobar');
+});