Strip Raven's wrapped try/catch call from frames [WIP]
#581
Conversation
| // If not an Error is passed through, recall as a message instead | ||
| if (!isError(ex)) return this.captureMessage(ex, options); | ||
| if (!isError(ex)) return this.captureMessage(ex, options, skipframes + 1); |
There was a problem hiding this comment.
Err, this isn't necessary ... yet.
There was a problem hiding this comment.
So, by incrementing and passing skipframes to captureMessage, we can generate a new stack by throwing an error from inside captureMessage, slice off skipframes frames, and then we can send that w/ the payload to Sentry. As in, we can now attach valid stack traces alongside captureMessage calls.
I was going to do that in a separate PR.
|
Alternatively we could keep the frame, but tag it as |
|
@benvinegar why do we not just completely remove all frames that originate in raven-js? |
|
@mitsuhiko – If you mean, why don't we identify them as being from raven-js via the culprit/filename, the problem is that we cannot reliably identify our frames when the code is minified, which most raven-js users are doing. We could potentially do this, on the server, after source maps have been applied ... but not everyone uses source maps. I also wouldn't make such a change until we move stack trace parsing onto the server. But you did just tip me off to a problem: |
|
Mark as |
|
This was partially implemented in #582 |
When users see try/catch in their stack traces, they believe the bug is Raven's fault (read: ours).
This patch makes it so that the correct number of frames are sliced from the top of the frames array before being sent to Sentry.
This also means that wrapped functions should now produce the same results as strictly doing
window.onerror. This makes it possible for us to remove function instrumentation from modern browsers that support a fully-functioningwindow.onerror(which we can feature detect).cc @mattrobenolt @mitsuhiko