sendgrid / sendgrid-ruby

The Official Twilio SendGrid Led, Community Driven Ruby API Library
https://sendgrid.com
MIT License
621 stars 324 forks source link

Exception stemming from starkbank-ecdsa inclusion in today's 6.3.0 release #426

Closed vestedpr-dev closed 4 years ago

vestedpr-dev commented 4 years ago

Thanks for your new release! Something to escalate: as our tests were running in CI today we suddenly got a ton of failures and quite obscure ones too. It took me about 45m in the debugger to track it down to your gem's latest update to 6.3.0 today.

I filed the fundamental issue in their repo here: https://github.com/starkbank/ecdsa-ruby/issues/4 ... but I wanted to drop you a line here since I think this issue may be more urgent for you guys at SendGrid ... since you are a commercial enterprise with a lot of clients like myself whereas that seems to be a relatively more obscure library.

I suspect starkbank-ecdsa overrides Ruby's elemental File.read method in a way which might cause a lot of hard to debug problems in codebases which include your sendgrid-ruby gem. Since you just released your new gem a few hours ago this might be something to verify and consider patching up.

In our tests I saw exceptions like:

undefined method `upcase' for {:mode=>"r:UTF-8"}:Hash
starkbank-ecdsa (0.0.2) lib/utils/file.rb:4:in `read'
ffaker (2.15.0) lib/ffaker/utils/module_utils.rb:21:in `const_missing'

To fix I just pinned our codebase to use the previous version of your gem:

s.add_runtime_dependency "sendgrid-ruby", "6.2.1"
sybohy commented 4 years ago

Same problem here. Can't deploy anymore. Rolling back to 6.2.1

jacobat commented 4 years ago

Overriding File.read is a big no-no. Please don't include this library until it stops doing things like this.

swiknaba commented 4 years ago

can the maintainers yank this version then? Messing with a core Ruby method is not cool.

childish-sambino commented 4 years ago

Ack, working on yanking it now ...

tmertens commented 4 years ago

Same here. This library is very new, only on version 0.0.2 and does not seem to be production ready.

childish-sambino commented 4 years ago

6.3.0 yanked. Will hold it back until the above linked issue is resolved.