Closed Bug 1456907 Opened 3 years ago Closed 3 years ago
Wrong require path from browser loader modules are hard to debug
59 bytes, text/x-review-board-request
Let's say you make a mistake in this require path here: https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/RequestList.js#8 const dom = require("devtools/client/shared/vendor/react-dom-factories-ERROR"); This RequestList.js module is loaded via the browser-loader and it will ends up printing this on stdout: console.error: (new Error("Module `resource://devtools/client/shared/vendor/react-dom-factoriesxxx.js` is not found at resource://devtools/client/shared/vendor/react-dom-factories-ERROR.js", "resource://devtools/client/shared/browser-loader.js", 143)) The issues comes from multiple factors: * The exception is reported via console.error, from here: https://searchfox.org/mozilla-central/rev/36dec78aecc40539ecc8d78e91612e38810f963c/devtools/client/framework/toolbox.js#1711 But console.error won't print the stack trace of exception, per this c++ code: https://searchfox.org/mozilla-central/rev/36dec78aecc40539ecc8d78e91612e38810f963c/dom/console/Console.cpp#2874 `console.error(new Error("foo"))` will dump to stdout only `new Error("foo").toSource()`, which is what we see on stdout. The exception message and the first frame. * base-loader doesn't filter out browser-loader frames. When it catches an exception, it filters out all frames relatif to itself, but not the one related to browser-loader.
With the attached patch, you get a more actionnable message: console.error: (new Error("Module `resource://devtools/client/shared/vendor/react-dom-factoriesxxx.js` is not found at resource://devtools/client/shared/vendor/react-dom-factoriesxxx.js", "resource://devtools/client/netmonitor/src/components/RequestList.js", 8)) But that makes me wonder if we should keep using `console.error` and prefer DevToolsUtils.reportException instead??
Comment on attachment 8970933 [details] Bug 1456907 - Report correct file name of modules requiring modules with a wrong path. https://reviewboard.mozilla.org/r/239690/#review245472 Thanks! I don't have any particular feelings about the logging style... Feel free to add what makes sense. Maybe `console.trace()` is helpful for this?
Attachment #8970933 - Flags: review?(jryans) → review+
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #3) > Comment on attachment 8970933 [details] > Bug 1456907 - Report correct file name of modules requiring modules with a > wrong path. > > https://reviewboard.mozilla.org/r/239690/#review245472 > > Thanks! > > I don't have any particular feelings about the logging style... Feel free > to add what makes sense. Maybe `console.trace()` is helpful for this? From toolbox.js, console.trace would log the trace of the current stack (the one to call Toolbox.loadTool), whereas we are more interested into the stack of the exception we just catch. What we could do from toolbox.js is: - }, console.error); + }, e => console.error(e, e.stack)); or: - }, console.error); + }, e => DevToolsUtils.reportException("loadTool", e));
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/3acc17739c22 Report correct file name of modules requiring modules with a wrong path. r=jryans
You need to log in before you can comment on or make changes to this bug.