In bug 1345533, I'm changing the source map service to report source map errors to the user on the web console. However, I think at least failures to fetch the original source will still not be properly reported -- that requires another wrapper.

This is going to need a small tweak in the debugger, because right now a missing source causes an uncaught promise rejection, causing the new test to fail.

Comment on attachment 8893957 [details] Bug 1387477 - report source map errors due to missing sources; https://reviewboard.mozilla.org/r/165020/#review170700 Thanks for the patch Tom! Have a look at my suggestion about the Proxy, and let me know if you want to discuss that. You might have already agreed with debugger folks that this was the right approach, but I prefer to check. ::: devtools/client/framework/toolbox.js:572 (Diff revision 1) > + .catch(text => { > + let message = L10N.getFormatStr("toolbox.sourceMapSourceFailure", > + text, originalSource.url); > + this.target.logErrorInPage(message, "source map"); > + // Also replace the result with the error text. > + return { It feel it would be better to let the consumer (here the debugger) deal with the failure in order to display whatever they want in their UI. Here we swallow the exception and the debugger has no way to act upon this. What about adding the catch, but still returning the original promise? > let p = target.getOriginalSourceText(originalSource); > p.catch(/** catch code **/); > return p; Otherwise we need to add comments here to say that {text, contentType} should remain in sync with the return value structure of getOriginalSourceText on GitHub, and vice versa. On a side node, maybe we should apply the same treatment to the other Proxy for getOriginalURLs, which is swallowing exceptions as well at the moment. ::: devtools/client/locales/en-US/toolbox.properties:188 (Diff revision 1) > # The text of the error: %1$S > # The URL that caused DevTools to try to fetch a source map: %2$S > # The URL of the source map itself: %3$S > toolbox.sourceMapFailure=Source map error: %1$S\nResource URL: %2$S\nSource Map URL: %3$S > + > +# LOCALIZATION NOTE (toolbox.sourceMapSoruceFailure): This is shown in sourceMapSo[ru]ceFailure > sourceMapSo[ur]ceFailure ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_sourcemap_nosource.js:40 (Diff revision 1) > + yield testOpenInDebugger(hud, toolbox, "here"); > + > + info("Selecting the console again"); > + yield toolbox.selectTool("webconsole"); > + > + const node2 = yield waitFor(() => findMessage(hud, "original source")); nit: there is no node1, node2 -> node? ::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:208 (Diff revision 1) > }); > observer.observe(node, observeConfig); > }); > } > + > +function* testOpenInDebugger(hud, toolbox, text) { Small JSDoc here, mainly to explain that text will be used to identify the message that is supposed to contain the debugger link. ::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:209 (Diff revision 1) > observer.observe(node, observeConfig); > }); > } > + > +function* testOpenInDebugger(hud, toolbox, text) { > + info(`Testing message with text "${text}"`); Would be nice to have a general info statement that explains that this will assess clicking on a link that should open the debugger.

Comment on attachment 8893957 [details] Bug 1387477 - report source map errors due to missing sources; https://reviewboard.mozilla.org/r/165020/#review170700 > It feel it would be better to let the consumer (here the debugger) deal with the failure in order to display whatever they want in their UI. > > Here we swallow the exception and the debugger has no way to act upon this. What about adding the catch, but still returning the original promise? > > > let p = target.getOriginalSourceText(originalSource); > > p.catch(/** catch code **/); > > return p; > > Otherwise we need to add comments here to say that {text, contentType} should remain in sync with the return value structure of getOriginalSourceText on GitHub, and vice versa. > > On a side node, maybe we should apply the same treatment to the other Proxy for getOriginalURLs, which is swallowing exceptions as well at the moment. In this case, propagating the failure will cause the new test to fail, because the debugger doesn't check for this. This doesn't matter in the debugger, because it doesn't have a test for failed source fetching, and because it doesn't have the mochitest behavior of automatic failure on an uncaught rejected promise. I brought this up on #debugger on slack and Jason said: > up to you. I don’t care about showing the error at the moment So I went forward with this approach because it is simpler and because it puts all the source map error handling in one spot. I will add some comments. As for getOriginalURLs, it's always ok to swallow exceptions there, because a null result means no source map was found.

Comment on attachment 8893957 [details] Bug 1387477 - report source map errors due to missing sources; https://reviewboard.mozilla.org/r/165020/#review170778 Thanks for the update.

I think the test just needed to wait for the source map to be applied. I'll upload the new version shortly. Try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=95d5606107451c30adb31c9fa51eb5b3d72868f3

Pushed by ttromey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eb95d04c5948 report source map errors due to missing sources; r=jdescottes