ruckus / quickbooks-ruby

Quickbooks Online REST API V3 - Ruby
MIT License
374 stars 302 forks source link

oauth2 1.4.8 update causes NoMethodError: undefined method `dependency' for Gzip:Class #570

Closed rickbarrette closed 2 years ago

rickbarrette commented 2 years ago

I ran into a weird issue after trying to restart my official Redmine 4.2.3 docker container which is using ruby 2.7.5p203 (2021-11-24 revision f69aeb8314) [x86_64-linux].

I have been running a plugin I wrote that uses the quickbooks-ruby gem found here for years without issues, however now I'm getting the following error:

 NoMethodError: undefined method `dependency' for Gzip:Class
/usr/local/bundle/gems/quickbooks-ruby-1.0.19/lib/quickbooks/faraday/middleware/gzip.rb:15:in `<class:Gzip>'
/usr/local/bundle/gems/quickbooks-ruby-1.0.19/lib/quickbooks/faraday/middleware/gzip.rb:14:in `<top (required)>'
/usr/local/bundle/gems/quickbooks-ruby-1.0.19/lib/quickbooks-ruby.rb:16:in `require'
/usr/local/bundle/gems/quickbooks-ruby-1.0.19/lib/quickbooks-ruby.rb:16:in `<top (required)>'
/usr/src/redmine/config/application.rb:18:in `<top (required)>'
/usr/src/redmine/Rakefile:5:in `<top (required)>'
/usr/local/bundle/gems/rake-13.0.6/exe/rake:27:in `<top (required)>'
(See full trace by running task with --trace)

Here is the first 15 lines from /lib/quickbooks/faraday/middleware/gzip.rb

# https://github.com/lostisland/faraday_middleware/blob/master/lib/faraday_middleware/gzip.rb

require 'faraday'

# Middleware to automatically decompress response bodies. If the
# "Accept-Encoding" header wasn't set in the request, this sets it to
# "gzip,deflate" and appropriately handles the compressed response from the
# server. This resembles what Ruby 1.9+ does internally in Net::HTTP#get.
#
# This middleware is NOT necessary when these adapters are used:
# - net_http on Ruby 1.9+
# - net_http_persistent on Ruby 2.0+
# - em_http
class Gzip < Faraday::Middleware
  dependency 'zlib' 

Turns out that when oauth2 is updated to 1.4.8 it breaks quickbooks-ruby.

confirmed that 1.4.7 works as designed and added the following to my Gemfile

gem 'oauth2', '1.4.7'
Zajn commented 2 years ago

Faraday removed the dependency method in v2: https://github.com/lostisland/faraday/pull/1398

The oauth2 gem seems to have a fairly loose dependency requirement for faraday:

  # In version 1.4.8 of oauth2
  spec.add_dependency 'faraday', ['>= 0.8', '< 3.0']

It seems like the immediate solution is to add a require 'zlib' and remove the dependency 'zlib', based on the issue above.

I'm not sure what else the upgrade to faraday v2 entails. Would it be worth adding an explicit dependency on Faraday that is < 2.0, since quickbooks-ruby does explicitly use a Faraday::Middleware?

ruckus commented 2 years ago

Thanks @rickbarrette and @Zajn for surfacing this.

I'm not sure what else the upgrade to faraday v2 entails.

Indeed, I have no clue either.

Would it be worth adding an explicit dependency on Faraday that is < 2.0, since quickbooks-ruby does explicitly use a Faraday::Middleware?

I think this is the safest solution for now. @rickbarrette are you able to try this and let us know? I think mechanically you would need to fork the repo, adjust the quickbooks-ruby.gemspec to add that versioned dependency then point your applications's Gemfile to your forked repo as a test.

Thanks everyone!

Zajn commented 2 years ago

Hey @ruckus, I added a PR to add an explicit dependency on < v2.0 of Faraday, which at least temporarily addresses this issue.

The upgrade to using v2.0 of Faraday doesn't seem terribly difficult, but it's a fair amount of work.

ruckus commented 2 years ago

Thanks @Zajn for the PR, I just merged it. I don't have the bandwidth right now to look into a Faraday 2+ migration/upgrade. So we'll just have to put a pin in this for now. Thanks again.

Zajn commented 2 years ago

Sure thing! I might have some free time in the near future to take a look at it.

ruckus commented 2 years ago

Maybe its just simpler to remove the gzip support? This might make the faraday upgrade simpler. thoughts?