Components.utils.import throws NS_ERROR_FILE_NOT_FOUND when there's a syntax error

VERIFIED FIXED in mozilla1.9.1a2

Status

()

Core
XPConnect
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: thunder, Assigned: mrbkap)

Tracking

(Blocks: 1 bug)

Trunk
mozilla1.9.1a2
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
I'm getting this:

JS Component Loader: ERROR (null):0
                     uncaught exception: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: file:///Users/thunder/sources/hg.sandmill.org/weave/modules/weave.js :: <TOP_LEVEL> :: line 44"  data: no]

When one of the files I'm importing has a syntax error.  Fixing the error (which I track down by commenting out large chunks of code and uncommenting bit by bit) makes it all work again.

I just found out that it's because I import files which import other files.  Errors in the first one get reported OK, but errors in deep files don't.

Comment 1

10 years ago
I just ran into this and it makes debugging quite hard :(
(Reporter)

Comment 2

10 years ago
In my case, I work around the problem by loading all of modules from one file in chrome js, in reverse order to how would normally be imported (i.e., bottom-up).  That way errors get reported correctly.
I suppose I avoid this problem by using a syntax checking editor.

Updated

10 years ago
Blocks: 435025
Gah, this just hit me to. This is hell to debug. :(
Flags: blocking1.9.1?

Comment 5

10 years ago
This report needs a testcase.
Whiteboard: needTestcase
Hardware: PC → All
Whiteboard: needTestcase → [testcase needed]
(Assignee)

Comment 6

10 years ago
If someone gives me a concrete way to reproduce this (even if it's something like "add a syntax error in file 'x'") then I'll fix it.
Assignee: nobody → mrbkap
Created attachment 328475 [details] [diff] [review]
unit test

Here is a unit test that fails if the Component.utils.import of a js file with invalid syntax throws an exception. Might need to modify it, not sure exactly what your expecting to change this to.

Updated

10 years ago
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 421392

Updated

10 years ago
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Updated

10 years ago
Duplicate of this bug: 421392
(Assignee)

Comment 10

10 years ago
Created attachment 328493 [details] [diff] [review]
Proposed fix, v1

This includes Mossop's unit test. With this patch, we propagate exceptions from our context onto the outer context from C.u.import(), which propagates to the console when starting Firefox.
Attachment #328475 - Attachment is obsolete: true
Attachment #328493 - Flags: review?(shaver)
(Assignee)

Updated

10 years ago
Status: REOPENED → ASSIGNED
Whiteboard: [testcase needed]
Comment on attachment 328493 [details] [diff] [review]
Proposed fix, v1

r=shaver.  Some pretty gross stylistic inconsistencis in there, all of them my fault!
Attachment #328493 - Flags: review?(shaver) → review+
(Assignee)

Updated

10 years ago
Attachment #328493 - Flags: superreview?(brendan)
Comment on attachment 328493 [details] [diff] [review]
Proposed fix, v1

>+        uint32 oldopts = 0;
>+        if (exception) {
>+            oldopts = JS_SetOptions(cx,
>+                            JS_GetOptions(cx) | JSOPTION_DONT_REPORT_UNCAUGHT);

Might be tidier to set oldopts = JS_GetOptions(cx); and then JS_SetOptions(cx, oldopts | JSOPTION_DONT...).

sr=me in any case.

/be
Attachment #328493 - Flags: superreview?(brendan) → superreview+
(Assignee)

Comment 13

10 years ago
Fix pushed as http://hg.mozilla.org/index.cgi/mozilla-central/rev/90020c4ad446
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 14

10 years ago
I had to back this out in http://hg.mozilla.org/index.cgi/mozilla-central/rev/d194c529cba0 because a unit test started failing.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 15

10 years ago
Created attachment 332229 [details] [diff] [review]
patch v2

The functional difference here is that in the case where we don't find the file, we explicitly return NS_ERROR_FILE_NOT_FOUND (since on some systems we could return NS_ERROR_FILE_TARGET_DOES_NOT_EXIST which breaks some unit tests).

I also noticed some early returns where we have to reset our context's options.
Attachment #328493 - Attachment is obsolete: true
Attachment #332229 - Flags: superreview?(brendan)
Attachment #332229 - Flags: review?(brendan)
Benjamin, do you recall why we have NS_ERROR_FILE_TARGET_DOES_NOT_EXIST? It seems unnecessary and misnamed?

/be
Status: REOPENED → ASSIGNED
Comment on attachment 332229 [details] [diff] [review]
patch v2

>+        // If |exception| is non-null, then our caller wants to propagate any
>+        // exceptions out to our caller. Ensure that the engine doesn't

First sentence doesn't make sense -- either "our caller wants *us* to propagate ..." or "our caller wants to propagate any exceptions out to our grand-caller."

Thanks for the irc-pastebinned interdiff -- r+sr=me with above nit picked.

/be
Attachment #332229 - Flags: superreview?(brendan)
Attachment #332229 - Flags: superreview+
Attachment #332229 - Flags: review?(brendan)
Attachment #332229 - Flags: review+
(Assignee)

Comment 18

10 years ago
Pushed as http://hg.mozilla.org/mozilla-central/index.cgi/rev/5cd3b111caa8 and http://hg.mozilla.org/mozilla-central/index.cgi/rev/10c6fbeceb79 (since I missed brendan's review comment in the first commit).
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
Latest patch contained a unit test => in-testsuite+
Flags: in-testsuite+
Target Milestone: --- → mozilla1.9.1a2
Running test_bug408412.js:
*** test pending
*** test finished
*** exiting
*** PASS ***

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080807233207 Minefield/3.1a2pre ID:2008080620
Status: RESOLVED → VERIFIED

Updated

10 years ago
Flags: blocking1.9.1?
Blocks: 463122
You need to log in before you can comment on or make changes to this bug.