The default bug view has changed. See this FAQ.

Components.utils.import throws NS_ERROR_FILE_NOT_FOUND for components that fail to import other components

RESOLVED FIXED in mozilla12

Status

()

Core
XPConnect
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: myk, Assigned: jdm)

Tracking

(Blocks: 1 bug)

Trunk
mozilla12
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

9 years ago
When a JS component imports another component that contains a syntax error or does not exist, Components.utils.import throws NS_ERROR_FILE_NOT_FOUND for the first component, even though that component does exist.  It should throw an error that reflects the actual problem.

For example, I created two components in an extension, foo.js and bar.js.  foo.js imports bar.js, and bar.js contains a syntax error:

foo.js:
---
Components.utils.import("resource://snowl/modules/bar.js");
---

bar.js:
---
foo bar;
---

Then I made browser chrome import foo.js and started Firefox, whereupon it reported the following errors to the console:

---
Error: missing ; before statement
Source file: file:///mnt/hgfs/myk/Projects/snowl/modules/bar.js
Line: 1
Source code:
foo bar; 
 ----------
Error: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]
Source file: chrome://snowl/content/browser.js
Line: 38
---

Afterwards, I deleted bar.js and restarted Firefox, whereupon it reported the following errors to the console:

---
Error: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]
Source file: file:///mnt/hgfs/myk/Projects/snowl/modules/foo.js
Line: 1
 ----------
Error: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]
Source file: chrome://snowl/content/browser.js
Line: 38
---

In both of these cases, the initial error (the syntax error in the first case, and the FILE_NOT_FOUND error on bar.js in the second case) is correct, but the second reported error (the FILE_NOT_FOUND error on foo.js in both cases) is not correct, as foo.js does exist.

Note: this is related to bug 408412, although I think it's a different problem.
Blocks: 435025
That also happens for files that contain runtime errors, such as access to undefined properties in strict mode. Although the file is syntactically correct, a run-time error results in a NS_ERROR_FILE_NOT_FOUND.

Comment 2

6 years ago
This caused me a lot of headaches because I didn't realize there was a runtime error issue, and thought the problem was in my manifest or uri being imported.

This bug should be able to be fixed by changing the return value returned at js/src/xpconnect/loader/mozJSComponentLoader.cpp:1381 in function mozJSComponentLoader::ImportInto.  The comment above that return statement currently says "Something failed, but we don't know what it is, guess."

I suggest one of two solutions:

1. Return the error NS_ERROR_FILE_EXECUTION_FAILED.  I think technically this error is meant to be used when failing to execute an executable file, but I think it conveys the right idea much better than NS_ERROR_FILE_NOT_FOUND.

2. Create a new error code, maybe NS_ERROR_FILE_COMPILATION_FAILED, and return that.  I think this route is preferable, but it might be harder to get a change that adds a new error code.

This bug has a relatively easy solution and has been open for quite some time.  Please let's get this fixed.  Thanks.
(Assignee)

Comment 3

6 years ago
I dug into a simple xpcshell testcase in which a JSM is imported that imports another JSM containing a syntax error. The exception is correctly set in the mCallContext in the CallMethodHelper::Call that invokes the second nsXPCComponents_Utils::Import, but that doesn't seem to propagate back through the JS interpreter.
(Assignee)

Comment 4

6 years ago
I've confirmed that at mozJSComponentLoader.cpp:1021, when the JS_ExecuteScriptVersion call fails (in the first, correctly-formed jsm), there is no exception pending on the context. It's lodged somewhere inside the XPConnect machinery that called the second import.
(Assignee)

Comment 5

6 years ago
I have figured out where the exception is cleared, and it makes sense. I now understand that we don't necessarily want the inner exception to propagate out all the way, and the current setup is pretty correct. But yes, Glenn is correct that the FILE_NOT_FOUND error that is the default is really not a great choice.

Comment 6

5 years ago
I will buy quality beer, scotch, bourbon, or similar bounty for whoever makes the error message actionable: I've lost too much time over the lack of a helpful error message and want to see this addressed.
I would contribute gps' offered bounty.
(Assignee)

Comment 8

5 years ago
Created attachment 580571 [details] [diff] [review]
Add new error code to return a more sensible guess as to why a JSM import failed.
Attachment #580571 - Flags: review?(benjamin)
(Assignee)

Updated

5 years ago
Attachment #580571 - Flags: review?(mrbkap)
Comment on attachment 580571 [details] [diff] [review]
Add new error code to return a more sensible guess as to why a JSM import failed.

You should not be declaring a new xpconnect message code here. They are all declared in nsIXPConnect.idl and you're duplicating NS_ERROR_XPC_NOT_ENOUGH_ARGS. You should probably also add this to xpc.msg.
Attachment #580571 - Flags: review?(benjamin) → review-
Comment on attachment 580571 [details] [diff] [review]
Add new error code to return a more sensible guess as to why a JSM import failed.

