chargebee / chargebee-ruby

Ruby library for the Chargebee API.
https://apidocs.chargebee.com/docs/api?lang=ruby
MIT License
32 stars 60 forks source link

ChargeBee::APIError#message overrides (and breaks) StandardError#message #5

Closed bkon closed 7 years ago

bkon commented 9 years ago

Reproduction steps:

Assuming that you have ChargeBee already configured, run

begin
  ChargeBee::Addon.retrieve(1) # or any other ID which does NOT exit in your environment
rescue ChargeBee::Error => e
  p e.message
end

Current output:

nil

Expected output:

Sorry, we couldn't find that resource

Cause:

ChargeBee::APIError (indirectly) inherits from StandardError and passes message from json_obj[:message] as a parameter to StandardError constructor. Normally that should make message accessible as .message, but APIError defines attr_accessor :message which is never initialized.

kubenstein commented 8 years ago

i have same issue with InvalidRequestError. @bkon diagnosis is correct. I prepared PR with fix and test.

ayanko commented 8 years ago

@SangeethaBaskaran @kubenstein

Just remove :message from :attr_reader. There is no need to override StandardError#message.

https://github.com/chargebee/chargebee-ruby/blob/v2.1.1/lib/chargebee/errors.rb#L16

igneus commented 8 years ago

Dear ChargeBee team, I'm not sure if you understand how serious this issue is. It's a very bad surprise for a Ruby developer to encounter an exception without #message explaining what's going on. Please address this issue.

(Yes, I know already that the message can be retrieved from #json_obj. But it's very unusual in the Ruby community to force developers using a library to read it's source code only to find out how to get an exception's error message.)

ayanko commented 8 years ago

@igneus As workaround add to your chargebee initializer:

# @see https://github.com/chargebee/chargebee-ruby/issues/5
# TODO: Remove next hack when they fix the "bug"
ChargeBee::APIError.send :remove_method, :message
saravana-cb commented 8 years ago

@vivek: Please look into this

On Sat, Sep 10, 2016 at 1:10 PM, Andriy Yanko notifications@github.com wrote:

@igneus https://github.com/igneus As workaround add to your chargebee initializer:

@see https://github.com/chargebee/chargebee-ruby/issues/5# TODO: Remove next hack when they fix the "bug"ChargeBee::APIError.send :remove_method, :message

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/chargebee/chargebee-ruby/issues/5#issuecomment-246097534, or mute the thread https://github.com/notifications/unsubscribe-auth/AC3cIJYNJTigsmdCEiyNJGMf7F2rv6Zdks5qol7XgaJpZM4GANOy .

ayanko commented 8 years ago

bkon opened this issue on Sep 19, 2015

@vivek: I suggest to wait yet a bit (9 days) so that you can celebrate this PR anniversary :smile:

ayanko commented 7 years ago

Is this fixed?

SangeethaBaskaran commented 7 years ago

@ayanko This issue has been fixed. Refer the change log here.

ayanko commented 7 years ago

Thx