Closed
Bug 408412
Opened 17 years ago
Closed 16 years ago
Components.utils.import throws NS_ERROR_FILE_NOT_FOUND when there's a syntax error
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
VERIFIED
FIXED
mozilla1.9.1a2
People
(Reporter: hello, Assigned: mrbkap)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
8.78 KB,
patch
|
brendan
:
review+
brendan
:
superreview+
|
Details | Diff | Splinter Review |
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•16 years ago
|
||
I just ran into this and it makes debugging quite hard :(
Reporter | ||
Comment 2•16 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.
Comment 3•16 years ago
|
||
I suppose I avoid this problem by using a syntax checking editor.
Updated•16 years ago
|
Hardware: PC → All
Whiteboard: needTestcase → [testcase needed]
Assignee | ||
Comment 6•16 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
Comment 7•16 years ago
|
||
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.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Comment 10•16 years ago
|
||
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•16 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•16 years ago
|
Attachment #328493 -
Flags: superreview?(brendan)
Comment 12•16 years ago
|
||
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•16 years ago
|
||
Fix pushed as http://hg.mozilla.org/index.cgi/mozilla-central/rev/90020c4ad446
Status: ASSIGNED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 14•16 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•16 years ago
|
||
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)
Comment 16•16 years ago
|
||
Benjamin, do you recall why we have NS_ERROR_FILE_TARGET_DOES_NOT_EXIST? It seems unnecessary and misnamed? /be
Status: REOPENED → ASSIGNED
Comment 17•16 years ago
|
||
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•16 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
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Comment 19•16 years ago
|
||
Latest patch contained a unit test => in-testsuite+
Flags: in-testsuite+
Target Milestone: --- → mozilla1.9.1a2
Comment 20•16 years ago
|
||
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•16 years ago
|
Flags: blocking1.9.1?
You need to log in
before you can comment on or make changes to this bug.
Description
•