Why not use a default window.onError event instead of explicit try/catch? #1

Closed
meeiw opened this Issue Jan 17, 2013 · 7 comments

Comments

Projects
None yet
3 participants
@meeiw

meeiw commented Jan 17, 2013

Then we have a global entry point for all errors, and configuration becomes minimal.

https://developer.mozilla.org/en-US/docs/DOM/window.onerror

@joshuap

This comment has been minimized.

Show comment
Hide comment
@joshuap

joshuap Jan 17, 2013

Member

Actually, TraceKit does set a window.onerror handler which we are
subscribing to by default. I forgot to put anything in the readme about
that :). Eventually I want to make it a configuration option, where you can
choose whether to catch global/uncaught window errors or not. In the
meantime, it should automatically notify Honeybadger for errors you don't
explicitly catch. Please let me know how it works for you.

On Jan 17, 2013, at 12:43 AM, Kristian Hellquist notifications@github.com
wrote:

Then we have a global entry point for all errors, and configuration becomes
minimal.

https://developer.mozilla.org/en-US/docs/DOM/window.onerror


Reply to this email directly or view it on
GitHubhttps://github.com/honeybadger-io/honeybadger-js/issues/1.

Member

joshuap commented Jan 17, 2013

Actually, TraceKit does set a window.onerror handler which we are
subscribing to by default. I forgot to put anything in the readme about
that :). Eventually I want to make it a configuration option, where you can
choose whether to catch global/uncaught window errors or not. In the
meantime, it should automatically notify Honeybadger for errors you don't
explicitly catch. Please let me know how it works for you.

On Jan 17, 2013, at 12:43 AM, Kristian Hellquist notifications@github.com
wrote:

Then we have a global entry point for all errors, and configuration becomes
minimal.

https://developer.mozilla.org/en-US/docs/DOM/window.onerror


Reply to this email directly or view it on
GitHubhttps://github.com/honeybadger-io/honeybadger-js/issues/1.

@joshuap

This comment has been minimized.

Show comment
Hide comment
@joshuap

joshuap Jan 17, 2013

Member

I take that back. While the current build does support catching window.onerror events, the data that we can get back from TraceKit is much more limited than what you get if you wrap your code in a try/catch block. From TraceKit's README:

In order to get stack traces, you need to wrap your code in a try/catch block like above. Otherwise the error hits window.onerror handler and will only contain the error message, line number, and column number.

I just tried this, and while it did report the unhandled error to Honeybadger, it did not include the error class name, and only the first line of the stack trace (where the global error occurred).

I will look into a fix for this, but in the meantime my official recommendation is to use try/catch, as window.onerror seems to come with some caveats.

Sorry for the confusion!

Member

joshuap commented Jan 17, 2013

I take that back. While the current build does support catching window.onerror events, the data that we can get back from TraceKit is much more limited than what you get if you wrap your code in a try/catch block. From TraceKit's README:

In order to get stack traces, you need to wrap your code in a try/catch block like above. Otherwise the error hits window.onerror handler and will only contain the error message, line number, and column number.

I just tried this, and while it did report the unhandled error to Honeybadger, it did not include the error class name, and only the first line of the stack trace (where the global error occurred).

I will look into a fix for this, but in the meantime my official recommendation is to use try/catch, as window.onerror seems to come with some caveats.

Sorry for the confusion!

@meeiw

This comment has been minimized.

Show comment
Hide comment
@meeiw

meeiw Jan 19, 2013

I see. I'll try the global the handler and see if its sufficient for our app, otherwise I'll use the try/catch block

Thanks!

meeiw commented Jan 19, 2013

I see. I'll try the global the handler and see if its sufficient for our app, otherwise I'll use the try/catch block

Thanks!

@cmer

This comment has been minimized.

Show comment
Hide comment
@cmer

cmer Mar 10, 2013

Would love to be able to turn off window.onerror. I just get tons of random exceptions from 3rd party libraries sent to HB. AFAIK they don't affect our site at all...

Any ETA on this?

cmer commented Mar 10, 2013

Would love to be able to turn off window.onerror. I just get tons of random exceptions from 3rd party libraries sent to HB. AFAIK they don't affect our site at all...

Any ETA on this?

@joshuap

This comment has been minimized.

Show comment
Hide comment
@joshuap

joshuap Mar 10, 2013

Member

I'll work on this in the next couple days. I think I'm going to disable the window.onerror handler by default and add a config option to explicitly enable it.

Since it's not a very reliable way to monitor JavaScript errors, we should probably be good citizens and discourage its use. :)

Member

joshuap commented Mar 10, 2013

I'll work on this in the next couple days. I think I'm going to disable the window.onerror handler by default and add a config option to explicitly enable it.

Since it's not a very reliable way to monitor JavaScript errors, we should probably be good citizens and discourage its use. :)

@joshuap joshuap closed this in a6fb183 Mar 13, 2013

@joshuap

This comment has been minimized.

Show comment
Hide comment
@joshuap

joshuap Mar 13, 2013

Member

@cmer if you use the latest build, you shouldn't need to change anything else. :)

Member

joshuap commented Mar 13, 2013

@cmer if you use the latest build, you shouldn't need to change anything else. :)

@cmer

This comment has been minimized.

Show comment
Hide comment
@cmer

cmer Mar 13, 2013

Timely! Thanks!

cmer commented Mar 13, 2013

Timely! Thanks!

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