Bug (carlsmith) – Allowing eval'ed code to be named in stack traces

• Component: WebCore JavaScript
• Status: NEW

[Carl Smith]

Following a discussion on es-discuss [http://esdiscuss.org/topic/maximally-minimal-stack-trace-standardization], Filip Pizlo asked me to bring this here. That discussion is about improving stack traces generally long term, but this ticket is to do with some specifics that can be dealt with sooner.

In applications that allow the user to enter and execute code ~ shells, interactive educational environments, scriptable applications ~ the app needs certain information on errors to provide tracebacks to the user, basically where the error occurred, which amounts to the filename, the line number and the column number for each call on the stack.

Because these applications use `eval` to execute the users' inputs, often repeatedly, there needs to be a way to name the eval'ed code strings, else they are not distinct in the traceback, and any line and column numbers are then useless.

V8 now allows applications to use the source URL directive `// #sourceURL=<filename>` in eval'ed code strings, and will use the given name in tracebacks [instead of eval, eval, eval...].

For an example of how this is useful, in a shell I work on, each user input gets internally given an incremented name like input1.js, and the input string and any meta is stashed for later. If any of the user's inputs end up in a stack trace [not uncommon] the app can grab the information about the original input from the stash using the 'filename'. This allows for source maps too, so you can have languages like CoffeeScript mixed into the stack trace and still produce a consistent traceback, with the original source code, and line and column numbers mapped correctly. It works really nicely, but only in Chrome/Opera.

Different engines currently vary in what they do regarding line and column numbers too. V8 does line and column numbers on runtime errors, but not syntax/compilation errors, though I'm told that's now fixed, but not stable yet. Gecko does line and column numbers for both kinds of error, but doesn't expose the source URL in tracebacks provided to apps. A couple of Moz guys seem at least open to following Chrome on that now, and making it a de-facto standard.

TL;DR: All errors should pass `window.onerror` an `error` object with a `stack` that includes a filename ~ either the actual filename or a given filename if it's provided as a source URL and the item is eval'ed code ~ and the line and column numbers, for each item in the stack.

[Rasmus Porsager]

Would love to know if this is something that could get priority soon? Also if I'd want to get started contributing to webkit implementing a thing like this, where would you recommend that I start looking?

[Saam Barati]

(In reply to Rasmus Porsager from comment #1)
> Would love to know if this is something that could get priority soon? Also
> if I'd want to get started contributing to webkit implementing a thing like
> this, where would you recommend that I start looking?

Is this implemented in other browsers? If so, what does the JS syntax look like?

[Rasmus Porsager]

Created attachment 314554 [details]
A short example to show stack traces using sourceURL

[Rasmus Porsager]

Yes.

This is implemented in Firefox & Chrome.

You can visit this url or check out attached index.html in each browser to see stack traces that include the sourceURL.
https://porsager.com/shared/sourceurlstack/

Safari doesn't support //# sourceURL= in script tags either. Would you recommend that I create another bug for that?

[Joseph Pecoraro]

> Safari doesn't support //# sourceURL= in script tags either. Would you
> recommend that I create another bug for that?

Safari supports sourceURL in <script>, Function, and eval for locations logged to Web Inspector. For example a `console.trace()` or uncaught exception case you will see the source URL location:

Global Code (this is missing sourceURL):

    [Trace] Trace
    	Global Code (test-page.html:25)
    [Error] Error
    	Global Code (test-page.html:26)


Function: (sourceURL shows up for the anonymous function)

    [Trace] Trace
    	anonymous (namedFunction.js:3)
    	Global Code (test-page.html:36:202)
    [Error] Error
    	anonymous (namedFunction.js:4)
    	Global Code (test-page.html:36:194)

eval: (sourceURL shows up for the eval code)

    [Trace] Trace
    	Eval Code (namedEval.js:2)
    	eval
    	Global Code (test-page.html:39)
    [Error] Error
    	Eval Code (namedEval.js:3)
    	eval
    	Global Code (test-page.html:39)

JavaScriptCore just doesn't use those locations by default in its Error.stack reporting.

It looks like that comes from Interpreter::stackTraceAsString which ultimately gets the ScriptExecutable's sourceURL which is the SourceProvider's url, not the SourceProvider's sourceURL. I think it would be reasonable to use the best name available in all error stacks.

So there are two issues here:

    1. sourceURL directive should show up in Error.stack.
    2. sourceURL directive in <script> global code wasn't displayed properly in Web Inspector.

[Joel Einbinder]

Created attachment 411571 [details]
Patch

[Joel Einbinder]

I have a patch that fixes this. It uses //# sourceURL comments to annotate error stack traces. As discussed in 2017, this matches the behavior in Firefox and Chromium.

[Joel Einbinder]

Created attachment 411597 [details]
Patch

[Devin Rousso]

Comment on attachment 411597 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411597&action=review

> Source/JavaScriptCore/runtime/StackFrame.cpp:70
> +    String sourceURLDirective = m_codeBlock->ownerExecutable()->source().provider()->sourceURLDirective();

Are we sure that `provider()` exists?

> LayoutTests/inspector/model/remote-object/error-expected.txt:34
> +        "_name": "sourceURL",
> +        "_type": "string",
> +        "_value": "__WebInspectorInternal__"

This slightly concerns me, as it exposes the fact that the evaluation came from Web Inspector.

> LayoutTests/inspector/model/remote-object/error-expected.txt:-117
> -        "_name": "name",
> -        "_type": "string",
> -        "_value": "IndexSizeError"

NOTE: I think this disappeared because the preview only shows five properties

[Joel Einbinder]

(In reply to Devin Rousso from comment #9)
> Comment on attachment 411597 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=411597&action=review
> 
> > Source/JavaScriptCore/runtime/StackFrame.cpp:70
> > +    String sourceURLDirective = m_codeBlock->ownerExecutable()->source().provider()->sourceURLDirective();
> 
> Are we sure that `provider()` exists?

A few lines later, ownerExecutable()->sourceURL(), calls provider()->sourceURL() internally. I can add sourceURLDirective to ScriptExecutable to make it look less sketchy. I guess I was cutting corners in pursuit of a smaller patch.

> > LayoutTests/inspector/model/remote-object/error-expected.txt:34
> > +        "_name": "sourceURL",
> > +        "_type": "string",
> > +        "_value": "__WebInspectorInternal__"
> 
> This slightly concerns me, as it exposes the fact that the evaluation came
> from Web Inspector.

I totally agree that it's concerning. However I think it just makes it more clear what's going on. The stack trace from the console is already highly detectable. It looks like

global code
evaluateWithScopeExtension@[native code]

_wrapCall

Fixing this requires stripping the stack from the console evaluations. And even then, the lack of a stack could be detected. For comparison Firefox's stack is

@debugger eval code:1:1

Whereas Chromium's is

Error
    at <anonymous>:1:1

[Sam Verschueren]

Hey everyone! I'm a developer at StackBlitz (https://stackblitz.com) and we're doing our best to support WebContainers (https://blog.stackblitz.com/posts/introducing-webcontainers/) on all browser engines. Basically allowing you to run Node.js in the browser.

We're now running into an issue where we need the information out of a stack trace, which comes from eval'd code.

A small example looks like this

```
eval(`
    function bar() {
        console.log(new Error().stack);
    }

    bar();

    //# sourceURL=foo-bar.js
`)
```

The stack trace currently logs the following

```
bar@
eval code@
eval@[native code]
module code@http://localhost:3000/foo.js:1:5
```

While it should be

```
[e-mail]:3:21
eval [e-mail]:6:5
eval@[native code]
module code@http://localhost:3000/foo.js:1:5
```

In Firefox and Chrome this works perfectly fine. It would be great if this just worked in Safari as well.