Repository navigation
Return the main resource response from goto and reload - #35
Conversation
Page.goto and Frame.goto dropped the response that Playwright resolves with, so an error status such as 404 could only be read through use. Return it as Option<Response>, like goBack and goForward.
commit: |
fiws
left a comment
There was a problem hiding this comment.
thanks, looks good in general.
personally i would not mark this as a breaking change, i don't see many cases where it would be - but also fine if we wanna keep it as breaking.
i also think it totally makes sense to change Page.reload the same way.
They all pass except two tests in src/playwright.test.ts (launchPersistentContext and connectCDP), which fail in my environment on main as well.
can you post the output of the failures here? curious what could be the problem.
Code that ignores the result keeps working, so the change is not breaking. Drop the comparison with Playwright from the goto docs, since the whole library mirrors Playwright.
Page.reload dropped the response that Playwright resolves with and took no options. Make it a method that takes the reload options and returns Option<Response>, like goto, goBack and goForward.
|
Yeah sorry for the clanker slop comments, should be good now and I added |
|
thanks for the contribution |
Problem
Page.goto,Frame.gotoandPage.reloadare typedEffect<void, PlaywrightError>and drop theResponse | nullthat Playwright resolves with. Playwright doesn't reject when the server answers with an error status such as 404 or 500, so the response is the only way to tell whether you got the page you asked for or an error page. Today that takes the escape hatch:We hit this capturing screenshots of pages from a CMS. Without the status check, a 404 page gets captured as if it were the page.
Page.reloadalso takes no options, unlikePage.goBackandPage.goForward.Change
Page.gotoandFrame.gotonow returnEffect<Option<Playwright.Response>, PlaywrightError>, mapped the same way asPage.goBackandPage.goForwardand as theimplement-playwright-methodskill describes for an asyncT | nullresult.Nonecovers the navigations Playwright resolves withnull, such asabout:blankor a change of the URL hash.Page.reloadis now a method that takes Playwright's reload options and returns the response in the same way.The TSDoc says that an error status doesn't fail the navigation, and
Page.gotohas an example that reads the status.Changelog
The
Unreleasedsection lists the response fromgotoandreloadunder Features, since code that ignores the result keeps working and code that needsEffect<void, PlaywrightError>can pipeEffect.asVoid.Turning
Page.reloadfrom anEffectproperty into a method is breaking, so it is listed under Breaking Changes. Callers replaceyield* page.reloadwithyield* page.reload(), like theconsoleMessagesandpageErrorschange in 0.5.0.Tests
Page.gotoreturns the response of a route fulfilled with status 404.Page.gotoreturnsNoneforabout:blank.Frame.gotoreturns the response throughpage.mainFrame().Page.reloadreturns the response of the reload rather than of the first load.Page.reloadfails with aTimeouterror when the reload exceeds itstimeoutoption.The new tests fail when
gotoandreloadgo back to the old implementation, and the timeout test fails when the options are not passed on.pnpm biome ci .,pnpm type-check,pnpm check-doc-examplesandpnpm test:playwrightpass. I ran the vitest files one at a time with Chromium 154. They all pass except two tests insrc/playwright.test.ts,should connect via CDP (confirm browser.close only closes CDP connection)andshould fail with timeout 1, which fail in my environment onmainas well.Note
Medium Risk
Breaking TypeScript API on a core navigation path; runtime behavior is unchanged when the return value is ignored, but typed consumers must update.
Overview
Breaking change:
Page.goto()andFrame.goto()now returnEffect<Option<Response>, PlaywrightError>instead ofEffect<void, …>, matchinggoBack/goForwardand Playwright’sResponse | null. The wrapper mapsnulltoNoneand wraps responses withmakeResponse; navigation still succeeds on HTTP error statuses (404/500), so callers can inspectresponse.status()without usingpage.use.Docs and CHANGELOG (Unreleased) describe the migration (
Effect.asVoidif you still want void). New tests cover 404 responses,about:blank→None, and framegoto.Reviewed by Cursor Bugbot for commit 41c3dc2. Bugbot is set up for automated code reviews on this repo. Configure here.