3.0.5 breaks Airbrake.notify(hash) #34

Closed
msepcot opened this Issue Nov 14, 2011 · 12 comments

Comments

Projects
None yet
@msepcot

msepcot commented Nov 14, 2011

This commit breaks Airbrake.notify when called with a hash.

Example:

ruby-1.9.2-p180 :001 > require 'airbrake'
 => true 
ruby-1.9.2-p180 :002 > Airbrake.notify({})
NoMethodError: undefined method `backtrace' for {}:Hash
    from /Users/msepcot/.rvm/gems/ruby-1.9.2-p180/gems/airbrake-3.0.5/lib/airbrake/notice.rb:223:in `from_exception'
    from /Users/msepcot/.rvm/gems/ruby-1.9.2-p180/gems/airbrake-3.0.5/lib/airbrake/notice.rb:208:in `exception_attribute'
    from /Users/msepcot/.rvm/gems/ruby-1.9.2-p180/gems/airbrake-3.0.5/lib/airbrake/notice.rb:96:in `initialize'
    from /Users/msepcot/.rvm/gems/ruby-1.9.2-p180/gems/airbrake-3.0.5/lib/airbrake.rb:142:in `new'
    from /Users/msepcot/.rvm/gems/ruby-1.9.2-p180/gems/airbrake-3.0.5/lib/airbrake.rb:142:in `build_notice_for'
    from /Users/msepcot/.rvm/gems/ruby-1.9.2-p180/gems/airbrake-3.0.5/lib/airbrake.rb:103:in `notify'
    from (irb):2
    from /Users/msepcot/.rvm/rubies/ruby-1.9.2-p180/bin/irb:16:in `<main>'

In test/notifier_test.rb "create and send a notice for a hash" stubs out the Airbrake::Notice#new which hides the error. In 3.0.4 and below, build_notice_for only set :exception when the argument did not respond to a to_hash call. With 3.0.5, :exception is always set, which is causing the above error to be raised when the argument is a hash.

@der-flo

This comment has been minimized.

Show comment Hide comment
@der-flo

der-flo Nov 15, 2011

Jep, I can confirm this.

der-flo commented Nov 15, 2011

Jep, I can confirm this.

@seanmcoleman

This comment has been minimized.

Show comment Hide comment
@seanmcoleman

seanmcoleman Nov 15, 2011

This is also happening to me.

This is also happening to me.

@iurimatias

This comment has been minimized.

Show comment Hide comment
@iurimatias

iurimatias Nov 16, 2011

same here, please see #19

same here, please see #19

@ekampf

This comment has been minimized.

Show comment Hide comment
@ekampf

ekampf Nov 16, 2011

Contributor

same here

Contributor

ekampf commented Nov 16, 2011

same here

@iurimatias

This comment has been minimized.

Show comment Hide comment
@iurimatias

iurimatias Nov 16, 2011

begin
  raise "theexception"
rescue Exception => e
  Airbrake.notify(
    :error_class   => e.class.name,
    :error_message => e.try(:message),
    :parameters    => params,
    :backtrace     => e.try(:backtrace),
    :session       => session
  )
end

results in NoMethodError (undefined method `backtrace' for #Hash:0x0000010ba8c0a8)

begin
  raise "theexception"
rescue Exception => e
  Airbrake.notify(
    :error_class   => e.class.name,
    :error_message => e.try(:message),
    :parameters    => params,
    :backtrace     => e.try(:backtrace),
    :session       => session
  )
end

results in NoMethodError (undefined method `backtrace' for #Hash:0x0000010ba8c0a8)

@jcode

This comment has been minimized.

Show comment Hide comment
@jcode

jcode Nov 16, 2011

confirmed for me using Rails 2.3.14 and Ruby 1.8.7

jcode commented Nov 16, 2011

confirmed for me using Rails 2.3.14 and Ruby 1.8.7

@humandoing

This comment has been minimized.

Show comment Hide comment
@humandoing

humandoing Nov 16, 2011

This is a bug that I found as well. After looking at the code, it looks like the Airbrake.notify method now expects two parameters, the first being an Error object and the 2nd being a Hash. I haven't tested this extensively, but here is what should be an easy workaround (passing a new RuntimeError instance as the first parameter in the call to Airbrake.notify)

Airbrake.notify( RuntimeError.new,
:error_class => e.class.name,
:error_message => e.try(:message),
:parameters => params,
:backtrace => e.try(:backtrace),
:session => session
)

This is a bug that I found as well. After looking at the code, it looks like the Airbrake.notify method now expects two parameters, the first being an Error object and the 2nd being a Hash. I haven't tested this extensively, but here is what should be an easy workaround (passing a new RuntimeError instance as the first parameter in the call to Airbrake.notify)

Airbrake.notify( RuntimeError.new,
:error_class => e.class.name,
:error_message => e.try(:message),
:parameters => params,
:backtrace => e.try(:backtrace),
:session => session
)

@msepcot

This comment has been minimized.

Show comment Hide comment
@msepcot

msepcot Nov 16, 2011

It's not necessary to pass a RuntimeError to Airbrake.notify, simply passing nil as the first parameter is an easy workaround, but there is still a mismatch between what is expected from the documentation and what is implemented.

msepcot commented Nov 16, 2011

It's not necessary to pass a RuntimeError to Airbrake.notify, simply passing nil as the first parameter is an easy workaround, but there is still a mismatch between what is expected from the documentation and what is implemented.

@humandoing

This comment has been minimized.

Show comment Hide comment
@humandoing

humandoing Nov 16, 2011

Yup, fair enough. I just figured since it seems to be expecting an Error object, might as well give it one.

Yup, fair enough. I just figured since it seems to be expecting an Error object, might as well give it one.

@stuartchaney

This comment has been minimized.

Show comment Hide comment
@stuartchaney

stuartchaney Nov 17, 2011

Contributor

Guys thanks for raising this. Fix on its way.

Contributor

stuartchaney commented Nov 17, 2011

Guys thanks for raising this. Fix on its way.

@charleseff

This comment has been minimized.

Show comment Hide comment
@charleseff

charleseff Nov 28, 2011

+1, waiting for the fix

+1, waiting for the fix

@apolzon

This comment has been minimized.

Show comment Hide comment
@apolzon

apolzon Nov 28, 2011

+1, looking forward to fix, thanks for tracking this guys!

apolzon commented Nov 28, 2011

+1, looking forward to fix, thanks for tracking this guys!

leonid-shevtsov added a commit to Playhem/airbrake that referenced this issue Nov 30, 2011

leonid-shevtsov added a commit to Playhem/airbrake that referenced this issue Nov 30, 2011

leonid-shevtsov added a commit to Playhem/airbrake that referenced this issue Nov 30, 2011

leonid-shevtsov added a commit to Playhem/airbrake that referenced this issue Nov 30, 2011

dvdplm added a commit that referenced this issue Dec 2, 2011

Merge pull request #42 from Playhem/fix-issue-34-cherrypicked
Fix issue #34 (undefined method backtrace for Hash)

@ghost ghost assigned dvdplm Dec 2, 2011

@dvdplm dvdplm closed this Dec 11, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment