Support for "Unhandled promise rejection" / "Uncaught (in promise)" #424
Comments
|
Now, an argument could be made that an unhandled promise rejection shouldn't be caught by Raven ... because sometimes you intentionally choose not to handle rejection. An example in non-Promise world would be to create an XMLHttpRequest without a traditional error handler (e.g. no |
|
In our case, it happens in a React Component context:
Even tho the error is raised, it's not tracked by Raven. |
|
Why you talkin' that way 'bout my momma? But that's a great example, and in this case, absolutely it should be caught. Now ... do we add this to the "react" plugin? Or do we add a "promises" plugin, and then instruct React users to use this? Or just make it on by default? cc @mattrobenolt |
|
@joaocunha – so @mattrobenolt and I have had a discussion about this. It seems like So ... we feel this might be a If anyone has similar examples re: uncaught promise rejections, we'd love to hear 'em. |
|
Great to know you guys found the starting point. Really appreciated. In the meanwhile, should we be using the React build of Raven? How do we do that with npm and CommonJS? We are currently loading |
|
@benvinegar also, let me know if you need more info about our stack. We're using an express server, Webpack and Redux. Adding to that, we're not doing isomorphic/universal JS at the moment. These are the dependencies we currently have in: |
|
@benvinegar We changed the API and, as it was not matching the new specs, it raised some errors. Straight from
It seems it won't ever capture exceptions in that context. |
|
@joaocunha – looking at the code above, it looks like you could do the following as a workaround: window.onunhandledrejection = function(data) {
Raven.captureException(data.reason);
} |
|
Oops wrong button :) |
|
That workaround does work indeed, thanks! |
|
Thanks for verifying @joaocunha. :) I'm not sure if we'll want to add this into raven-js core though since this I'm going to just drop some links about this topic here for reference: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/SRsFwwEgnUQ I'd want to research and see if this is actually implemented by the popular Promise polyfills and if it's starting to get implemented in browsers (looks like Blink merged in support), I'll happily add some native support like we do for |
|
Also, since it passes the promise itself through as |
|
Note that we've since documented how to work with Promises in the docs: https://docs.getsentry.com/hosted/clients/javascript/usage/#promises |
|
It seems steps are been taken to standardise the |
|
@jabooth – thanks for linking this. I'm still not sure that this should be done by default, though. I think the code example we've added to the docs is fine for now. |
|
I've tried it and all it presents from the object in Sentry's interface is "[object PromiseRejectionEvent]". Using the same code as in the docs:
|
|
@joaomilho – Oops. I'm looking for a specification for But from your example above, and from the relevant Bluebird tests, it looks like the Error object is the window.addEventListener('unhandledrejection', (err) => {
Raven.captureException(err.reason);
});I'm going to update the docs. |
|
@joaomilho – So, I played around with this, and the thing is, The examples from MDN actually demonstrate passing both an error and a string: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/reject The problem here is that Perhaps the best solution is: window.addEventListener('unhandledrejection', (err) => {
var reason = err.reason;
if (err instanceof Error) {
Raven.captureException(reason);
} else if (Object.prototype.toString.call(reason) === '[object String]') {
Raven.captureMessage(reason);
}
});We could also introduce a function, say, Edit: @mattrobenolt points out that |
|
@benvinegar fwiw: https://github.com/getsentry/raven-js/blob/master/src/raven.js#L292-L293
|
|
Bam, perfect. |
|
|
|
So, just to be 100% clear, window.addEventListener('unhandledrejection', (err) => {
Raven.captureException(err.reason);
});would be enough, right? Another question, since I'm using fetch, all errors will go to my Example: // checkResponse will return the response or throw it based on "ok"
fetch(myFetchConfig).then(checkResponse)
.then((response) => {
///... some correct code
someFnThatDoesntExist()
})
.catch((response) => {
// try to do something with response, like response.json().then(...)
})In this case, I'll get an err object instead of a response object in my catch. So I'll get an error in sentry related to trying to use the response object, like "TypeError: response.json is not a function" instead of "Uncaught ReferenceError: someFnThatDoesntExist is not defined". What do you this is the best approach to handle it? |
|
Doing a simple check solves it, like: But it seems incorrect to me to do it on every catch handler. |
|
I'm using Babel (which uses core-js), to get it to work there I've had to:
My handler looks like: window.onunhandledrejection = e => {
console.warn('Unhandled Promise Rejection', e.reason)
Raven.captureException(e.reason, {
extra: { unhandledPromise: true }
})
}And to delete the native Promise, I've got a file if (window.Promise) {
console.debug(`This browser provides a native Promise. I'm disabling it to force use of core-js to allow "onunhandledrejection".`)
delete window.Promise
}and I'm loading that as the first file in my module.exports = {
entry: [
'./src/disableNativePromises',
'babel-polyfill',
'./src/index'
],
// ...
} |
|
@joaomilho – any chance you could throw up a JSBin of a fully-functioning example? |
Painful. If I'm reading this correctly, I guess this is because Babel sees the native Hmm, I wonder how we should document this ... or perhaps open a ticket on Babel? |
^ Same here. |
|
It's a core-js issue, not Babel (Babel just uses core-js for most polyfills afaik). But yeah, the issue is core-js currently doesn't take the Promise rejection events into account when deciding whether it should replace the native Promise implementation. There's a discussion thread here: zloirock/core-js#140, and there's a warning in their readme here. |
|
@benvinegar added to my backlog, but can't promise a deadline ;) |
|
zloirock/core-js#140 was resolved, though I wouldn't close this issue until a suitable workaround can be automated. awesome project! |
ghost
mentioned this issue
|
Here's an example Gist implementing @benvinegar original workaround, with backoff for latent network connections (handy if you're loading this library via the Sentry integration for Segment). |
|
I'm running into this issue and I've added the
Any ideas? |
|
what's the TL;DR solution here? |
@jessepollak – Raven.js wraps async methods in try/catch for uncaught exceptions (literally
@MarkMurphy – We're going to make it a config option soon-ish, but in the meantime you can implement yourself pretty easily: |
|
Just a heads up for anyone coming across this thread. There is a bug in Chrome 51 and higher (as recent as Chrome 56) where unhandled promise rejections are never fired on localhost. If you're testing locally then you may never see this event fired! |
|
Just updated the issue above about Chrome. |
|
Just a note. If you are using Redux-Thunk with async/await, you can catch these unhandled promise rejections by catching redux-thunk errors manually. code here https://gist.github.com/akhayoon/24be6ca9634d228893b8c4bd79f0f33c |
|
It looks like the original issue has been already resolved |
|
@kamilogorek @benvinegar have you implemented the config option? It took a year, isn't it? |
|
Hey @avesus, no, not yet, as nowadays people are using various promises implementation, which makes it hard to do. For now this https://docs.sentry.io/clients/javascript/usage/#promises is what we recommend, but this may change one day. |
It's making the investigation of performance issues easier.
Example use case (JSBin):
If the promise throws an exception, and there's no error handler, the browser will spit out an uncaught promise error.
The text was updated successfully, but these errors were encountered: