Skip to content

Add support for browser.on(:response). #294

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

postmodern
Copy link
Contributor

I wanted to be able to have a callback similar to on(:request) but triggered once a response was received. This adds an additional callback for the Network.responseReceived event when a response is received. It will look up the Ferrum::Network::Exchange object with the matching requestId and passes it back to the block, once a response has been received for a request.

I didn't know where to add specs for this, since there does not appear to be specs for Ferrum::Page or a spec/page_spec.rb file? I also didn't know where I should mention browser.on(:response) in the README, since browser.on(...) is only mentioned in conjunction to using browser.network.intercept.

@postmodern
Copy link
Contributor Author

@route would appreciate some guidance on this new feature and where I should add the specs and documentation?

@route
Copy link
Member

route commented Oct 5, 2022

@postmodern I was reorganizing specs so currently page specs are in the browser_spec. I think we can create page_spec and put it there. Though it overlaps with network_spec a bit, because page.on(:request) only works with interception, but page.on(:response) works without it. I'm now thinking that maybe it's better to use something like page.network.on(:request | :response | :auth). Need to experiment with that...

@ttilberg
Copy link
Contributor

ttilberg commented Feb 7, 2023

I can't say for certain, but I believe the act of enabling network intercept requires you to authorize the requests to proceed. For example, some of the specs show:

https://github.com/rubycdp/ferrum/blob/main/spec/network/exchange_spec.rb#L28

network.intercept
page.on(:request) { |r, _, _| r.continue }

@postmodern
Copy link
Contributor Author

postmodern commented Feb 7, 2023

@ttilberg hmm that solved the exception, except now it does not appear that my on(:response) callback is being called back.

@ttilberg
Copy link
Contributor

ttilberg commented Feb 7, 2023

@postmodern I took a look but didn’t find a CDP event translation helper for on(:response). However, in an earlier issue I noted and used network.responseReceived, might this get you on your way?

#197 (comment)

Edit: haha… I just looked at the rest of the context of this issue… lol. I’ll see myself out!

@postmodern
Copy link
Contributor Author

@ttilberg yeah it's added in this PR. Feel free to pull my branch and try to mess with it. I swear it was working last year.

@postmodern
Copy link
Contributor Author

OK I added some echo debugging and the @client.on('Network.responseReceived') callback is being called, but it's not finding the exchange object for the given params['requestId'], so the passed in block is never called.

* This adds an additional callback for the `Network.responseReceived`
  event when a response is received. It will look up the
  `Ferrum::Network::Exchange` object with the matching `requestId` and passes
  it back to the block, once a response has been received for a request.
@postmodern
Copy link
Contributor Author

Got it, I needed to do network.select from the context of Ferrum::Page.

@ilyacherevkov
Copy link

This is useful, I use it as a monkey patch while it's not merged.

@ilyacherevkov
Copy link

ilyacherevkov commented Feb 4, 2025

Turned out it doesn't work for really heavy and chunked responses (I m testing particular response which returns 20MB), so I needed to use Network.requestWillBeSent to save mapping table of "requestId" and 'url', Network.loadingFinished to ensure full response got loaded.

Then I used mapping table to match requestId from loadingFinished with url from requestWillBeSen, and thus being 100% sure request got fully loaded.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants