TraceKit's regex for matching stacktraces in Chrome doesn't match anonymous functions

[mwcz]

In the version of TraceKit that Raven is including currently, this is the regex being used to match stacktrace entries in Chrome:

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

direct link

The part I marked there is a capture group for grabbing the name of the function. Notice that the regex as it currently is written, requires there be a least one character in the name. Anonymous functions don't have a name, so they get totally dropped from the stacktrace that gets reported to Sentry.

I dug into this when I was seeing errors in Sentry being attributed to the wrong file. Eventually I figured out it was only when the error occurred in an anonymous function that it was mis-attributed.

By changing the capture group to...

from this:  (.+?)
to this:    (.+)?

The error is fixed and all tests still pass...

TraceKit's latest version has a heavily revamped regex that does match anonymous functions, but just pulling the new version of TraceKit into Raven caused many tests to fail. :(

TraceKit also looks abandoned. No updates in over a year, several open issues and pull requests not being addressed.

Here's a link to my fix. I also ran grunt dist which looks like it pulled in a lot more changes into dist/raven.js than just mine. I guess grunt dist hasn't been run in a while. :)

So I'm fine for now, but this is an extremely confusing issue to deal with. Any thoughts on how to best handle it, @mattrobenolt ?

[mattrobenolt]

Yeah, we have a heavily modified TraceKit since we were modifying too much and it's abandoned.

Don't run grunt dist. There's a lot of floating stuff in master that needs to be addressed before cutting a new release. A lot of minor changes that touch lots of things.

With that said, can you just provide a reference stack trace so I can see exactly what it looks like? The diff itself seems reasonable. I just want to personally see what it's filtering out now.

[mattrobenolt]

Also, fwiw, our regex has been loosened up to catch more stuff than TraceKit's does. Their looks more complex, but we stripped it down so it wasn't as rigid. I'd err on the side of capturing more here.

[mwcz]

Cool, sounds good to me! Here's the stacktrace I was dealing with.

["Error: fake error",
 "    at script.js",
 "    at Object.context.execCb (lib/require.js:1663:33)",
 "    at Object.Module.check (lib/require.js:879:51)",
 "    at Object.Module.enable (lib/require.js:1156:22)",
 "    at Object.Module.init (lib/require.js:787:26)",
 "    at callGetModule (lib/require.js:1183:63)",
 "    at Object.context.completeLoad (lib/require.js:1557:21)",
 "    at HTMLScriptElement.context.onScriptLoad (lib/require.js:1684:29)"]

That's what the lines variable contained. The current regex expects the name to be there, so lines[1] doesn't get included, and the error gets attributed to require.js, when it actually occurred in script.js.

For super convenience here's a script you can paste into your console to see the difference in behavior.

// current regex
var chrome = /^\s*at (.+?) ?\(?((?:file|https?|chrome-extension):.*?):(\d+)(?::(\d+))?\)?\s*$/i
// sample stacktrace
lines = ["Error: fake error",
    "    at script.js",
    "    at Object.context.execCb (lib/require.js:1663:33)",
    "    at Object.Module.check (lib/require.js:879:51)",
    "    at Object.Module.enable (lib/require.js:1156:22)",
    "    at Object.Module.init (lib/require.js:787:26)",
    "    at callGetModule (lib/require.js:1183:63)",
    "    at Object.context.completeLoad (lib/require.js:1557:21)",
    "    at HTMLScriptElement.context.onScriptLoad (lib/require.js:1684:29)"];
chrome.exec(lines[1]); // nothing
chrome.exec(lines[2]); // works fine

// new regex
chrome = /^\s*at (.+)? ?\(?((?:file|https?|chrome-extension):.*?):(\d+)(?::(\d+))?\)?\s*$/i
chrome.exec(lines[1]); // works!
chrome.exec(lines[2]); // still works!

If you like, I can put together a PR with a test case that includes a sample stacktrace with Chrome's phrasing, like this one, and the regex fix.

[mattrobenolt]

Awesome, that makes sense to me. A PR would be appreciated. 👍

[mattrobenolt]

Hmm, in the example you posted, both the old and new fail since the regular expression assumes you'll get a fully qualified url, not just a path.

So it doesn't match lib/require.js:1663:33 unless you just redacted for the public.

BUT, does Chrome dump a full url for the at script.js? Like, will it actually be at http://example.com/script.js in the wild?

[mattrobenolt]

It still doesn't even match: at http://example.com/script.js for me.

I can look into this and match the case, but something's not adding up here. :) It sounds like you have it working somehow.

[mwcz]

Crud, I'm sorry! I "cleaned up" the URLs before posting, to hide some internal hostnames and stuff, and accidentally ruined them.

Here's an actually working example!

var lines = ["error: fake error",
 "    at https://example.com/js/script.js?v=2.0.15:13:11",
 "    at object.context.execcb (https://example.com/js/lib/require.js?version=unknown.1415632459870:1663:33)",
 "    at object.module.check (https://example.com/js/lib/require.js?version=unknown.1415632459870:879:51)",
 "    at object.module.enable (https://example.com/js/lib/require.js?version=unknown.1415632459870:1156:22)",
 "    at object.module.init (https://example.com/js/lib/require.js?version=unknown.1415632459870:787:26)",
 "    at callgetmodule (https://example.com/js/lib/require.js?version=unknown.1415632459870:1183:63)",
 "    at object.context.completeload (https://example.com/js/lib/require.js?version=unknown.1415632459870:1557:21)",
 "    at htmlscriptelement.context.onscriptload (https://example.com/js/lib/require.js?version=unknown.1415632459870:1684:29)"]

chrome = /^\s*at (.+?) ?\(?((?:file|https?|chrome-extension):.*?):(\d+)(?::(\d+))?\)?\s*$/i;
console.log(    chrome.exec(lines[1]      )); // no worky :(
console.log(    chrome.exec(lines[2]      )); // still works!

chrome = /^\s*at (.+)? ?\(?((?:file|https?|chrome-extension):.*?):(\d+)(?::(\d+))?\)?\s*$/i;
console.log(    chrome.exec(lines[1]      )); // works!
console.log(    chrome.exec(lines[2]      )); // still works!

[mattrobenolt]

Ah, ok. Yeah, that works now. :)

[mwcz]

It looks like at http://example.com/script.js didn't match because it's expecting :LINE_NUMBER at the end. at http://example.com/script.js:777 matches.

chrome.exec("    at http://example.com/script.js:777")
["    at http://example.com/script.js:777", undefined, "http://example.com/script.js", "777", undefined]

I'll start on that PR.

[mwcz]

PR opened with a test case! I did a small tweak to the regex I posted before... it's now:

(\S*)

I didn't notice earlier but in the working example I posted, the function name was being extracted as "object.context.execcb (" for example, when it should have been "object.context.execcb". \S is non-whitespace characters, so that problem's gone.

By the way @mattrobenolt I really enjoyed the video of your talk at JSConf. A buddy of mine went, and told me about it. We have our own hand-rolled client logging solution at Red Hat, but I'm working on migrating us to Sentry+Raven because it's much better.