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

Exchange#finished returns true before response body is ready #197

Closed
ttilberg opened this issue Sep 28, 2021 · 6 comments · Fixed by #309
Closed

Exchange#finished returns true before response body is ready #197

ttilberg opened this issue Sep 28, 2021 · 6 comments · Fixed by #309

Comments

@ttilberg
Copy link
Contributor

ttilberg commented Sep 28, 2021

Hello, I'm trying to intercept the response body of an XHR request. After looking through the spec files, I saw that Ferrum keeps track of all of the network traffic, which is convenient for this case. (I tried using on('Network.responseReceived') hooks, but couldn't work it out.)

While waiting for the correct Exchange to be finished? I kept running into Ferrum::BrowserError: No data found for resource with given identifier. Example code:

    response = Timeout.timeout(15) do
      # Find ajax request for search results
      until xhr = browser.network.traffic.find { |exchange| exchange.request.url =~ /search\/bySize/ }
        print '.'
        sleep 0.2
      end
      # Wait for response to complete
      until xhr.finished?
        print 'x'
        sleep 0.2
      end
      xhr.response.body
    end

After some research, I've learned that instead you should wait for Network.loadingFinished before trying to get the response body.

I wanted to highlight a few notes in Ferrum's API as I wonder if Exchange#finished? should wait for that event, or if the Response object c/should be more clever. (Or both).

# network.rb #subscribe
# ...
      @page.on("Network.responseReceived") do |params|
        if exchange = select(params["requestId"]).last
          response = Network::Response.new(@page, params)
          exchange.response = response
        end
      end

      @page.on("Network.loadingFinished") do |params|
        exchange = select(params["requestId"]).last
        if exchange && exchange.response
          exchange.response.body_size = params["encodedDataLength"]
        end
      end
# exchange.rb
# ...
      def finished?
        blocked? || response || error
      end

Note how the exchange is finished? when it has a response object. But also notice that this response attribute is assigned in the Network.responseReceived hook, not the Network.loadingFinished hook. There is, however, a property that gets set at Network.loadingFinished, body_size, so I tried keying off that:

    response = Timeout.timeout(15) do
      # Find ajax request for search results
      until xhr = browser.network.traffic.find { |exchange| exchange.request.url =~ /search\/bySize/ }
        print '.'
        sleep 0.2
      end
      # Wait for response to complete
      until xhr&.response&.body_size
        print 'x'
        sleep 0.2
      end
      xhr.response.body
    end

With this change, this code no longer raises the noted exception, though the API is being used in a strange way.

  • I feel that Exchange#finished? should account for the response being fully loaded and prepared to query. Maybe this isn't sensible due to existing usages for #finished? and things like streamed content. So, worst case, maybe something new?
  • I wonder about an attribute Response#finished? or loaded? or ready?
    • I also wonder if the api for querying the response.body should wait for an attribute to signal that the response is finished loading, leveraging the standard Ferrum timeouts, similar to how other calls on the browser block until CDP has reported it's ready.
@route
Copy link
Member

route commented Oct 21, 2021

Great research! I'm open to all options. Everything you said sounds correct, we just need to take only one direction.

As for me I usually start to play with code and then after a few implementations see a solution, so I cannot tell right now what's better or best.

Are you going to work on a PR?

@ttilberg
Copy link
Contributor Author

If you don't know of a reason not to, I'll change the #finished? flag to represent when the response is ready, not just received. PR to come.

@route
Copy link
Member

route commented Oct 21, 2021

Ok! Sounds right

@hkmaly
Copy link

hkmaly commented Dec 27, 2021

Grrr ... I tried to use it on something else and got Ferrum::BrowserError: No resource with given identifier found ... AND when I tried to use inspect, I found the resource was loaded from cache, although it's not mentioned anywhere in that response object (all from*Cache are false).

@rmarot
Copy link
Contributor

rmarot commented Feb 11, 2022

@ttilberg I am also facing this issue, do you still plan to open a PR on your suggestions (which look good !) ? Thanks !

@ttilberg
Copy link
Contributor Author

I was having a hard time figuring out how to create the tests to set up the delayed payload, and I kind of got discouraged and tabled it. Please feel free!

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 a pull request may close this issue.

4 participants