Don't leave the request hanging when a route handler throws

bin/lib/core.js

@@ -167,6 +167,15 @@ class CommandRegistry {
   }
 }
 
+class NavigationRedirects extends Map {
+  #waiters = new Map();
+  fulfilled(pageId) { return new Promise(resolve => this.#waiters.set(pageId, resolve)); }
+  markFulfilled(pageId) {
+    this.#waiters.get(pageId)?.();
+    this.#waiters.delete(pageId);
+  }
+}
+
 class BaseHandler {
   constructor(deps = {}) { Object.assign(this, deps); }
   validateResource(resourceMap, resourceId, resourceType) {
@@ -190,7 +199,7 @@ class BaseHandler {
     return { handleId };
   }
   async executeWithRegistry(registry, method, ...args) { return await registry.execute(method, ...args); }
-  async followNavigationRedirects(pageId, page, action) {
+  async followNavigationRedirects(pageId, page, action, settlesOnRedirect = false) {
     let nextAction = action;
     let redirectCount = 0;
     let result;
@@ -199,7 +208,10 @@ class BaseHandler {
       this.navigationRedirects.delete(pageId);
 
       try {
-        result = await nextAction();
+        // from Playwright 1.64 only goto() settles on the redirect's 204; a goto() to the same url before it does fails with net::ERR_ABORTED
+        result = await (settlesOnRedirect
+          ? nextAction()
+          : Promise.race([nextAction(), this.navigationRedirects.fulfilled(pageId)]));
       } catch (error) {
         if (!this.navigationRedirects.has(pageId)) throw error;
       }
@@ -212,6 +224,7 @@ class BaseHandler {
         throw new Error(`Too many navigation redirects for page ${pageId}`);
       }
       nextAction = () => page.goto(target);
+      settlesOnRedirect = true;
     }
   }
 }
@@ -330,5 +343,5 @@ const logger = new Logger();
 
 module.exports = {
   logger, ErrorHandler, LspFraming, sendFramedResponse,
-  CommandRegistry, BaseHandler, PromiseUtils, FrameUtils, RouteUtils,
+  CommandRegistry, NavigationRedirects, BaseHandler, PromiseUtils, FrameUtils, RouteUtils,
 };

bin/lib/handlers.js

@@ -440,7 +440,8 @@ class PageHandler extends BaseHandler {
     const gotoResponse = await this.followNavigationRedirects(
       command.pageId,
       page,
-      () => page.goto(command.url, command.options)
+      () => page.goto(command.url, command.options),
+      true
     );
     return { response: this.serializeResponse(gotoResponse) };
   }

bin/playwright-server.js

@@ -1,5 +1,5 @@
 const {chromium, firefox, webkit, request} = require('playwright');
-const { logger, ErrorHandler, LspFraming, sendFramedResponse, CommandRegistry, BaseHandler } = require('./lib/core');
+const { logger, ErrorHandler, LspFraming, sendFramedResponse, CommandRegistry, NavigationRedirects, BaseHandler } = require('./lib/core');
 const { APIRequestHandler, ContextHandler, PageHandler, LocatorHandler, InteractionHandler, FrameHandler, JSHandleHandler, SelectorsHandler, VideoHandler } = require('./lib/handlers');
 const { globalCoordinator } = require('./lib/coordination');
 
@@ -20,7 +20,7 @@ class PlaywrightServer extends BaseHandler {
     this.dialogs = new Map();
     this.elementHandles = new Map();
     this.contextThrottling = new Map();
-    this.navigationRedirects = new Map();
+    this.navigationRedirects = new NavigationRedirects();
     this.servers = new Map();
     this.videos = new Map();
     this.apiContexts = new Map();
@@ -148,6 +148,7 @@ class PlaywrightServer extends BaseHandler {
     if (!this.pages.has(info.contextId)) throw new Error('Navigation redirects require a page route');
     this.navigationRedirects.set(info.contextId, command.url);
     await route.fulfill({ status: 204, headers: command.options?.headers });
+    this.navigationRedirects.markFulfilled(info.contextId);
   }
 
   async continueRoute(route, info, command) {

src/Browser/BrowserContext.php

@@ -124,7 +124,13 @@ public function dispatchEvent(string $eventName, array $params): void
             );
             foreach ($this->routeHandlers as $handler) {
                 if (fnmatch($handler['url'], $route->request()->url())) {
-                    $handler['handler']($route);
+                    try {
+                        $handler['handler']($route);
+                    } catch (\Throwable $e) {
+                        $route->abortUnlessHandled();
+
+                        throw $e;
+                    }
 
                     return;
                 }

src/Network/Route.php

@@ -20,6 +20,8 @@ final class Route implements RouteInterface
 {
     private RequestInterface $request;
 
+    private bool $handled = false;
+
     /**
      * @param array<string, mixed> $requestData
      */
@@ -38,6 +40,8 @@ public function request(): RequestInterface
 
     public function abort(string $errorCode = 'failed'): void
     {
+        $this->handled = true;
+
         $this->transport->sendAsync([
             'action' => 'route.abort',
             'routeId' => $this->routeId,
@@ -47,6 +51,8 @@ public function abort(string $errorCode = 'failed'): void
 
     public function continue(?array $options = null): void
     {
+        $this->handled = true;
+
         $this->transport->sendAsync([
             'action' => 'route.continue',
             'routeId' => $this->routeId,
@@ -61,6 +67,8 @@ public function continue(?array $options = null): void
      */
     public function redirectNavigationRequest(string $url, array $options = []): void
     {
+        $this->handled = true;
+
         $this->transport->sendAsync([
             'action' => 'route.redirectNavigationRequest',
             'routeId' => $this->routeId,
@@ -71,6 +79,8 @@ public function redirectNavigationRequest(string $url, array $options = []): voi
 
     public function fulfill(array $options): void
     {
+        $this->handled = true;
+
         $this->transport->sendAsync([
             'action' => 'route.fulfill',
             'routeId' => $this->routeId,
@@ -80,13 +90,27 @@ public function fulfill(array $options): void
 
     public function fallback(?array $options = null): void
     {
+        $this->handled = true;
+
         $this->transport->sendAsync([
             'action' => 'route.fallback',
             'routeId' => $this->routeId,
             'options' => $options,
         ]);
     }
 
+    /**
+     * A handler that throws must not leave its request hanging in the browser.
+     *
+     * @internal
+     */
+    public function abortUnlessHandled(): void
+    {
+        if (!$this->handled) {
+            $this->abort('aborted');
+        }
+    }
+
     public function fetch(?array $options = null): ResponseInterface
     {
         $response = $this->transport->send([

src/Page/Page.php

@@ -191,7 +191,13 @@ public function dispatchEvent(string $eventName, array $params): void
                 } else {
                     break;
                 }
-                $this->eventHandler->publicEmit('route', [$route]);
+                try {
+                    $this->eventHandler->publicEmit('route', [$route]);
+                } catch (\Throwable $e) {
+                    $route->abortUnlessHandled();
+
+                    throw $e;
+                }
                 break;
             default:
                 $this->eventHandler->publicEmit($eventName, $params);

src/Transport/JsonRpc/ProcessJsonRpcClient.php

@@ -37,6 +37,7 @@ final class ProcessJsonRpcClient extends JsonRpcClient implements JsonRpcClientI
     private array $responses = [];
 
     private ?\Closure $eventHandler = null;
+    private ?\Throwable $eventHandlerException = null;
 
     public function __construct(
         private readonly Process $process,
@@ -113,6 +114,8 @@ private function waitForResponse(int|string $requestId, ?float $deadline): array
 
             $response = $this->findResponseForRequest($requestId);
             if (null !== $response) {
+                $this->throwEventHandlerException();
+
                 return $response;
             }
 
@@ -205,7 +208,12 @@ private function dispatchMessage(string $messageContent): void
             if (isset($data['event'])) {
                 $this->logger->debug('Received event from process', ['event' => $data['event']]);
                 if ($this->eventHandler) {
-                    ($this->eventHandler)($data);
+                    try {
+                        ($this->eventHandler)($data);
+                    } catch (\Throwable $e) {
+                        // unwinding now would abandon the request in flight while the browser carries on with it
+                        $this->eventHandlerException ??= $e;
+                    }
                 }
             }
         } catch (\JsonException $e) {
@@ -221,6 +229,17 @@ private function dispatchMessage(string $messageContent): void
         }
     }
 
+    private function throwEventHandlerException(): void
+    {
+        if (null === $exception = $this->eventHandlerException) {
+            return;
+        }
+
+        $this->eventHandlerException = null;
+
+        throw $exception;
+    }
+
     /**
      * @return array<string, mixed>|null
      */

tests/Functional/Network/RouteTest.php

@@ -221,6 +221,20 @@ public function testCanRedirectNavigationRequestAfterClick(): void
         $this->assertSame($baseUrl.'/page-2.html', $this->page->url());
     }
 
+    public function testCanRedirectNavigationRequestAfterFormSubmit(): void
+    {
+        $baseUrl = $this->getBaseUrl();
+        $this->goto('/index.html');
+        $this->page->evaluate("document.body.innerHTML = '<form method=\"post\" action=\"/login\"><button>Log in</button></form>'");
+        $this->page->route('**/login', static function (Route $route) use ($baseUrl): void {
+            $route->redirectNavigationRequest($baseUrl.'/page-2.html');
+        });
+
+        $this->page->locator('button')->click();
+
+        $this->assertSame($baseUrl.'/page-2.html', $this->page->url());
+    }
+
     public function testCanRedirectNavigationRequestAfterReload(): void
     {
         $baseUrl = $this->getBaseUrl();
@@ -290,4 +304,69 @@ public function testRedirectNavigationRequestPreservesResponseCookies(): void
         $cookies = $this->context->cookies([$baseUrl]);
         $this->assertSame('yes', array_column($cookies, 'value', 'name')['redirected'] ?? null);
     }
+
+    public function testRouteHandlerExceptionIsThrownOnceTheNavigationSettles(): void
+    {
+        $url = $this->getBaseUrl().'/throws';
+        $this->page->route('**/throws', $this->throwingOnceHandler());
+
+        try {
+            $this->page->goto($url);
+            $this->fail('The route handler exception was not thrown.');
+        } catch (\RuntimeException $e) {
+            $this->assertSame('handler failed', $e->getMessage());
+        }
+
+        $this->page->goto($url);
+
+        $this->assertSame('recovered', $this->page->locator('body')->textContent());
+    }
+
+    public function testRouteHandlerExceptionIsThrownOnceTheClickSettles(): void
+    {
+        $this->goto('/index.html');
+        $this->page->evaluate("document.body.innerHTML = '<a href=\"/throws\">go</a>'");
+        $this->page->route('**/throws', $this->throwingOnceHandler());
+
+        try {
+            $this->page->locator('a')->click();
+            $this->fail('The route handler exception was not thrown.');
+        } catch (\RuntimeException $e) {
+            $this->assertSame('handler failed', $e->getMessage());
+        }
+
+        $this->page->goto($this->getBaseUrl().'/throws');
+
+        $this->assertSame('recovered', $this->page->locator('body')->textContent());
+    }
+
+    public function testContextRouteHandlerExceptionIsThrownOnceTheNavigationSettles(): void
+    {
+        $url = $this->getBaseUrl().'/throws';
+        $this->context->route('**/throws', $this->throwingOnceHandler());
+
+        try {
+            $this->page->goto($url);
+            $this->fail('The route handler exception was not thrown.');
+        } catch (\RuntimeException $e) {
+            $this->assertSame('handler failed', $e->getMessage());
+        }
+
+        $this->page->goto($url);
+
+        $this->assertSame('recovered', $this->page->locator('body')->textContent());
+    }
+
+    private function throwingOnceHandler(): \Closure
+    {
+        $calls = 0;
+
+        return static function (Route $route) use (&$calls): void {
+            if (1 === ++$calls) {
+                throw new \RuntimeException('handler failed');
+            }
+
+            $route->fulfill(['status' => 200, 'contentType' => 'text/html', 'body' => 'recovered']);
+        };
+    }
 }