search

bugsnag / bugsnag-node

[DEPRECATED] Please upgrade to our Universal JS notifier "@bugsnag/js" • https://github.com/bugsnag/bugsnag-js
https://www.bugsnag.com/
MIT License
48 stars 55 forks source link

Cannot read property 'stack' of null #110

Closed adnan-i closed 7 years ago

adnan-i commented 7 years ago

Is .notify method designed to only accept instance of Error as first param? If this method is called with null it fails with Cannot read property 'stack' of null exception. I only discovered this after I experienced it on my production server. Not good.

I understand that the docs say the first param should be either Error object or String, but still I can't appreciate the error reporting library crashing my app when I call it with something unexpected. Thanks.

bengourley commented 7 years ago

Hi @adnan-i, thanks for the filing this issue.

The notify(Error, …) method should only ever be called with Errors or Strings. Using it in any other way is incorrect and is not supported. Sending null or undefined or any other value where property access throws an error is not supported and will definitely cause the application to crash.

I agree with you that incorrect usage of the Bugsnag client should not crash your program, so I will bring this up in our internal roadmap discussions. I could imagine something like notifying with a BugsnagUsageError sending along the metadata of what you tried to report. These would get reported to your dashboard but prevent your program from crashing.

From the mean time you will need to guard against such situations with something like the following:

if (error) Bugsnag.notify(error)
kattrali commented 7 years ago

I could imagine something like notifying with a BugsnagUsageError sending along the metadata of what you tried to report.

I think this would be a solid approach. We could also log a warning when this coercion happens.