Better stack trace parsing in the browser #1097
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Note that runtime.Stack() and, by extension, runtime/debug.Stack() do not currently do stack trace parsing/formatting. This means that panic stack traces printed to the console will remain as-is.
This is not a bad thing (I personally prefer to have the original stack trace with more breadcrumbs), just making sure it matches your expectations.
I agree. In fact, if it were possible to encapsulate the original stack trace in the return from |
This comment has been minimized.
This comment has been minimized.
|
Reference app: jQuery TodoMVC (
#outputSize |
|
@nevkontakte Care to take another look at the updates before I merge? |
|
LGTM, thanks for making the changes! |
This reimplements and expands on #658.
Getting these tests to work was a bit of a challenge, and requires an ugly hack. We don't run the standard tests against the
runtimepackage, since the majority of that functionality in upstream is irrelevant to us. So adding tests to that package, where this new functionality lives, is meaningless by default.Getting those tests to work is no small task, due to many unavailable symbols, etc. And making those symbols available is no small task, because the GopherJS compiler has several special cases surrounding the
runtimepackage that prevent our overlays from being used, etc.To work around this, we run the new tests with
gopherjs test ./compiler/natives/src/runtime, which bypasses the checks for theruntimepackage. But it does mean we need to add one more special case forruntime, and stop checking its imports list against upstream.In the grand scheme of things, adding one more exception to something that's already a huge special case didn't seem like that big of a deal, but maybe others disagree.
On to the functionality... This is more robust than #658 (one great reason to uses tests, to make that easy!) My browser (Chrome) actually outputs multiple formats of stack trace lines, so this new implementation tries to parse all of them.
Some of the formats omit the function name and/or file name, which I've indicated with
<none>and<anonymous>in the response fromruntime.Caller(). I'm not sure if that's actually a good idea, but it seemed more descriptive than simply returning the empty string, which is the only alternative that occurred to me.With this PR merged, it should be trivial to add new stack traces (i.e. from Safari, Mozilla, wherever) and extend the parsing functionality to be more robust.