Skip to content

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

Merged
kbond merged 4 commits into
zenstruck:1.xfrom
Amoifr:fix-202-stale-state-after-expected-exception
Aug 29, 2026
Merged

kbond merged 4 commits into
zenstruck:1.xfrom
Amoifr:fix-202-stale-state-after-expected-exception

Conversation

@Amoifr

@Amoifr Amoifr commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #202

Implements the proposal from the issue, in the shared layer so both browsers get the same behavior:

  • Driver::wrapRequest() records that the expected exception was consumed (the flag is cleared by the next request and by reset()).
  • Session::ensureNoException() already runs before every action and assertion, so it now fails fast with "The last request threw the expected exception: make another request before continuing." instead of leaking BrowserKit's BadMethodCallException or silently acting on the previous page.

The shared test covers the three cases from the issue: stale previous page, no previous request at all, and recovery once another request is made.

Thanks for writing the issue up with the proposal included, it basically reviewed itself. 😄

Amoifr and others added 4 commits August 29, 2026 10:45
After expectException() and a request that throws, the throwing request
never produced a response, so the client still holds the state of an
earlier request (or none at all). Actions and assertions then either
leak a raw BadMethodCallException from BrowserKit or silently run
against the previous page.

wrapRequest() now records that the expected exception was consumed and
ensureNoException() fails the next action with an explicit message
until another request is made.

Fixes zenstruck#202
… and crawler()

These read the response without going through `page()`, so they skipped the check entirely and asserted against the previous request's page.

`ensureNoException()` is split so they get the stale-response check without the exception-page detection: asserting the status of an error page is legitimate.
They navigate without going through `wrapRequest()`, so the flag survived and left the browser failing on a page that is perfectly valid. Only reachable through the Mink driver today.
`content()` went through `page()`, so it was the only route to an error page's body that failed: `crawler()->html()`, `client()->getResponse()`, `saveSource()` and `dump()` all already allowed it.
@kbond
kbond force-pushed the fix-202-stale-state-after-expected-exception branch from 97a4b3d to fa9c6ec Compare August 29, 2026 14:46

@kbond kbond left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I pushed a few commits on top: assertStatus(), assertSuccessful() and crawler() were skipping the check entirely, back()/forward()/reload() never cleared the flag, and I loosened content() so an error page body can still be read.

@kbond
kbond merged commit d0f90bf into zenstruck:1.x Aug 29, 2026
16 checks passed
kbond added a commit that referenced this pull request Oct 10, 2026
* 1.x:
  test: unique artifact filenames to fix a race under paratest (#209)
  bot: fix cs [skip ci]
  fix: fail when acting on the browser after an expected exception (#207)
  fix: don't mutate the default http options when merging request options (#210)
  changelog: update [skip ci]
  feat(playwright): use the playwright bundle configuration when registered (#206)
  test: support phpunit 11, 12 and 13, and run the suite with paratest (#205)
  chore: drop EOL Symfony 8.0 from the ci matrix
  test: enable and cover the browser extension
  chore: remove stray `attachment.zip` from the project root
  test: run the suite with paratest
  test: support phpunit 11, 12 and 13
  fix: Mink deprecation (#203)
  changelog: update [skip ci]
  docs: document the shared redirect and profiling api
  fix: honor `BROWSER_FOLLOW_REDIRECTS` in the `PlaywrightBrowser`
  feat: Add `PlaywrightBrowser` (#195)
  fix: phpstan doc comment (#178)
  feat: make saved artifact paths clickable (#165)
  fix: detect Symfony 7.4+ exception pages (#197)
  doc: Standardize `README` security policy section (#193)
  chore!: bump minimum PHP version to 8.2 (#196)
  changelog: update [skip ci]
  changelog: update [skip ci]
  fix: mink deprecation
  bot: sync with template [skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Acting on the browser after expectException() works on stale state

2 participants