Skip to content
This repository has been archived by the owner on Jan 21, 2024. It is now read-only.

Only getting a single stack frame with report.subscribe #23

Open
aleksabl opened this issue Jan 4, 2013 · 15 comments
Open

Only getting a single stack frame with report.subscribe #23

aleksabl opened this issue Jan 4, 2013 · 15 comments

Comments

@aleksabl
Copy link

aleksabl commented Jan 4, 2013

I have an issue with the stackInfo I get from report.subscription. It only contains a single stack element (stackInfo.stack.length is 1), which is the frame where the exception was thrown.

This JSFiddle shows the issue: http://jsfiddle.net/aleksabl/nMmTm/. My JSFiddle doesn't work with IE (because of mime-content mismatch with the tracekit.js file from GitHub), so you have to test it with Chrome or Firefox. I've also raised my issue at stackoverflow: http://stackoverflow.com/q/13975264/227192.

@devinrhode2
Copy link

This is because the error is not being caught anywhere and instead hitting
the window.onerror handler.. but I might know a way to actually get the
full stack trace in at least chrome and probably all modern browsers

-Devin http://zerply.com/DevinRhode2
http://zerply.com/devinrhode2

On Fri, Jan 4, 2013 at 12:40 AM, Aleksander Blomskøld <
[email protected]> wrote:

I have an issue with the stackInfo I get from report.subscription. It only
contains a single stack element (stackInfo.stack.length is 1), which is the
frame where the exception was thrown.

This JSFiddle shows the issue: http://jsfiddle.net/aleksabl/nMmTm/. My
JSFiddle doesn't work with IE (because of mime-content mismatch with the
tracekit.js file from GitHub), so you have to test it with Chrome or
Firefox. I've also raised my issue at stackoverflow:
http://stackoverflow.com/q/13975264/227192.


Reply to this email directly or view it on GitHubhttps://github.com//issues/23.

@aleksabl
Copy link
Author

aleksabl commented Jan 4, 2013

Oh, I thought using TraceKit to get nice stack traces of unhandled exceptions was one of it's main use cases.

If this behavior is expected/considered a feature, it should probably be documented somewhere... :)

@devinrhode2
Copy link

Indeed, the behavior is to get a full stack trace, you need to properly
throw an error like this: throw new Error('oops')

  1. Always try to catch errors instead of letting them hit the
    window.onerror handler. Once they hit the onerror handler, there's no real
    stack trace we can glean.
  2. Prefer throw new Error("foo") until I do this little patch of
    re-throwing a real Error object
  3. Nicholas C. Zakas recommends this style here:
    http://www.nczonline.net/blog/2009/03/03/the-art-of-throwing-javascript-errors/

@devinrhode2
Copy link

Actually, I won't be doing this patch because I can't actually get stack trace info from where you throw "error" - therefore in our readme/guide we need to note to always throw errors like this: throw new Error('foo')

@aleksabl
Copy link
Author

aleksabl commented Jan 4, 2013

Oh, that's a bummer, since most of the unhandled exceptions doesn't come from my own code.

@aleksabl
Copy link
Author

aleksabl commented Jan 4, 2013

I did a new test with throw new Error, and it doesn't seem to change anything: http://jsfiddle.net/aleksabl/uvtJ5/1/

TraceKit.report.subscribe(function(stackInfo) { alert(stackInfo.stack.length);});

function foo() {
   bar();
}

function bar() {                                                
   throw new Error("oops");
}

foo();               

Alerts 1.

@devinrhode2
Copy link

  1. Always try to catch errors instead of letting them hit the
    window.onerror handler. Once they hit the onerror handler, there's no real
    stack trace we can glean.
  2. Always throw errors with throw new Error('foo') we can't get a full stack trace when you throw 'foo'

We'll have add this to usage instructions.

Here is a function that wraps another function in a try/catch block and sends the caught errors to TraceKit, it should help you out:

/**
 * TraceKit.catch
 * catch all errors from some function scope.
 * 
 * Example using the prototype:
 * (function() {
 *   //your program
 * }).wrapInTryCatch()() //.wrapInTryCatch does not execute the function
 * 
 * Without prototype:
 * TraceKit.catch(function(){
 *   //code
 * })();
 */
