fix: fail when acting on the browser after an expected exception

src/Browser.php

@@ -120,12 +120,12 @@ final public function assertNotContains(string $expected): self
 
     final public function crawler(): Crawler
     {
-        return $this->client()->getCrawler();
+        return $this->session()->crawler();
     }
 
     final public function content(): string
     {
-        return $this->session()->page()->getContent();
+        return $this->session()->content();
     }
 
     /**
@@ -223,7 +223,7 @@ final public function assertElementAttributeNotContains(string $selector, string
      */
     public function assertStatus(int $expected): self
     {
-        Assert::that($this->session()->getStatusCode())
+        Assert::that($this->session()->statusCode())
             ->is($expected, 'Current response status code is {actual}, but {expected} expected.')
         ;
 
@@ -235,10 +235,12 @@ public function assertStatus(int $expected): self
      */
     public function assertSuccessful(): self
     {
+        $statusCode = $this->session()->statusCode();
+
         Assert::true(
-            $this->session()->getStatusCode() >= 200 && $this->session()->getStatusCode() < 300,
+            $statusCode >= 200 && $statusCode < 300,
             'Expected successful status code (2xx) but got {actual}.',
-            ['actual' => $this->session()->getStatusCode()],
+            ['actual' => $statusCode],
         );
 
         return $this;

src/Browser/Session.php

@@ -68,6 +68,29 @@ public function page(): DocumentElement
         return $this->getPage();
     }
 
+    public function statusCode(): int
+    {
+        // not ensureNoException(): asserting the status of an error page is legitimate
+        $this->ensureResponseExists();
+
+        return $this->getStatusCode();
+    }
+
+    public function crawler(): Crawler
+    {
+        $this->ensureResponseExists();
+
+        return $this->client()->getCrawler();
+    }
+
+    public function content(): string
+    {
+        // reading an error page's body is legitimate: crawler() and saveSource() both allow it
+        $this->ensureResponseExists();
+
+        return $this->getPage()->getContent();
+    }
+
     public function isRedirect(): bool
     {
         return $this->getStatusCode() >= 300 && $this->getStatusCode() < 400;
@@ -168,12 +191,21 @@ public function exit(): void
         exit(1);
     }
 
-    private function ensureNoException(): void
+    private function ensureResponseExists(): void
     {
         if (!$this->isStarted()) {
             ZenstruckAssert::fail('A request has not yet been made.');
         }
 
+        if ($this->getDriver()->lastRequestThrewExpectedException()) {
+            ZenstruckAssert::fail('The last request threw the expected exception: make another request before continuing.');
+        }
+    }
+
+    private function ensureNoException(): void
+    {
+        $this->ensureResponseExists();
+
         // an exception page always carries an error status: this runs before every action and
         // assertion, so successful responses are never inspected
         if (!$this->couldBeExceptionPage()) {
@@ -210,6 +242,7 @@ private function ensureNoException(): void
     private function couldBeExceptionPage(): bool
     {
         try {
+            // not statusCode(), which calls back into here
             return $this->getStatusCode() >= 400;
         } catch (DriverException) {
             // the driver cannot tell us, so fall through to inspecting the response itself

src/Browser/Session/Driver.php

@@ -33,6 +33,7 @@ abstract class Driver extends CoreDriver
     private $expectedException;
     private ?string $expectedExceptionMessage = null;
     private bool $catchExceptionsEnabled = true;
+    private bool $lastRequestThrewExpectedException = false;
 
     /**
      * @param AbstractBrowser<Request, Response> $client
@@ -68,6 +69,8 @@ public function stop(): void
     public function reset(): void
     {
         $this->client()->restart();
+
+        $this->lastRequestThrewExpectedException = false;
     }
 
     public function quit(): void
@@ -100,13 +103,24 @@ public function catchExceptions(bool $catch): void
 
     abstract public function request(string $method, string $url, HttpOptions $options): void;
 
+    /**
+     * Whether the last request threw the exception expected by expectException(): its response
+     * never existed, so whatever the client holds belongs to an earlier request.
+     */
+    public function lastRequestThrewExpectedException(): bool
+    {
+        return $this->lastRequestThrewExpectedException;
+    }
+
     /**
      * Tell the client to stop, or resume, converting kernel exceptions into responses.
      */
     abstract protected function clientCatchExceptions(bool $catch): void;
 
     final protected function wrapRequest(callable $callback): void
     {
+        $this->lastRequestThrewExpectedException = false;
+
         if (!$this->expectedException) {
             $callback();
 
@@ -117,6 +131,8 @@ final protected function wrapRequest(callable $callback): void
 
         try {
             Assert::that($callback)->throws($this->expectedException, $this->expectedExceptionMessage);
+
+            $this->lastRequestThrewExpectedException = true;
         } finally {
             // a request the browser has not finished can be handled after this one returns, so
             // leaving catching disabled would throw the same exception again, out of a later call

src/Browser/Session/Driver/BrowserKitDriver.php

@@ -105,19 +105,19 @@ public function getCurrentUrl(): string
 
     public function reload(): void
     {
-        $this->client()->reload();
+        $this->wrapRequest(fn() => $this->client()->reload());
         $this->forms = [];
     }
 
     public function forward(): void
     {
-        $this->client()->forward();
+        $this->wrapRequest(fn() => $this->client()->forward());
         $this->forms = [];
     }
 
     public function back(): void
     {
-        $this->client()->back();
+        $this->wrapRequest(fn() => $this->client()->back());
         $this->forms = [];
     }
 

tests/BrowserTests.php

@@ -193,6 +193,70 @@ public function can_expect_exception_for_link_click(): void
         ;
     }
 
+    /**
+     * @test
+     */
+    #[Test]
+    public function fails_if_acting_after_an_expected_exception(): void
+    {
+        // the throwing request never produced a response, the previous page is still loaded
+        Assert::that(function() {
+            $this->browser()
+                ->visit('/page1')
+                ->expectException(\Exception::class, 'exception thrown')
+                ->visit('/exception')
+                ->assertSee('h1 title')
+            ;
+        })->throws(AssertionFailedError::class, 'The last request threw the expected exception: make another request before continuing.');
+
+        // without a previous request there is no page at all
+        Assert::that(function() {
+            $this->browser()
+                ->expectException(\Exception::class, 'exception thrown')
+                ->visit('/exception')
+                ->click('a link')
+            ;
+        })->throws(AssertionFailedError::class, 'The last request threw the expected exception: make another request before continuing.');
+
+        // a new request makes the browser usable again
+        $this->browser()
+            ->expectException(\Exception::class, 'exception thrown')
+            ->visit('/exception')
+            ->visit('/page1')
+            ->assertSee('h1 title')
+        ;
+    }
+
+    /**
+     * @test
+     *
+     * @dataProvider responseAccessorProvider
+     */
+    #[Test]
+    #[DataProvider('responseAccessorProvider')]
+    public function fails_if_reading_the_response_when_the_request_threw(callable $accessor): void
+    {
+        Assert::that(function() use ($accessor) {
+            $browser = $this->browser()
+                ->visit('/page1')
+                ->expectException(\Exception::class, 'exception thrown')
+                ->visit('/exception')
+            ;
+
+            $accessor($browser);
+        })->throws(AssertionFailedError::class, 'The last request threw the expected exception: make another request before continuing.');
+    }
+
+    /**
+     * These read the response without going through page(), so they need the check of their own.
+     */
+    public static function responseAccessorProvider(): iterable
+    {
+        yield 'assertStatus' => [fn(Browser $browser) => $browser->assertStatus(200)];
+        yield 'assertSuccessful' => [fn(Browser $browser) => $browser->assertSuccessful()];
+        yield 'crawler' => [fn(Browser $browser) => $browser->crawler()];
+    }
+
     /**
      * @test
      */
@@ -1240,6 +1304,17 @@ public function can_get_content(): void
         $this->assertStringContainsString('text content', $content);
     }
 
+    /**
+     * @test
+     */
+    #[Test]
+    public function can_get_content_of_an_exception_page(): void
+    {
+        $content = $this->browser()->visit('/exception')->content();
+
+        $this->assertStringContainsString('exception thrown', $content);
+    }
+
     protected static function catchFileContents(string $expectedFile, callable $callback): string
     {
         (new Filesystem())->remove($expectedFile);