diff --git a/bin/lib/core.js b/bin/lib/core.js index ae77ed7..641d0a3 100644 --- a/bin/lib/core.js +++ b/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, }; diff --git a/bin/lib/handlers.js b/bin/lib/handlers.js index 6210335..68f6016 100644 --- a/bin/lib/handlers.js +++ b/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) }; } diff --git a/bin/playwright-server.js b/bin/playwright-server.js index 05fde22..561abf6 100644 --- a/bin/playwright-server.js +++ b/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) { diff --git a/src/Browser/BrowserContext.php b/src/Browser/BrowserContext.php index 5bf917d..e23edcd 100644 --- a/src/Browser/BrowserContext.php +++ b/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; } diff --git a/src/Network/Route.php b/src/Network/Route.php index e441658..1c34bed 100644 --- a/src/Network/Route.php +++ b/src/Network/Route.php @@ -20,6 +20,8 @@ final class Route implements RouteInterface { private RequestInterface $request; + private bool $handled = false; + /** * @param array $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,6 +90,8 @@ 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, @@ -87,6 +99,18 @@ public function fallback(?array $options = null): void ]); } + /** + * 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([ diff --git a/src/Page/Page.php b/src/Page/Page.php index 3a35201..44f4166 100644 --- a/src/Page/Page.php +++ b/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); diff --git a/src/Transport/JsonRpc/ProcessJsonRpcClient.php b/src/Transport/JsonRpc/ProcessJsonRpcClient.php index 26e5188..85bd4be 100644 --- a/src/Transport/JsonRpc/ProcessJsonRpcClient.php +++ b/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|null */ diff --git a/tests/Functional/Network/RouteTest.php b/tests/Functional/Network/RouteTest.php index 382850e..aa1089b 100644 --- a/tests/Functional/Network/RouteTest.php +++ b/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 = '
'"); + $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 = 'go'"); + $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']); + }; + } }