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

fix: fail when acting on the browser after an expected exception#207
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
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