Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StackTrace not included in Electron (Tracekit update resolves this) #201

Closed
McPo opened this issue Nov 10, 2016 · 4 comments
Closed

StackTrace not included in Electron (Tracekit update resolves this) #201

McPo opened this issue Nov 10, 2016 · 4 comments

Comments

@McPo
Copy link

McPo commented Nov 10, 2016

Currently developing a React App in Electron 1.2.3.
Ive noticed that Raygun is not reporting the stacktrace, while the correct stacktrace is printed out in the Chrome console.

Ive noticed by changing the following Im capable of obtaining the stack trace. (Im printing the stack trace out using onBeforeSend and am using the v1 API).

  function traceKitWindowOnError(message, url, lineNo, columnNo, errorObj) {
        var stack = null;

        if (errorObj) {
          stack = TraceKit.computeStackTrace(errorObj);
        }
        else

to

  function traceKitWindowOnError(message, url, lineNo, columnNo, errorObj) {
        var stack = null;

        console.log('Error object', errorObj);
        if (false) {
          stack = TraceKit.computeStackTrace(errorObj);
        }
        else

Basically skipping that case allows the correct stacktrace to be constructed.
To further confuse the matter, printing out errorObj shows the correct stacktrace.

So it is there but it appears TraceKit.computeStackTrace drops the stack trace.

Ive tried a++; (a is undefined) and throw Error('test error'); to generate the error. Both results are the same.

Any help much appreciated, Ill continue investigating.

@McPo
Copy link
Author

McPo commented Nov 10, 2016

So looking at the computeStackTrace method.

computeStackTraceFromStacktraceProp and computeStackTraceFromStackProp
Both raise an error, when calling processUnhandledException.

Uncaught TypeError: Cannot read property 'stack' of null

In this snippet of code.

if (stackTrace.stack && stackTrace.stack.length) {
            forEach(stackTrace.stack, function (i, frame) {
                stack.push({

The error contains a .stack property but not a .stacktrace property.

I notice computeStackTraceFromStackProp protects against the case of not having a .stack property while computeStackTraceFromStackTraceProp does not, and computeStackTraceFromStackTraceProp is called first.

So it look like computeStackTraceFromStackProp is where the problem lies. Although computeStackTraceFromStackTraceProp should check if stacktrace is null, otherwise the behaviour is gonna vary if 'debug' mode is on. (As the exception will be re-thrown in debug mode).

@McPo
Copy link
Author

McPo commented Nov 10, 2016

So the issue appears to be caused by an outdated regex for chrome within the computeStackTraceFromStackProp method.

I changed

/^\s*at (.*?) ?\(?((?:file|http|https|chrome-extension):.*?):(\d+)(?::(\d+))?\)?\s*$/i

to

/^\s*at (.*?) ?\(((?:file|https?|blob|chrome-extension|native|webpack|eval).*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i,

As per the latest Tracekit release https://github.com/csnover/TraceKit/blob/cf5fd10fccde8b4e29586d4f2dba15ba18c72c95/tracekit.js

Would it be possible to update Tracekit to resolve this issue. Im not sure if this is an electron specific issue but considering there is a blog post regarding electron support it would be nice if it was supported. https://raygun.com/blog/2015/10/nodejs-desktop-apps-and-tracking-those-elusive-offline-errors/

Also I know theres several issue here regarding updating Tracekit, would you reconsider not using a custom build?

@McPo McPo changed the title StackTrace not included in Electron StackTrace not included in Electron (Tracekit update resolves this) Nov 10, 2016
@fundead
Copy link
Contributor

fundead commented Nov 10, 2016

Hi Emmet,

Thanks for raising this issue. We were forced to vendor Tracekit early on as that library was abandoned by the creator a couple of years ago and maintain our own fork, since then as you note it has been picked up by another maintainer and we do have plans to update our dependency to that version. Due to the need to not break backwards compatibility this requires QA as our fork has extra fixes which weren't available while Tracekit was unmaintained, and we can't have regressions with regards to these.

There is also a further outstanding issue regarding Tracekit where it causes a spurious warning due to a sychronous XHR call, as analysed by Chrome (even though it isn't called). Due to this we will maintain our own fork but will roll in the updated regexes for you in the near term.

@McPo
Copy link
Author

McPo commented Nov 11, 2016

Thanks @fundead for the detail.

Also today I wiped out dependencies and attempted to apply the same fix and for whichever reason it didnt resolve the issue this time. It appears that whatever way Im throwing errors now, the filename in the stack trace isn't prefixed with "file://".

Therefore instead of using the following regex

/^\s*at (.*?) ?\(((?:file|https?|blob|chrome-extension|native|webpack|eval).*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i

I had to use

/^\s*at (.*?) ?\(((?:file|https?|\s*|blob|chrome-extension|native|webpack|eval).*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i

I believe the change is pretty safe, but Im gonna raise this with tracekit as well. occ/TraceKit#74

I also ran into an issue in which the stacktrace begun with

(native)

I was throwing the error as Error('msg'), however when I started to throw errors as new Error('msg'), the 'native' part got dropped.

Also using throw window.Error('msg') and throw new window.Error('msg'), results in a nonsense stacktrace. I believe this is caused by a different in Node and Chrome Error objects, within Electron apps.

Again Im not sure why my errors suddenly dropped the file:// prefix but they did, I even tried checking out a previous commit, but the result was the same. Modifying the regex resolved the issue.

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

No branches or pull requests

2 participants