myk noted in https://bugzilla.mozilla.org/show_bug.cgi?id=654095#c21 that the messages emitted by the SDK when a require() statement cannot be resolved should be nicer. The link-time (python) code ought to give a clearer message and mention a list of places where it tried to find the missing module. The runtime code should throw an exception with enough information to point the developer at the verbose link-time message. Most of these errors are likely to be caused by simple typos, so they need to be gentle and informative.

The code landed in https://github.com/mozilla/addon-sdk/commit/658fb89ca4d3dc4a8cfb9616ae873e979a7f3955 includes an improvement for this, maybe enough to close this. Now errors look like this: (jetpack-sdk-git)362:warner@Cookies% cfx xpi Traceback (most recent call last): File "/Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/bin/cfx", line 29, in <module> cuddlefish.run() File "/Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/python-lib/cuddlefish/__init__.py", line 640, in run manifest = build_manifest(target_cfg, pkg_cfg, deps, uri_prefix, False) File "/Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/python-lib/cuddlefish/manifest.py", line 541, in build_manifest mxt.build(scan_tests) File "/Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/python-lib/cuddlefish/manifest.py", line 189, in build self.top_uri = self.process_module(self.find_top(self.target_cfg)) File "/Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/python-lib/cuddlefish/manifest.py", line 354, in process_module raise err cuddlefish.manifest.ModuleNotFoundError: ModuleNotFoundError: unable to satisfy require(unknown) from /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/examples/reading-data/lib/main.js . Looked for it in: /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/examples/reading-data/lib/unknown.js /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/packages/addon-kit/lib/unknown.js /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/packages/api-utils/lib/unknown.js /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/examples/reading-data/lib/unknown.js What'dya think?

(automatic reprioritization of 1.0 bugs)

I'd like to think this is good enough for 1.1, but if we do want to improve it further, here's what I'd consider: * include the line number of the require() statement * don't show the Python traceback * add a few sentences of suggestions: * maybe "unknown" is a misspelling? * did you forget to use --package-path to include a required package? * did you really want to include this module anyways? The latter two items are trivial. Getting the line number will take a bit more work: that information has been discarded by the time the module-search code is executed, so I'll need to find a place to hang on to it.

(In reply to Brian Warner [:warner :bwarner] from comment #1) > What'dya think? That's definitely better, although my eyes glaze over when they hit the traceback, and it's an effort to find the error message that succeeds it. Better would be to do something like this, if possible: ModuleNotFoundError: unable to satisfy require(unknown) from /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/examples/reading-data/lib/main.js. Looked for it in: /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/examples/reading-data/lib/unknown.js /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/packages/addon-kit/lib/unknown.js /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/packages/api-utils/lib/unknown.js /Users/warner/stuff/mozilla/jetpack/jetpack-sdk-git/examples/reading-data/lib/unknown.js If the traceback can't (or shouldn't) be suppressed, can we make it appear after the error message, with a blank line between them (or at least put a blank line between them, if the traceback has to appear first)?

Ok, I've got a patch which supresses the traceback and produces the following message: % cfx xpi ModuleNotFoundError: unable to satisfy: require(missing) from /Users/warner/stuff/mozilla/jetpack/my-addons/minimal/lib/main.js:13: Looked for it in: /Users/warner/stuff/mozilla/jetpack/my-addons/minimal/lib/missing.js /Users/warner/stuff/mozilla/jetpack/jp-git/packages/api-utils/lib/missing.js The second line contains a trick: by announcing the location of the problem with that "FILENAME:LINENO:" format (the same as GCC emits), tools like emacs (and other IDEs) can parse it and give you a button to jump directly to the offending line. How's that look?

Comment on attachment 558913 [details] [diff] [review] improve the "module not found" error message (In reply to Brian Warner [:warner :bwarner] from comment #5) > Ok, I've got a patch which supresses the traceback and produces the following > message: > > % cfx xpi > ModuleNotFoundError: unable to satisfy: require(missing) from > /Users/warner/stuff/mozilla/jetpack/my-addons/minimal/lib/main.js:13: > Looked for it in: > /Users/warner/stuff/mozilla/jetpack/my-addons/minimal/lib/missing.js > > /Users/warner/stuff/mozilla/jetpack/jp-git/packages/api-utils/lib/missing.js > > The second line contains a trick: by announcing the location of the problem > with that "FILENAME:LINENO:" format (the same as GCC emits), tools like emacs > (and other IDEs) can parse it and give you a button to jump directly to the > offending line. > > How's that look? Looks great! Minor formatting nits are to add a second space before the reference to the file/line on which the require is located and remove the space from the "Looked for it in:" line, i.e.: ModuleNotFoundError: unable to satisfy: require(missing) from /Users/myk/Dropbox/addon-sdk/foo/lib/main.js:2: Looked for it in: /Users/myk/Dropbox/addon-sdk/foo/lib/missing.js /Users/myk/Dropbox/addon-sdk/packages/addon-kit/lib/missing.js /Users/myk/Dropbox/addon-sdk/packages/api-utils/lib/missing.js /Users/myk/Dropbox/addon-sdk/foo/lib/missing.js The final nit is that it isn't clear why most lists added by the patch are padded with spaces after commas, but a couple aren't, namely: >+ for lineno0,line in enumerate(lines): And: >- requires,problems = scan_module(fn, open(fn).readlines()) >+ requires,problems,locations = scan_module(fn, open(fn).readlines()) I defer to you on Python code style, and the latter change replaces a non-padded list, so perhaps you were maintaining existing code style there (but other lists in the vicinity of that change are padded, so it seems like that one might as well be made padded too in the process).

Landed, in https://github.com/mozilla/addon-sdk/commit/922ee524ac551afd5bb3991073dcf0c2bea44f85 , with your suggestions applied. Yeah, I'm somewhat inconsistent on list padding. In Python the parenthesis are usually optional, and sometimes I remove the spaces to keep those symbols visually bound together. But if there's more than two elements in the list then it starts to look kind of weird. Also, sometimes I omit spaces to keep the overall line from wrapping or falling off the right-hand margin.