bogus frame added to stack trace

[acthp]

I'm not sure what problem augmentStackTraceWithInitialElement is trying to solve, but its effect is to add a bogus frame element pointing to TraceKit. It's particularly bad because it happens via window.onerror, so there's no column information, and so it can't be source-mapped back to TraceKit automatically: if it were I could prune it.

Does TraceKit always add this frame? If so, I can just always prune the first frame when logging the stack. However, I see multiple calls to notifyHandlers, and only one of them calls augmentStackTraceWithInitialElement, so probably I can't ignore it.

I'm also puzzled that an error caught with TraceKit.wrap() is (sometimes) bounced through window.onerror. Not sure what the point of that is.

[rmacfadyen]

I get this too. Spent a bunch of time trying to track it down... but no solid details.

The problem appears to be tracekit re-throwing exceptions... which results in windowonerror, which if being handled by tracekit ends up resulting in the spurious frame.

My "fix" is to have tracekit not rethrow exceptions (either in the windowonerror handler, in TraceKit.report, or in TraceKit.wrap).

All in all I'm not happy with the solution though. I'm sure there's a reason why these "throws" exist... but it's totally unclear from the code why they're necessary.

EDIT: I think the problem may be related to an "confirm(..)" that my handler invokes.

[devinrhode2]

I've been working on TraceKit for awhile and was also puzzled by this, similarly thinking there must be a good reason that it's being done..

From the first commit ever: 46c56d9

This is REQUIRED in order to get a useful stack trace in IE.
If the exception does not reach the top of the browser, you will only
get a stack trace from the point where TraceKit.report was called.

However, by the logic of this comment, this should be removed because we don't actually care about anything after TraceKit.report

If we think purely about the concept of re-throwing errors... you could take the stance that the exception would bubble up if it wasn't wrapped in a try/catch block, and should therefore be thrown so no behavior changes, triggering window.onerror.

But in reality, the only thing that happens when you re-throw an error is window.onerror gets called. OR another function that wrapped this function will catch the error. Both of these are redundant/inefficient and can be more directly/efficiently handled - call window.onerror directly with standardized arguments, or the other function that wrapped this function can simply subscribe to exceptions from TraceKit.

So TraceKit should not re-throw errors, but the the user can manually re-throw an error if they so choose.

Would you agree @mattrobenolt?

Also, if TraceKit.report were to re-throw errors, this otherwise perfectly fine use of try/catch blocks would not work:

  function sendUncaughtException(ex) {
    // Assume good input and catch the error if there is one.
    try {
      // Ensure stack property is computed. Or, attempt to alias Opera 10's stacktrace property to it
      ex.stack || (ex.stacktrace ? (ex.stack = ex.stacktrace) : '');
    } catch ( ex2 ) {
      TraceKit.report(ex2);
      if (!root['sendUncaughtException']['allowPrimitives']) {
        if (window.console && console.error) {
          console.error([
            'PRIMITIVE VALUE THROWN:' + ex,
            'Please do: throw new Error("message"); instead of: throw "message"; so that you have a stack trace.',
            'Stack trace up to this point:\n' + (new Error('creating stack')).stack,
            'To silences these messages, do: window.sendUncaughtException.allowPrimitives = true'
          ].join('\n\n'));
        }
      }
    }
    // 

[niemyjski]

@acthp, I'm helping maintain TraceKit upstream.. If you want to recreate this issue in the master (https://github.com/csnover/TraceKit) That would be greatly appreciated.