Skip to content

Raise Ferrum::StatusError for any top frame navigation error#341

Merged
route merged 1 commit intorubycdp:mainfrom
francisbeaudoin:statuserror-navigation
Feb 16, 2023
Merged

Raise Ferrum::StatusError for any top frame navigation error#341
route merged 1 commit intorubycdp:mainfrom
francisbeaudoin:statuserror-navigation

Conversation

@francisbeaudoin
Copy link
Contributor

@francisbeaudoin francisbeaudoin commented Feb 15, 2023

Context

The Page#go_to is only raising a Ferrum::StatusError for a subset of exceptions:

ferrum/lib/ferrum/page.rb

Lines 113 to 124 in 0cab51d

def go_to(url = nil)
options = { url: combine_url!(url) }
options.merge!(referrer: referrer) if referrer
response = command("Page.navigate", wait: GOTO_WAIT, **options)
# https://cs.chromium.org/chromium/src/net/base/net_error_list.h
if %w[net::ERR_NAME_NOT_RESOLVED
net::ERR_NAME_RESOLUTION_FAILED
net::ERR_INTERNET_DISCONNECTED
net::ERR_CONNECTION_TIMED_OUT
net::ERR_FILE_NOT_FOUND].include?(response["errorText"])
raise StatusError, options[:url]
end

Per the Chrome DevTools documentation an errorText is present if and only if navigation has failed.

Proposal

This PR is proposing to raise regardless of the errorText given that the navigation failed.

Warning
This could be considered as a breaking change.

Reference

Discussion: #340

@route route self-requested a review February 16, 2023 07:48
@route route merged commit a6708f9 into rubycdp:main Feb 16, 2023
@route
Copy link
Member

route commented Feb 16, 2023

Thank you! Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants