Wrong require path from browser loader modules are hard to debug

RESOLVED FIXED in Firefox 61

Status

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: ochameau, Assigned: ochameau)

Tracking

unspecified
Firefox 61

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment)

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.
Assignee: nobody → poirot.alex
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 apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3acc17739c22
Report correct file name of modules requiring modules with a wrong path. r=jryans
https://hg.mozilla.org/mozilla-central/rev/3acc17739c22
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.