Does anybody know why the exception from the nested compilation doesn't end up getting successfully propagated out here?
Attachment #580571 - Flags: review?(mrbkap)
(Assignee)

Comment 11

5 years ago
Gregory, it looks like the loader should be throwing a very actionable error in the JS error console (note: the global one, not the webpage one). Is this true?
(Assignee)

Comment 12

5 years ago
I did some more digging, and I can answer comment 10. The exception doesn't propagate out because it occurs inside the JS_ExecuteScript call in GlobalForLocation, and at that point we don't have DONT_REPORT_UNCAUGHT set, so the exception is handled by the mozJSLoaderErrorReporter (see comment 11). With the patch I will attach, I can make the exception bubble out to the outermost level, ie. the xpcshell test shows this output:

TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | SyntaxError: missing ; before statement

This is significantly less actionable in my opinion, as it's lacking the relevant file information.
(Assignee)

Comment 13

5 years ago
Created attachment 582000 [details] [diff] [review]
Propagate mozJSComponentLoader exceptions that occur while executing the script.
(Assignee)

Comment 14

5 years ago
For reference, this is what I see for the xpcshell test without my changes:

JS Component Loader: ERROR resource://test/syntax_error.jsm:1
                     SyntaxError: missing ; before statement

TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: /Users/jdm/src/mozilla-central/obj-x86_64-apple-darwin10.8.0/_tests/xpcshell/js/xpconnect/tests/unit/test_import_fail.js :: run_test :: line 3"  data: no]
(In reply to Josh Matthews [:jdm] from comment #12)
> I did some more digging, and I can answer comment 10. The exception doesn't
> propagate out because it occurs inside the JS_ExecuteScript call in
> GlobalForLocation

Is this code path also used when loading XPCOM components (rather than JS modules)? Could the root cause be the same for bug 700421?
(Assignee)

Comment 16

5 years ago
Created attachment 582005 [details] [diff] [review]
Improve xpcshell error reporting for generic exceptions caught at the top level.

Nevermind, I can make the xpcshell harness error reporting better.
(Assignee)

Comment 17

5 years ago
With the xpcshell harness patch and my mozJSComponentLoader patch, the output now reads:

TEST-UNEXPECTED-FAIL | (resource://test/syntax_error.jsm:1) | SyntaxError: missing ; before statement
(Assignee)

Updated

5 years ago
Attachment #580571 - Attachment is obsolete: true
(Assignee)

Comment 18

5 years ago
Created attachment 582040 [details] [diff] [review]
Propagate mozJSComponentLoader exceptions that occur while executing the script.

Tested by adding a failing import to NetUtil.jsm. The error messages in the console were significantly more reasonable, and nary a trace of FILE_NOT_FOUND was seen.
Attachment #582040 - Flags: review?(mrbkap)
(Assignee)

Updated

5 years ago
Attachment #582000 - Attachment is obsolete: true
(Assignee)

Comment 19

5 years ago
Created attachment 582041 [details] [diff] [review]
Improve xpcshell error reporting for generic exceptions caught at the top level.
Attachment #582041 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #582005 - Attachment is obsolete: true

Comment 20

5 years ago
Try run for d203eee17989 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d203eee17989
Results (out of 76 total builds):
    success: 55
    warnings: 19
    failure: 2
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-d203eee17989

Updated

5 years ago
Attachment #582040 - Flags: review?(mrbkap) → review+

Comment 21

5 years ago
Try run for d203eee17989 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d203eee17989
Results (out of 76 total builds):
    success: 55
    warnings: 19
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-d203eee17989
(In reply to Mozilla RelEng Bot from comment #21)
 sorry for the double bot post - slight differences in string caused this.
(Assignee)

Updated

5 years ago
Assignee: nobody → josh

Comment 23

5 years ago
Try run for c72d1a2ba108 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=c72d1a2ba108
Results (out of 55 total builds):
    success: 42
    warnings: 13
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-c72d1a2ba108

Comment 24

5 years ago
Try run for 021e6f733a85 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=021e6f733a85
Results (out of 55 total builds):
    success: 48
    warnings: 7
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-021e6f733a85
Comment on attachment 582041 [details] [diff] [review]
Improve xpcshell error reporting for generic exceptions caught at the top level.

Review of attachment 582041 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/xpcshell/head.js
@@ +341,5 @@
> +          msg += ":" + e.lineNumber;
> +        }
> +        msg += ")";
> +      } else {
> +        msg += "(xpcshell/head.js)";

I'd just get rid of the parentheses. I'm not sure where they came from anyway.
Attachment #582041 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 26

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/ec375c688831
http://hg.mozilla.org/integration/mozilla-inbound/rev/c99ee2ec1016
https://hg.mozilla.org/mozilla-central/rev/ec375c688831
https://hg.mozilla.org/mozilla-central/rev/c99ee2ec1016
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Depends on: 731093
You need to log in before you can comment on or make changes to this bug.