Initial report on Bug 1289570, comment 7 by Cesar Izurieta (pasted verbatim here). But unfortunately I cannot make it work. I'm using Firefox 50 and enabled 'devtools.sourcemap.locations.enabled' in 'about:config'. I restarted Firefox and opened a Ember app. When I get an error I just see a backtrace in the 'Call Stack' tab of the error with lines like this: > .send http://localhost/assets/vendor.js:19333:6 > .ajax http://localhost/assets/vendor.js:18814:5 > makeRequest http://localhost/assets/vendor.js:124613:14 > authenticate/< http://localhost/assets/vendor.js:124532:9 > initializePromise http://localhost/assets/vendor.js:70977:7 Some other errors don't show the tabbed error but just a backtrace directly. I do have a 'http://localhost/assets/vendor.map'. When I click on the links in any of those cases a new tab opens with a "source:" URL of the file. To contrast that, chromium shows the following stack trace: > send @ jquery.js:9175 > ajax @ jquery.js:8656 > makeRequest @ oauth2-password-grant.js:245 > (anonymous) @ oauth2-password-grant.js:164 > initializePromise @ ember.debug.js:51004

I have a good understanding of what needs to be done to resolve this. If it is okay I can take it and maybe ask Jarda to mentor me on this. Brian could you let me know if I should go ahead with this, or is there something conflicting in the new-console-frontend work/timeline. Thanks.

> I have a good understanding of what needs to be done to resolve this. Cool ! IMO you can go and work on this, I don't see any conflict with what we're doing on the console ATM. Thanks !

I have an initial patch which seems to be doing the trick. I wanted to get feedback though, since it has been some time since I last touched these parts of the code.

