-
-
Notifications
You must be signed in to change notification settings - Fork 166
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
chore: roll to Playwright v1.40.1 #394
Conversation
files can be one of: string, []string, InputFile, []InputFile
artifact.go
Outdated
@@ -18,7 +18,7 @@ func (a *artifactImpl) PathAfterFinished() (string, error) { | |||
return "", errors.New("Path is not available when connecting remotely. Use SaveAs() to save a local copy") | |||
} | |||
path, err := a.channel.Send("pathAfterFinished") | |||
if path == nil { | |||
if path == nil { // < v1.40.0 |
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.
I personally would remove that, since we from the last person throw if you connect to a server which is not matching the protocol version you are using. The user use-case if that you are using the normal driver which we download for you where we ensure that the version always matches.
browser.go
Outdated
} | ||
// if b.isClosedOrClosing { |
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.
redundant code?
errors.go
Outdated
ErrBrowserClosed = errors.New("Browser has been closed") | ||
ErrBrowserOrContextClosed = errors.New("Target page, context or browser has been closed") |
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.
ErrBrowserClosed = errors.New("Browser has been closed") | |
ErrBrowserOrContextClosed = errors.New("Target page, context or browser has been closed") |
can't we remove these two? Ideally we only have ErrorPlaywright, ErrTimeout and ErrTargetClosed. (maybe use more Go like names, but something around these lines)
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 latest commit refactors the error type, bringing breaking changes:
ErrPlaywright
wraps all Playwright errorsErrTimeout
wraps all timeout errors and replacesTimeoutError
pls review it.
BREAKING CHANGE: - `ErrPlaywright` wraps all Playwright errors - `ErrTimeout` wraps all timeout errors and replaces `TimeoutError`
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.
Looks reasonable!
SetInputFiles
now supports both file paths and InputFile objects