Skip to content
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

fix: resolve navigation flakiness #260

Merged
merged 1 commit into from Sep 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/puppeteer/frame_manager.rb
Expand Up @@ -144,13 +144,13 @@ def navigate_frame(frame, url, referer: nil, timeout: nil, wait_until: nil)
watcher.same_document_navigation_promise
end,
)

watcher.navigation_response
rescue Puppeteer::TimeoutError => err
raise NavigationError.new(err)
ensure
watcher.dispose
end

watcher.navigation_response
end

# @param timeout [number|nil]
Expand All @@ -168,13 +168,13 @@ def wait_for_frame_navigation(frame, timeout: nil, wait_until: nil)
watcher.same_document_navigation_promise,
watcher.new_document_navigation_promise,
)

watcher.navigation_response
rescue Puppeteer::TimeoutError => err
raise NavigationError.new(err)
ensure
watcher.dispose
end

watcher.navigation_response
end

# @param event [Hash]
Expand Down
25 changes: 22 additions & 3 deletions lib/puppeteer/lifecycle_watcher.rb
Expand Up @@ -81,7 +81,10 @@ def initialize(frame_manager, frame, wait_until, timeout)
@frame_manager.add_event_listener(FrameManagerEmittedEvents::FrameSwapped, &method(:handle_frame_swapped)),
@frame_manager.add_event_listener(FrameManagerEmittedEvents::FrameDetached, &method(:handle_frame_detached)),
]
@listener_ids['network_manager'] = @frame_manager.network_manager.add_event_listener(NetworkManagerEmittedEvents::Request, &method(:handle_request))
@listener_ids['network_manager'] = [
@frame_manager.network_manager.add_event_listener(NetworkManagerEmittedEvents::Request, &method(:handle_request)),
@frame_manager.network_manager.add_event_listener(NetworkManagerEmittedEvents::Response, &method(:handle_response)),
]

@same_document_navigation_promise = resolvable_future
@lifecycle_promise = resolvable_future
Expand All @@ -90,10 +93,24 @@ def initialize(frame_manager, frame, wait_until, timeout)
check_lifecycle_complete
end

class AnotherRequestReceivedError < StandardError ; end

# @param [Puppeteer::HTTPRequest] request
def handle_request(request)
return if request.frame != @frame || !request.navigation_request?
@navigation_request = request
@navigation_response_received&.reject(AnotherRequestReceivedError.new('New navigation request was received'))
@navigation_response_received = resolvable_future
if request.response && !@navigation_response_received.resolved?
@navigation_response_received.fulfill(nil)
end
end

# @param [Puppeteer::HTTPResponse] response
def handle_response(response)
return if @navigation_request&.internal&.request_id != response.request.internal.request_id

@navigation_response_received.fulfill(nil) unless @navigation_response_received.resolved?
end

# @param frame [Puppeteer::Frame]
Expand All @@ -107,6 +124,8 @@ def handle_frame_detached(frame)

# @return [Puppeteer::HTTPResponse]
def navigation_response
# Continue with a possibly null response.
@navigation_response_received.value! rescue nil
if_present(@navigation_request) do |request|
request.response
end
Expand Down Expand Up @@ -175,8 +194,8 @@ def dispose
if_present(@listener_ids['frame_manager']) do |ids|
@frame_manager.remove_event_listener(*ids)
end
if_present(@listener_ids['network_manager']) do |id|
@frame_manager.network_manager.remove_event_listener(id)
if_present(@listener_ids['network_manager']) do |ids|
@frame_manager.network_manager.remove_event_listener(*ids)
end
end
end