Comment on attachment 8811237 [details] [diff] [review] 1317962.patch Review of attachment 8811237 [details] [diff] [review]: ----------------------------------------------------------------- Only a couple nits I saw. I can't see to which component you're passing sourceMapService , so Jarda would know better if this is the correct fix. IIUC, the bug is only on the old console right, since we do already pass sourceMapService to the FrameView in the new one ( http://searchfox.org/mozilla-central/source/devtools/client/webconsole/new-console-output/components/message.js#138 ) ? ::: devtools/client/shared/components/stack-trace.js @@ +38,4 @@ > }, > > render() { > + let { stacktrace, onViewSourceInDebugger, nit: put one prop per line let { stacktrace, onViewSourceInDebugger, sourceMapService, } = this.props; @@ +59,5 @@ > showFunctionName: true, > showAnonymousFunctionName: true, > showFullSourceUrl: true, > + onClick: onViewSourceInDebugger, > + sourceMapService nit: put a trailing comma here to make the next diff only show the added property

Thanks Nicolas. I'll fix these nits soon. I noticed that code in new-console-frontend. and I did think the stack-trace is sourcemapped there, AFAIK. So this bug is primarily for the old/current console, I believe, specifically since that code doesn't handle it. I basically passed sourceMapService to StackTrace to Frame Component and let it take care of the rest. I ran it on a tiny test, but I'll try it against a proper example after class.

Comment on attachment 8811237 [details] [diff] [review] 1317962.patch Yes, this looks good. Please write a mochitest, and consider adding sourcemapping also to the netlogging stacktraces (i.e., the expanded view of XHR requests logged in console). It should also be a matter of adding one prop at the right place. However, I don't like overall architecture of the sourcemapping very much: - the "sourceMapService" prop is passed down over many intermediate components, adding noise to the code. Using a React context attribute would be a great use case here - the stacktrace component should be stateless ideally, and just render the props. The sourcemap resolution should be handled somewhere else. Over time, it will be a Redux store everywhere. The current implementation is not very Redux friendly But all that certainly doesn't belong in this bug. Also, I heard that sourcemapping will be totally rearchitected soon, moved from server to the client. I don't know any details about that.

(In reply to Jaideep Bhoosreddy [:jbhoosreddy] from comment #5) > Thanks Nicolas. I'll fix these nits soon. I noticed that code in > new-console-frontend. and I did think the stack-trace is sourcemapped there, > AFAIK. So this bug is primarily for the old/current console, I believe, > specifically since that code doesn't handle it. AFAIK, neither console frontend will support source maps with the new debugger frontend enabled. The new debugger frontend uses sourcemaps on the client and I think console support will rely on that behavior being ported to the toolbox: https://github.com/devtools-html/debugger.html/issues/1220. If this is only adding support for the old debugger (is it?) then we shouldn't bother, since the new one is riding the train in 52.

(In reply to Brian Grinstead [:bgrins] from comment #7) > AFAIK, neither console frontend will support source maps with the new > debugger frontend enabled. The new debugger frontend uses sourcemaps on the > client and I think console support will rely on that behavior being ported > to the toolbox: https://github.com/devtools-html/debugger.html/issues/1220. > > If this is only adding support for the old debugger (is it?) then we > shouldn't bother, since the new one is riding the train in 52. Do I understand correctly that the new debugger will be turned on in default in 52, and that the console sourcemapping, announced with great fanfare in 50, will stop working? That doesn't sound very encouraging.

Comment on attachment 8811237 [details] [diff] [review] 1317962.patch Review of attachment 8811237 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/webconsole/console-output.js @@ +3564,5 @@ > if (this.stacktrace) { > this.output.owner.ReactDOM.render(this.output.owner.StackTraceView({ > stacktrace: this.stacktrace, > + onViewSourceInDebugger: frame => this.output.openLocationInDebugger(frame), > + sourceMapService: toolbox ? toolbox._sourceMapService : null, This needs to be fixed on the new frontend as well: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/new-console-output/components/message.js#112.

(In reply to Jarda Snajdr [:jsnajdr] from comment #8) > (In reply to Brian Grinstead [:bgrins] from comment #7) > > AFAIK, neither console frontend will support source maps with the new > > debugger frontend enabled. The new debugger frontend uses sourcemaps on the > > client and I think console support will rely on that behavior being ported > > to the toolbox: https://github.com/devtools-html/debugger.html/issues/1220. > > > > If this is only adding support for the old debugger (is it?) then we > > shouldn't bother, since the new one is riding the train in 52. > > Do I understand correctly that the new debugger will be turned on in default > in 52, and that the console sourcemapping, announced with great fanfare in > 50, will stop working? That doesn't sound very encouraging. Yes, that's right. Although the new sourcemapping has always been off-by-default IIRC. Once https://github.com/devtools-html/debugger.html/issues/1220 is resolved then source mapping should be switched to the client side transparently from consumers of the service, and hopefully we can pref on by default.

Prioritizing for the new frontend (which will have source maps working once the James' work lands)

Have had problems enabling source maps in web console for a Meteorjs app (understand this is for Ember, but may be similar issue?).

I'm taking this as part of the source map work. Jaideep, if you were hoping to continue working on this, contact me and I'll send over what I have.

I thought it was blocked on another bug. So I didn't work on it. But please go ahead since it'll take me some time to get back into it. And it's not that big of a change. :-D

Comment on attachment 8864265 [details] Bug 1317962 - use source maps in stack traces in the console; https://reviewboard.mozilla.org/r/135900/#review138990 Great, this looks good to me! :) Thanks for adding a test.

Pushed by ttromey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8ff81b6fdfb8 use source maps in stack traces in the console; r=jryans

Backed out in https://hg.mozilla.org/integration/autoland/rev/e65f43338f50 for frequently failing like https://treeherder.mozilla.org/logviewer.html#?job_id=96766616&repo=autoland on Windows. Inconveniently for Try, it's easiest to repro with a PGO build, maybe 50%, but even just opt will fail around 5%, if you don't want to PGO.

A few runs without PGO: https://treeherder.mozilla.org/#/jobs?repo=try&revision=628089f2d7a113d1b0dd9f7c836ab39dbf15211f Now some with PGO: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9e3229d28a7f02dbd8ab5acfa20283eaeeb5f8e I'm hoping I did these correctly. Still no failures.

The previous "pgo" run seems not to have included my patch. Not sure why that was, but I re-ran, definitely including the patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6aeec281d616faaba94cecd1fce0acfeae6fa447 So far I can't reproduce the failure.

I failed to run the job with e10s. KWierso did that (thanks) and now there are failures galore.

After several try pushes with extra logging I think I've discovered the problem. This code: https://dxr.mozilla.org/mozilla-central/rev/85e5d15c31691c89b82d6068c26260416493071f/devtools/client/framework/source-map-url-service.js#86 assumes that the source map worker will return the same object when the location can't be source mapped. However, it seems to me that there's no reason to believe that object identity will be preserved here. I still don't understand why this only fails in one particular environment, but changing this to check property values seems to fix the bug. https://treeherder.mozilla.org/#/jobs?repo=try&revision=7f2185dc88d60a4650e37256cd37947fa18b954f

Pushed by ttromey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/38a29dcf34c8 use source maps in stack traces in the console; r=jryans