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 ethon typhoeus exception handling #68

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

blaze182
Copy link

@blaze182 blaze182 commented Jul 13, 2022

Hi everyone and thanks for the fabulous gem.

What is the purpose of this pull request?

This PR improves integration with Ethon and Typhoeus libraries. Sniffer is currently interfering with the behaviour of these libs on network errors, and is not parsing urls prepended with http(s):// correctly.

What changes did you make? (overview)

NoMethodError

Sniffer is currently crashing with NoMethodError: undefined method 'each_with_object' for nil:NilClass if Ethon/Typhoeus request fails with any kind of network errors (host not found, timeout, etc.). However, the original behaviour of those libraries is to not raise any exceptions on .perform / .run respectively.

The reason is @response_headers is empty and is returning nil after split:

hash_headers = @response_headers
.split(/\r?\n/)[1..-1]
.each_with_object({}) do |item, res|
k, v = item.split(": ")
res[k] = v
end

In order to prevent interfering with original libraries' behaviour, I propose another shared spec example which runs unresolved_request twice (hence the method instead of let) with and without Sniffer, and compares the error outcomes.

URL parsing

URI("http://#{url}") should become URI(url.start_with?(%r{https?://}) ? url : "http://#{url}"), otherwise we have a shift in Sniffer's host and query (i.e. host equals "https", query is "//localhost:4567/?author=Matz"). Ethon and Typhoeus support both formats of the urls.

Checklist

  • I've added tests for this change
  • I've added a Changelog entry
  • I've updated a documentation

blaze182 added 3 commits July 13, 2022 18:50
`NoMethodError: undefined method `each_with_object' for nil:NilClass`
Ethon and Typhoeus can accept both raw host and protocol+host. Sniffer was parsing host and query incorrectly in the second case.
@composerinteralia
Copy link

FWIW I just ran into this while trying to use sniffer (via https://github.com/palkan/isolator/) at GitHub.

@Aubermean
Copy link

@aderyabin

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.

3 participants