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

Network#intercept silently discards unknown resource_types #313

Closed
ttilberg opened this issue Nov 24, 2022 · 2 comments
Closed

Network#intercept silently discards unknown resource_types #313

ttilberg opened this issue Nov 24, 2022 · 2 comments

Comments

@ttilberg
Copy link
Contributor

def intercept(pattern: "*", resource_type: nil)

If you try to create an intercept filter with an unknown resource type, the option is silently discarded:

browser.network.intercept resource_type: :Hotdogs
# => Okie dokie

Instead, it should raise an exception either by letting the CDP exception bubble up:

Unknown resource type in fetch filter: 'Hotdogs' (Ferrum::BrowserError)
	from /Users/ttilberg/.asdf/installs/ruby/3.1.2/lib/ruby/gems/3.1.0/gems/ferrum-0.12/lib/ferrum/browser/client.rb:49:in `command'
	from /Users/ttilberg/.asdf/installs/ruby/3.1.2/lib/ruby/gems/3.1.0/gems/ferrum-0.12/lib/ferrum/page.rb:177:in `command'
	from /Users/ttilberg/.asdf/installs/ruby/3.1.2/lib/ruby/gems/3.1.0/gems/ferrum-0.12/lib/ferrum/network.rb:98:in `intercept'
	from browse.rb:4:in `<main>'

Or, raise an exception that helps the user by tailoring the language to the Ferrum API:

/Users/ttilberg/.asdf/installs/ruby/3.1.2/lib/ruby/gems/3.1.0/gems/ferrum-0.12/lib/ferrum/network.rb:96:in `intercept': resource_type must be one of ["Document", "Stylesheet", "Image", "Media", "Font", "Script", "TextTrack", "XHR", "Fetch", "EventSource", "WebSocket", "Manifest", "SignedExchange", "Ping", "CSPViolationReport", "Other"] (RuntimeError)
	from browse.rb:4:in `<main>'

It looks like RESOURCE_TYPES is only used to guard this value, so perhaps by letting it bubble up you wouldn't have to maintain this list anyway.

This came up after trying to send WebSocket as a value, and finding that it wasn't accepted by CDP anyway. So maybe there's a higher-level thing that needs to change anyway.

@route route closed this as completed in 797558d Nov 25, 2022
@route
Copy link
Member

route commented Nov 25, 2022

Thanks for reporting, fixed!

@route
Copy link
Member

route commented Sep 11, 2023

I have reported to Chrome https://bugs.chromium.org/p/chromium/issues/detail?id=1465257#c3 but I think it's gonna take them forever to address this issue.

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

No branches or pull requests

2 participants