TraceKit icon indicating copy to clipboard operation
TraceKit copied to clipboard

Only getting a single stack frame with report.subscribe

Open aleksabl opened this issue 13 years ago • 15 comments

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.

aleksabl avatar Jan 04 '13 06:01 aleksabl

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/occ/TraceKit/issues/23.

devinrhode2 avatar Jan 04 '13 06:01 devinrhode2

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... :)

aleksabl avatar Jan 04 '13 07:01 aleksabl

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 avatar Jan 04 '13 07:01 devinrhode2

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')

devinrhode2 avatar Jan 04 '13 08:01 devinrhode2

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

aleksabl avatar Jan 04 '13 08:01 aleksabl

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.

aleksabl avatar Jan 04 '13 08:01 aleksabl

  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 avatar Jan 04 '13 20:01 devinrhode2

All javascript code should be wrapped in this:

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

instead of just:

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

devinrhode2 avatar Jan 04 '13 23:01 devinrhode2

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)

devinrhode2 avatar Jan 04 '13 23:01 devinrhode2

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.

occ avatar Jan 17 '13 15:01 occ

+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.

kmontag avatar Jan 26 '13 23:01 kmontag

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.

devinrhode2 avatar Jan 27 '13 17:01 devinrhode2

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.

occ avatar Jan 27 '13 18:01 occ

@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/occ/TraceKit/issues/23#issuecomment-12758250.

devinrhode2 avatar Jan 27 '13 18:01 devinrhode2

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.

occ avatar Jan 27 '13 18:01 occ