1192882 - Logged errors don't respect //# sourceURL=foo.js pragmas

Status: RESOLVED WONTFIX

(DevTools :: Console, defect, P3)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Testcase: http://htmlpad.org/console-source-url/

Note that `console.log(foo)` does respect //# sourceURL but uncaught errors that get logged to the console do not.

Comment 1

[valdrinkoshi]

Gentle ping, FF 51 would still fail to show the sourceURL file when the inlined script throws or logs and error, e.g.
<script>
var s;
s = document.createElement('script');
s.textContent = `x.y.z = a.b.c;\n//# sourceURL=script-0.js\n`;
document.body.appendChild(s);

s = document.createElement('script');
s.textContent = `x.y.z = a.b.c;\n//# sourceURL=script-1.js\n`;
document.body.appendChild(s);
</script>

^ does show in the console that there are 2 errors, but only shows script-0 in the stack (instead of considering them as separate errors coming from 2 files).

Comment 2

[Tom Tromey :tromey]

console.log works because, ultimately, the URL for the stack entry comes from this code, which uses the
display URL when available:

https://dxr.mozilla.org/mozilla-central/rev/bdb2387396b4a74dfefb7c983733eed3625e906a/js/src/vm/SavedStacks.cpp#1570-1571

The stack trace for the error shows the display URL as well, for the same reason.

I still haven't found where the location associated with the ReferenceError line itself is computed.

Perhaps using the displayURL at the lowest level like this is maybe not the best approach, though.
This approach makes the "script-1.js" text un-clickable in the console, because the source isn't known
to the debugger - but maybe if we did client-side remapping like we do for source maps, we could show the
display name and have clicking work.

Comment 3

[Tom Tromey :tromey]

> I still haven't found where the location associated with the ReferenceError line itself is computed.

Maybe here, just based on reading code, didn't try the debugger:

https://dxr.mozilla.org/mozilla-central/rev/bdb2387396b4a74dfefb7c983733eed3625e906a/js/src/jsexn.cpp#1044-1046

Comment 4

[Tom Tromey :tromey]

The html pad link is dead for me now, but I've moved the code from comment #1 to my source
map examples repository.
https://tromey.github.io/source-map-examples/source-url/index.html

Comment 5

[Tom Tromey :tromey]

Hi Nick, Ryan.  I'm looking for some input.

I have been thinking of moving sourceURL handling fully to the client.

Because sourceURL can apparently be a relative URL (the source actor currently takes pains to make this work),
it's troublesome to have SpiderMonkey emit this value in stack traces -- because either one winds up with
a relative URL (but relative to what - when we get the stack trace, that information is long gone); or one
must reimplement URL joining in SpiderMonkey.

The alternate plan goes:

* Implement stack trace parsing in devtools-core - https://github.com/devtools-html/devtools-core/issues/452
* Change places in platform that emit displayURL to instead emit the ordinary url
* Export the displayURL on the source actor, and export the line number information
  of the debugger script instances that are seen (and, I suppose, add the column numbers,
  to handle the case where there are multiple <script>s on a line?)
* Change the source map service to handle the display URL
* Also change the debugger to handle display URL

This idea also has the benefit of working for CSS.

This is complicated enough that I wonder if I'm missing parts, or if I'm missing a simpler approach.

If relative sourceURLs were not allowed, then maybe none of this would matter.  But AFAICT they are allowed.

Maybe a simple URL joiner isn't too bad for SpiderMonkey?

I haven't looked deeply at the CSS parts yet, but I wonder if CSS can emit console warnings before
lexing is complete.  If so then we'd need something like an error message buffer to update URLs at the end.

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I think the relativity of URLs is valuable for distinguishing between logical foo.com/whatever.js and bar.com/whatever.js when both physical scripts contain `//# sourceURL=whatever.js`.

> Maybe a simple URL joiner isn't too bad for SpiderMonkey?

Callback function provided by the embedder? I don't think SpiderMonkey wants to implement this itself...

That said, moving this into the client sees fine too. That would allow people to turn off display URLs if needed for whatever reason (buggy producers?).

Comment 7

[Tom Tromey :tromey]

I had thought of an embedder callback, and didn't like it; but now it is maybe more tempting.
Not sure what changed, maybe just thinking about how it would be simpler.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Tom Tromey :tromey from comment #5)
> I have been thinking of moving sourceURL handling fully to the client.
> 
> Because sourceURL can apparently be a relative URL (the source actor
> currently takes pains to make this work),
> it's troublesome to have SpiderMonkey emit this value in stack traces --
> because either one winds up with
> a relative URL (but relative to what - when we get the stack trace, that
> information is long gone); or one
> must reimplement URL joining in SpiderMonkey.
> 
> The alternate plan goes:
> 
> * Implement stack trace parsing in devtools-core -
> https://github.com/devtools-html/devtools-core/issues/452
> * Change places in platform that emit displayURL to instead emit the
> ordinary url
> * Export the displayURL on the source actor, and export the line number
> information
>   of the debugger script instances that are seen (and, I suppose, add the
> column numbers,
>   to handle the case where there are multiple <script>s on a line?)
> * Change the source map service to handle the display URL
> * Also change the debugger to handle display URL
> 
> This idea also has the benefit of working for CSS.

I think this proposal seems reasonable to me.  It's similar to client-side source map handling, which we've deemed a better way to go these days.

It also seems beneficial that the proposal would work for CSS as well.

At the same time though, I feel a bit too removed from the problem to offer the best advice.

> I haven't looked deeply at the CSS parts yet, but I wonder if CSS can emit
> console warnings before
> lexing is complete.  If so then we'd need something like an error message
> buffer to update URLs at the end.

Does "lexing is complete" mean "sourceURL has been read" here?  For Stylo at least, all of the CSS console warnings are reported during the parsing phase, so I believe the tokenizer should have already consumed the sourceURL when errors are reported from a quick scan.

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #8)
> (In reply to Tom Tromey :tromey from comment #5)
> > I haven't looked deeply at the CSS parts yet, but I wonder if CSS can emit
> > console warnings before
> > lexing is complete.  If so then we'd need something like an error message
> > buffer to update URLs at the end.
> 
> Does "lexing is complete" mean "sourceURL has been read" here?  For Stylo at
> least, all of the CSS console warnings are reported during the parsing
> phase, so I believe the tokenizer should have already consumed the sourceURL
> when errors are reported from a quick scan.

Hmm, actually though, we do report invalid rules:

http://searchfox.org/mozilla-central/rev/d08b24e613cac8c9c5a4131452459241010701e0/servo/components/style/stylesheets/stylesheet.rs#387-407

and I believe we're consuming tokens on demand, so you might _not_ have the sourceURL yet.  Worth a deeper look, I think.

Comment 11

[Logan Smyth [:loganfsmyth]]

I'm going to close this. V8 also prints the exact sourceURL value in traces and relying on devtools to resolve the sourceURL value seems reasonable. Users are always free to set absolute 'sourceURL' values if they want absolute URLs in string traces.