sourcelevel / faraday-http-cache

A Faraday middleware that respects HTTP cache
Other
343 stars 86 forks source link

Fix exception namespacing conflict with Faraday::Encoding #92

Closed timcheadle closed 7 years ago

timcheadle commented 7 years ago

It turns out that if you have https://github.com/ma2gedev/faraday-encoding loaded alongside faraday-http-cache, there is a namespace conflict in Faraday::HttpCache::Storage#write. In short, this happens:

NameError: uninitialized constant Faraday::Encoding::UndefinedConversionError
from /app/vendor/bundle/ruby/2.3.0/gems/faraday-http-cache-2.0.0/lib/faraday/http_cache/storage.rb:59:in `rescue in write'

Because Encoding::UndefinedConversionError is not scoped globally, Ruby thinks it should exist in Faraday::Encoding. Not only is this not the desired behavior, but it has the really nasty effect of hiding other exceptions that occur in that method, as the NameError happens when it tries to execute the rescue block.

Luckily it's a simple fix; just namespace the exception absolutely. 😄

sourcelevel-bot[bot] commented 7 years ago

Hello, @timcheadle! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

timcheadle commented 7 years ago

PS - I can't actually get the specs to run. I'm not sure what I'm missing.

» bundle
Resolving dependencies...
Using rake 12.0.0
Using concurrent-ruby 1.0.4
Using i18n 0.8.0
Using minitest 5.10.1
Using thread_safe 0.3.5
Using public_suffix 2.0.5
Using cookiejar 0.3.3
Using diff-lcs 1.3
Using eventmachine 1.2.2
Using http_parser.rb 0.6.0
Using multipart-post 2.0.0
Using mustermann 1.0.0.beta2
Using rack 2.0.1
Using rspec-support 3.5.0
Using tilt 2.0.6
Using bundler 1.13.6
Using tzinfo 1.2.2
Using addressable 2.5.0
Using em-socksify 0.3.1
Using faraday 0.11.0
Using rack-protection 2.0.0.beta2
Using rspec-core 3.5.4
Using rspec-expectations 3.5.0
Using rspec-mocks 3.5.0
Using activesupport 5.0.1
Using em-http-request 1.1.5
Using faraday-http-cache 2.0.0 from source at `.`
Using faraday_middleware 0.11.0.1
Using sinatra 2.0.0.beta2
Using rspec 3.5.0
Bundle complete! 7 Gemfile dependencies, 30 gems now installed.
Use `bundle show [gemname]` to see where a bundled gem is installed.

» bundle exec rake spec
/Users/tim/.rbenv/versions/2.3.3/bin/ruby -I/Users/tim/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.4/lib:/Users/tim/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/rspec-support-3.5.0/lib /Users/tim/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.4/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb
test server did not managed to start
/Users/tim/.rbenv/versions/2.3.3/bin/ruby -I/Users/tim/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.4/lib:/Users/tim/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/rspec-support-3.5.0/lib /Users/tim/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.4/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb failed
lucasmazza commented 7 years ago

w00t, totally missed this one. Thanks for the patch @timcheadle!