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 Ferrum::Network::Response#loaded? for redirect response #328

Closed
wants to merge 1 commit into from
Closed

Fix Ferrum::Network::Response#loaded? for redirect response #328

wants to merge 1 commit into from

Conversation

francisbeaudoin
Copy link
Contributor

@francisbeaudoin francisbeaudoin commented Jan 19, 2023

Issue

Fixes #321

Context

Redirect Ferrum::Network::Response are created within the Network.requestWillBeSent event:

# On redirects Chrome doesn't change `requestId` and there's no
# `Network.responseReceived` event for such request. If there's already
# exchange object with this id then we got redirected and params has
# `redirectResponse` key which contains the response.
if params["redirectResponse"]
previous_exchange = select(request.id)[-2]
response = Network::Response.new(@page, params)
previous_exchange.response = response
end

Additionally, the Network.loadingFinished event is where responses are marked as loaded = true:

@page.on("Network.loadingFinished") do |params|
response = select(params["requestId"]).last&.response
if response
response.loaded = true
response.body_size = params["encodedDataLength"]
end

As redirect responses are sharing the same requestId and as that code is only selecting the last exchange response e.g. the final response, only that exchange response is marked as loaded?.

Proposed fix

Assigning loaded = true at the time that redirect response object is created.

Note: Unfortunately I'm running into issues executing tests locally so I didn't add any. Per my quick testing, it looks to be solving the aforementioned issue.

@francisbeaudoin francisbeaudoin changed the title Fix Ferrum::Network::Response#loaded? for redirect response Fix Ferrum::Network::Response#loaded? for redirect responses Jan 19, 2023
@francisbeaudoin francisbeaudoin changed the title Fix Ferrum::Network::Response#loaded? for redirect responses Fix Ferrum::Network::Response#loaded? for redirect response Jan 19, 2023
@francisbeaudoin francisbeaudoin closed this by deleting the head repository Jan 31, 2023
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.

Ferrum::Network::Response#loaded? fails on 302 redirects after #309
1 participant