fastruby / harvesting

Ruby wrapper for the Harvest API v2
MIT License
28 stars 30 forks source link

Add require forwardable in havest_record_collection model #40

Closed sarcasticadmin closed 5 years ago

sarcasticadmin commented 5 years ago

Description

Im getting an error NameError (uninitialized constant Harvesting::Models::HarvestRecordCollection::Forwardable) when I require 'harvesting' after installing the gem in both 2.4.1 and 2.5.1. I have to require 'forwardable' first and then my error goes away. This seems like a side effect based on order of require and seems to be masked in the build system/test suite since all the tests are passing. Im not sure the best approach is to adding a require side effect check to Travis.

Detail are below, let me know what you think.

Checking this off master@54778805333dd22e4d44752ea1341aab091d6ade and running the latest version of the docker container defined in this repo (ruby 2.4.1):

$ docker run --rm -it harvesting /bin/sh
# bash
root@7f8112265798:/gem# rbenv
bash: rbenv: command not found
root@7f8112265798:/gem# gem install harvesting
Fetching: public_suffix-3.1.1.gem (100%)
Successfully installed public_suffix-3.1.1
Fetching: addressable-2.6.0.gem (100%)
Successfully installed addressable-2.6.0
Fetching: unf_ext-0.0.7.6.gem (100%)
Building native extensions.  This could take a while...
Successfully installed unf_ext-0.0.7.6
Fetching: unf-0.1.4.gem (100%)
Successfully installed unf-0.1.4
Fetching: domain_name-0.5.20180417.gem (100%)
Successfully installed domain_name-0.5.20180417
Fetching: http-cookie-1.0.3.gem (100%)
Successfully installed http-cookie-1.0.3
Fetching: http-form_data-2.1.1.gem (100%)
Successfully installed http-form_data-2.1.1
Fetching: http_parser.rb-0.6.0.gem (100%)
Building native extensions.  This could take a while...
Successfully installed http_parser.rb-0.6.0
Fetching: http-3.3.0.gem (100%)
Successfully installed http-3.3.0
Fetching: harvesting-0.3.0.gem (100%)
Successfully installed harvesting-0.3.0
10 gems installed
root@7f8112265798:/gem# irb
irb(main):001:0> require 'harvesting'
NameError: uninitialized constant Harvesting::Models::HarvestRecordCollection::Forwardable
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:5:in `<class:HarvestRecordCollection>'                                              
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:3:in `<module:Models>'                                                              
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:2:in `<module:Harvesting>'                                                          
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:1:in `<top (required)>'                                                             
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'                                                                                        
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'                                                                                        
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting.rb:7:in `<top (required)>'                                                                                              
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:133:in `require'                                                                                       
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:133:in `rescue in require'                                                                             
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:40:in `require'                                                                                        
        from (irb):1
        from /usr/local/bin/irb:11:in `<main>'

Ruby 2.5.1:

$ irb
irb(main):001:0> require 'harvesting'
Traceback (most recent call last):
       12: from /Users/robhernandez/.rbenv/versions/2.5.1/bin/irb:11:in `<main>'
       11: from (irb):1
       10: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:39:in `require'                                                 
        9: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:135:in `rescue in require'                                      
        8: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:135:in `require'                                                
        7: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/gems/harvesting-0.3.0/lib/harvesting.rb:7:in `<top (required)>'                                
        6: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:70:in `require'                                                 
        5: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:70:in `require'                                                 
        4: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:1:in `<top (required)>'
        3: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:2:in `<module:Harvesting>'
        2: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:3:in `<module:Models>'
        1: from /Users/robhernandez/dotfiles/rbenv/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:5:in `<class:HarvestRecordCollection>'
NameError (uninitialized constant Harvesting::Models::HarvestRecordCollection::Forwardable)
irb(main):002:0> require 'forwardable'
=> true
irb(main):003:0> require 'harvesting'
=> true

If then I specify require forwardable before require harvesting it works (ruby 2.4.1):

root@7f8112265798:/gem# export HARVEST_ACCOUNT_ID='1111111'
root@7f8112265798:/gem# export HARVEST_ACCESS_TOKEN='mytoken' 
root@7f8112265798:/gem# irb
irb(main):001:0> require 'forwardable'
=> true
irb(main):002:0> require 'harvesting'
=> true
irb(main):003:0> client = Harvesting::Client.new
irb(main):004:0> client.me
=> #<Harvesting::Models::User:0x0000000111111100....

Briefly investigating the travis tests passing regardless of the above error, it seems like the problem could be due to the rspec_helper.rb loading all bundler dependencies before harvesting: https://github.com/ombulabs/harvesting/blob/master/spec/spec_helper.rb#L1 which would load require 'http' amongst others defined in the harvesting.gemspec. Im able to repeat this behavior running:

$ irb
irb(main):001:0> require 'harvesting'
NameError: uninitialized constant Harvesting::Models::HarvestRecordCollection::Forwardable
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:5:in `<class:HarvestRecordCollection>'                                              
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:3:in `<module:Models>'                                                              
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:2:in `<module:Harvesting>'                                                          
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting/models/harvest_record_collection.rb:1:in `<top (required)>'                                                             
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'                                                                                        
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'                                                                                        
        from /usr/local/bundle/gems/harvesting-0.3.0/lib/harvesting.rb:7:in `<top (required)>'
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:133:in `require'                                                                                       
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:133:in `rescue in require'                                                                             
        from /usr/local/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:40:in `require'                                                                                        
        from (irb):1
        from /usr/local/bin/irb:11:in `<main>'
irb(main):002:0> require 'http'
=> true
irb(main):003:0> require 'harvesting'
=> true

Fix

Leveraging the changes in this PR I no longer get the NameError (uninitialized constant Harvesting::Models::HarvestRecordCollection::Forwardable) error against 2.4.1 or 2.5.1. Verified via the docker container from this repo (ruby 2.4.1):

$ docker run --rm -v $(pwd):/tmp/harvesting -it harvesting /bin/bash
root@124e1ba08767:/gem# cd /tmp/harvesting/
root@124e1ba08767:/tmp/harvesting# ./bin/setup
root@124e1ba08767:/tmp/harvesting# bundle exec rake install                                                                                                                                 
harvesting 0.3.0 built to pkg/harvesting-0.3.0.gem.                                                                                                                                         
harvesting (0.3.0) installed.                                                                                                                                                               
root@124e1ba08767:/tmp/harvesting# export HARVEST_ACCOUNT_ID='111111'
root@124e1ba08767:/tmp/harvesting# export HARVEST_ACCESS_TOKEN='mytoken'                          
root@124e1ba08767:/tmp/harvesting# irb
irb(main):001:0> require 'harvesting'
=> true
irb(main):002:0> client = Harvesting::Client.new
=> #<Harvesting::Client:0x00000001111112 @access_token="mytoken", @account_id="111111">
irb(main):003:0> client.me
=> #<Harvesting::Models::User:0x000000011111100001....

Version Info

2.4.1

$ ruby -v
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
$ gem list | grep harvesting
harvesting (0.3.0)

2.5.1

$ gem list | grep harvesting
harvesting (0.3.0)
$ ruby -v
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-darwin17]
bronzdoc commented 5 years ago

@sarcasticadmin this is definitely an issue I was able to reproduce it.

thanks for the PR! and sorry for taking that long to getting back at you!