-
-
Notifications
You must be signed in to change notification settings - Fork 88
fix: Prevent silent crashes on WebSocket failure and add cookie persistence #197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| namespace Pest\Browser\Api; | ||
|
|
||
| use InvalidArgumentException; | ||
| use Pest\Browser\Enums\BrowserType; | ||
| use Pest\Browser\Enums\ColorScheme; | ||
| use Pest\Browser\Enums\Device; | ||
|
|
@@ -154,6 +155,49 @@ public function geolocation(float $latitude, float $longitude): self | |
| ]); | ||
| } | ||
|
|
||
| /** | ||
| * Sets the storage state (cookies, localStorage) for the page context. | ||
| * | ||
| * This allows you to reuse authentication state from a previous session. | ||
| * | ||
| * @param array{cookies?: array<array{name: string, value: string, domain?: string, path?: string, expires?: float, httpOnly?: bool, secure?: bool, sameSite?: string}>, origins?: array<array{origin: string, localStorage: array<array{name: string, value: string}>}>} $storageState | ||
| */ | ||
| public function withStorageState(array $storageState): self | ||
| { | ||
| return new self($this->browserType, $this->device, $this->url, [ | ||
| 'storageState' => $storageState, | ||
| ...$this->options, | ||
| ]); | ||
| } | ||
|
|
||
| /** | ||
| * Loads storage state from a file path. | ||
| * | ||
| * The file should contain JSON with cookies and localStorage data | ||
| * from a previous Context::storageState() call. | ||
| */ | ||
| public function withStorageStateFromFile(string $path): self | ||
| { | ||
| if (! file_exists($path)) { | ||
| throw new InvalidArgumentException("Storage state file not found: $path"); | ||
| } | ||
|
|
||
| $contents = file_get_contents($path); | ||
|
|
||
| if ($contents === false) { | ||
| throw new InvalidArgumentException("Could not read storage state file: $path"); | ||
| } | ||
|
|
||
| /** @var array{cookies?: array, origins?: array}|null $storageState */ | ||
| $storageState = json_decode($contents, true); | ||
|
|
||
| if ($storageState === null) { | ||
| throw new InvalidArgumentException("Invalid storage state JSON in: $path"); | ||
| } | ||
|
|
||
| return $this->withStorageState($storageState); | ||
| } | ||
|
Comment on lines
+165
to
+199
|
||
|
|
||
| /** | ||
| * Creates the webpage instance. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| use Generator; | ||
| use Pest\Browser\Exceptions\PlaywrightOutdatedException; | ||
| use PHPUnit\Framework\ExpectationFailedException; | ||
| use RuntimeException; | ||
|
|
||
| use function Amp\Websocket\Client\connect; | ||
|
|
||
|
|
@@ -75,6 +76,9 @@ public function execute(string $guid, string $method, array $params = [], array | |
| assert($this->websocketConnection instanceof WebsocketConnection, 'WebSocket client is not connected.'); | ||
|
|
||
| $requestId = uniqid(); | ||
| $startTime = hrtime(true); | ||
| $operationTimeout = $params['timeout'] ?? $this->timeout; | ||
| $maxWaitTimeNs = $operationTimeout * 1_000_000; // Convert ms to ns | ||
|
|
||
| $requestJson = (string) json_encode([ | ||
| 'id' => $requestId, | ||
|
|
@@ -87,10 +91,33 @@ public function execute(string $guid, string $method, array $params = [], array | |
| $this->websocketConnection->sendText($requestJson); | ||
|
|
||
| while (true) { | ||
| // Check for timeout to prevent infinite loops | ||
| $elapsed = hrtime(true) - $startTime; | ||
| if ($elapsed > $maxWaitTimeNs) { | ||
| throw new RuntimeException( | ||
| "Playwright operation '$method' timed out after ".round($elapsed / 1_000_000_000, 2).' seconds' | ||
| ); | ||
| } | ||
|
Comment on lines
+94
to
+100
|
||
|
|
||
| $responseJson = $this->fetch($this->websocketConnection); | ||
| /** @var array{id: string|null, params: array{add: string|null}, error: array{error: array{message: string|null}}} $response */ | ||
|
|
||
| // Handle null responses (WebSocket connection lost) | ||
| if ($responseJson === null) { | ||
| throw new RuntimeException( | ||
| "WebSocket connection lost while executing '$method' on '$guid'" | ||
| ); | ||
| } | ||
|
|
||
| /** @var array{id: string|null, params: array{add: string|null}, error: array{error: array{message: string|null}}}|null $response */ | ||
| $response = json_decode($responseJson, true); | ||
|
|
||
| // Handle JSON decode failure | ||
| if ($response === null) { | ||
| throw new RuntimeException( | ||
| 'Invalid JSON response from Playwright server: '.substr($responseJson, 0, 100) | ||
| ); | ||
| } | ||
|
Comment on lines
+115
to
+119
|
||
|
|
||
| if (isset($response['error']['error']['message'])) { | ||
| $message = $response['error']['error']['message']; | ||
|
|
||
|
|
@@ -130,9 +157,17 @@ public function timeout(): int | |
|
|
||
| /** | ||
| * Fetches the response from the Playwright server. | ||
| * | ||
| * Returns null if the WebSocket connection is closed. | ||
| */ | ||
| private function fetch(WebsocketConnection $client): string | ||
| private function fetch(WebsocketConnection $client): ?string | ||
| { | ||
| return (string) $client->receive()?->read(); | ||
| $message = $client->receive(); | ||
|
|
||
| if ($message === null) { | ||
| return null; // Connection closed | ||
| } | ||
|
|
||
| return $message->read(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,4 +102,67 @@ public function addInitScript(string $script): self | |
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the storage state (cookies, localStorage, sessionStorage). | ||
| * | ||
| * @return array{cookies: array<array{name: string, value: string, domain: string, path: string, expires: float, httpOnly: bool, secure: bool, sameSite: string}>, origins: array<array{origin: string, localStorage: array<array{name: string, value: string}>}>} | ||
| */ | ||
| public function storageState(): array | ||
| { | ||
| $response = $this->sendMessage('storageState'); | ||
|
|
||
| /** @var array{result: array{cookies: array, origins: array}} $message */ | ||
| foreach ($response as $message) { | ||
| if (isset($message['result'])) { | ||
| return $message['result']; | ||
| } | ||
| } | ||
|
|
||
| return ['cookies' => [], 'origins' => []]; | ||
| } | ||
|
|
||
| /** | ||
| * Adds cookies into this browser context. | ||
| * | ||
| * @param array<array{name: string, value: string, domain?: string, path?: string, expires?: float, httpOnly?: bool, secure?: bool, sameSite?: string}> $cookies | ||
| */ | ||
| public function addCookies(array $cookies): self | ||
| { | ||
| $response = $this->sendMessage('addCookies', ['cookies' => $cookies]); | ||
| $this->processVoidResponse($response); | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * Gets all cookies in this browser context. | ||
| * | ||
| * @param array<string> $urls Optional URLs to filter cookies | ||
| * @return array<array{name: string, value: string, domain: string, path: string, expires: float, httpOnly: bool, secure: bool, sameSite: string}> | ||
| */ | ||
| public function cookies(array $urls = []): array | ||
| { | ||
| $response = $this->sendMessage('cookies', $urls !== [] ? ['urls' => $urls] : []); | ||
|
|
||
| /** @var array{result: array{cookies: array}} $message */ | ||
| foreach ($response as $message) { | ||
| if (isset($message['result']['cookies'])) { | ||
| return $message['result']['cookies']; | ||
| } | ||
| } | ||
|
|
||
| return []; | ||
| } | ||
|
|
||
| /** | ||
| * Clears all cookies from this browser context. | ||
| */ | ||
| public function clearCookies(): self | ||
| { | ||
| $response = $this->sendMessage('clearCookies'); | ||
| $this->processVoidResponse($response); | ||
|
|
||
| return $this; | ||
| } | ||
|
Comment on lines
+111
to
+167
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The withStorageStateFromFile method is missing validation for the JSON decode error case. While it checks if json_decode returns null, this could be due to either invalid JSON or valid JSON containing null. Consider using json_last_error() to differentiate between these cases and provide a more accurate error message.