twilio / twilio-ruby

A Ruby gem for communicating with the Twilio API and generating TwiML
MIT License
1.35k stars 462 forks source link

twilio-ruby 6.0 mangles .rbenv path #655

Closed broksonic21 closed 1 year ago

broksonic21 commented 1 year ago

Issue Summary

When using 6.0 in an rbenv/bundler setup, this line:

https://github.com/twilio/twilio-ruby/blob/1956b94a670527a2886d64ac72a596e84247b4f5/lib/twilio-ruby/rest.rb#L8

mangles paths and thus twilio-ruby can't be used as require "twilio-ruby"

This is because a path like this:

/Users/REDACTED/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/twilio-ruby-6.0.0/lib/twilio-ruby/rest/accounts.rb

gets turned into

/Users/REDACTED/.base_rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/twilio-ruby-6.0.0/lib/twilio-ruby/rest/accounts_base.rb

where the .rbenv gets modified into .base_rbenv (when the actual goal was just the filename getting that sub)

Steps to Reproduce

  1. use rbenv
  2. install twilio-ruby
  3. require 'twilio-ruby' ---> it fails

Code Snippet

n/a

Exception/Log

/Users/REDACTED/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/twilio-ruby-6.0.0/lib/twilio-ruby/rest/accounts.rb
Traceback (most recent call last):
/Users/REDACTED/.rbenv/versions/2.7.8/lib/ruby/site_ruby/2.7.0/rubygems/core_ext/kernel_require.rb:37:in `require': cannot load such file -- /Users/REDACTED/_base.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/twilio-ruby-6.0.0/lib/twilio-ruby/rest/accounts_base.rb (LoadError)

Technical details:

broksonic21 commented 1 year ago

Ah, likely covered by https://github.com/twilio/twilio-ruby/pull/654

shrutiburman commented 1 year ago

Thanks for the PR. We'll review it soon.Please expect the fix in upcoming release.

shrutiburman commented 1 year ago

Merged with 654.

ismasan commented 1 year ago

I hit this issue today. I know the fix is already merged, but: wouldn't it have been more reliable to just

base_file = file.gsub(/\.rb$/, '_base.rb')
require base_file if File.exists?(base_file)

... In that wat you let the existence/absence of the base files drive the load behaviour, instead of having to duplicate the logic here and bake in the exception for client.rb

Azzawie commented 1 year ago

I faced this issue today,

/Users/xxx/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in require': cannot load such file -- /Users/xxx/_base.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/twilio-ruby-6.0.0/lib/twilio-ruby/rest/accounts_base.rb (LoadError)

CanadianTux commented 1 year ago

I faced the exact same issue as @Azzawie did,

<internal:/Users/[REDACTED]/.rbenv/versions/3.1.2/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:148:in `require': cannot load such file -- /Users/[REDACTED]/_base.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/twilio-ruby-6.0.0/lib/twilio-ruby/rest/accounts_base.rb (LoadError)
    from <internal:/Users/[REDACTED]/.rbenv/versions/3.1.2/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:148:in `require'
    from /Users/[REDACTED]/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/twilio-ruby-6.0.0/lib/twilio-ruby/rest.rb:8:in `block in <top (required)>'
    from /Users/[REDACTED]/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/twilio-ruby-6.0.0/lib/twilio-ruby/rest.rb:7:in `each'
    from /Users/[REDACTED]/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/twilio-ruby-6.0.0/lib/twilio-ruby/rest.rb:7:in `<top (required)>'
    from <internal:/Users/[REDACTED]/.rbenv/versions/3.1.2/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:148:in `require'
    from <internal:/Users/[REDACTED]/.rbenv/versions/3.1.2/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:148:in `require'
    from send_sms.rb:6:in `<main>'
shrutiburman commented 1 year ago

The fix is merged, we're going to release it in the upcoming release this week. Meanwhile please use v5.77.0, if you're blocked by this.

shrutiburman commented 1 year ago

Hi everyone, we've released a new ruby version v6.0.1 with the fix. Please upgrade and let us know if you need further help. Thanks.

Closing this thread, please feel free to open another issue, if this persists.