TraceKit.catch = TraceKitCatch;
function TraceKitCatch(fn) {
  //to return
  function TraceKitCatchWrapper() {
    try {
      if (!fn || !fn.apply) {
        throw new Error('function passed to TraceKit.catch has no apply method..');
      }
      fn.apply(this, [].slice.call(arguments, 0));
    } catch (e) {
      TraceKit.report(e);
    }
  }
  return TraceKitCatchWrapper;
}

Function.prototype.wrapInTryCatch = FuncProtoWrapInTryCatch;
function FuncProtoWrapInTryCatch() {
  return TraceKit.catch(this);
}

@occ I say we flat out add this to TraceKit

@devinrhode2
Copy link

All javascript code should be wrapped in this:

(function(){
  //code
}).wrapInTryCatch()();

instead of just:

(function(){
  //code
})();

@devinrhode2
Copy link

Note: I could also hijack all function .calls to be run in a TraceKit try/catch wrapper, but I'm not sure if that's a good idea because it's kind of intrusive. For coffeescript code it might be pretty nice though, since all scopes are wrapped in (function(){ .. }).call(this)

@occ
Copy link
Owner

occ commented Jan 17, 2013

Overriding Function.call() is indeed too intrusive. It will cause performance issues.

@aleksabl has a proper use case here. TraceKit is handling the unhandled exception there. It's just not generating the right stack trace.

@kmontag
Copy link

kmontag commented Jan 26, 2013

+1 for supporting this use case with a proper stack trace. IMHO wrapping every bit of JS in a try/catch block is totally prohibitive, since it needs to be done for every async callback in an application (event handlers, AJAX calls, ...), and external libraries can't easily support this pattern.

That said, I don't know enough about the onerror implementations to offer anything particularly constructive. Is there really no way to get a proper stack trace once an error hits this point? Even partial browser support would certainly be better than nothing.

@devinrhode2
Copy link

For browsers supporting a stack property, something like this could be done I think:

jsonErrorReport.stack = (new Error('force-added stack trace!').stack;

Also, I'm not sure every bit of JS needs to be wrapped in a try/catch, just as long as the error bubbles up and is caught instead of hitting window.onerror

This is something I've been meaning to investigate for a really long time but haven't.

In the case that every callback does need to be wrapped in a try/catch, I have a method that mutates an api to automatically handle it.

@occ
Copy link
Owner

occ commented Jan 27, 2013

The problem is window.onerror's interface.

onerror handler gets the error message, URL, and the line number. (See https://developer.mozilla.org/en-US/docs/DOM/window.onerror#Parameters). Since we don't get the actual Error object, we can't gather a stack trace.

We need the try/catch block to be able to get the error object.
There are a few workarounds (like @devinrhode2's Function.call() suggestion) so we can automatically wrap function calls in try/catch blocks and catch uncaught/bubbling exceptions. Other suggestions include rewriting the JS (to wrap in try/catch) or attaching to debugger interfaces of the browser.

There are also suggestions on updating the .onerror event interface like adding a 4th argument and passing the error object. Obviously this would be the best option- but we'd still need to support older browsers.

@devinrhode2
Copy link

@occ that's a really good idea for onerror does this fall under the DOM
API standard? What exact standards group would discuss this addition to
browsers?

-Devin http://zerply.com/DevinRhode2
http://zerply.com/devinrhode2

On Sun, Jan 27, 2013 at 10:02 AM, Onur Can Cakmak
[email protected]:

The problem is window.onerror's interface.

onerror handler gets the error message, URL, and the line number. (See
https://developer.mozilla.org/en-US/docs/DOM/window.onerror#Parameters).
Since we don't get the actual Error object, we can't gather a stack trace.

We need the try/catch block to be able to get the error object.
There are a few workarounds (like @devinrhode2https://github.com/devinrhode2's
Function.call() suggestion) so we can automatically wrap function calls in
try/catch blocks and catch uncaught/bubbling exceptions. Other suggestions
include rewriting the JS (to wrap in try/catch) or attaching to debugger
interfaces of the browser.

There are also suggestions on updating the .onerror event interface like
adding a 4th argument and passing the error object. Obviously this would be
the best option- but we'd still need to support older browsers.


Reply to this email directly or view it on GitHubhttps://github.com//issues/23#issuecomment-12758250.

@occ
Copy link
Owner

occ commented Jan 27, 2013

Here's a thread I've found http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2012-May/035732.html
Even if this was accepted and implemented TraceKit (or similar projects :P) would still need a way to handle the issue on older browsers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants