I put the zr.open in the try because if the file does not exist it will throw exception, and if zip reader is left open it usually crashes the add-on.
Crash? I wouldn't know about that. But yes, it should be closed. But then again, only .close()
it after it has actually been opened.
try {
zr.open();
try {
// do some stuff
}
// another catch possible as well.
finally {
zr.close();
}
}
catch (ex) {
// handle
}
var reusableStreamInstance = Cc['@mozilla.org/scriptableinputstream;1'].createInstance(Ci.nsIScriptableInputStream);
I'm resuing the input stream create instnace, how is it not safe?
The stream wrappers, or XPCOM in general really, usually have a contract that you should call .init()
only once, unless stated otherwise. nsIScriptableInputStream
does not state otherwise, and while it just works(tm) to call .init()
again and again right now, that might change in the future.
Better create a stream wrapper per wrapped stream.
Should I close the input stream instance after i am all done or everytime after init
Yes.
Is Cu.forceGC needed? I'm not sure I thought it might be.
If you're not sure, then it is not needed. If you know it is "needed", check if there is another way. Manually calling the garbage collector does not only impact performance, it is - or at least used to be - a sure way to crash when calling it too often/too randomly.
My exception checking is weird, I check the .name I'm not sure if this is the recommended way
Actually, you can check ex.result
for nsIException
s, even within the catch.
catch (ex if ex.result == Components.results.NS_ERROR_FILE_NOT_FOUND) {
Services.ww.activeWindow.alert('XPI at path does not exist!\n\nPath = ' + pathToXpiToRead);
}
catch (ex) {
console.warn('exception occured = ', ex);
}
My take
var {classes: Cc, interfaces: Ci, results: Cr, Constructor: CC, utils: Cu } =
Components;
let ZipFileReader =
CC("@mozilla.org/libjar/zip-reader;1", "nsIZipReader", "open");
let ScriptableInputStream =
CC("@mozilla.org/scriptableinputstream;1", "nsIScriptableInputStream", "init");
function handleEntry(name) {
try {
let entry = this.getEntry(name);
if (entry.isDirectory) {
console.log(name + " is directory, no stream to read");
return false;
}
let stream = new ScriptableInputStream(this.getInputStream(name));
try {
// Use readBytes to get binary data, read to read a (null-terminated) string
let contents = stream.readBytes(entry.realSize);
console.log("Contents of " + name, contents);
}
finally {
stream.close();
}
return true;
}
catch (ex) {
console.warn("Failed to read " + name);
}
return false;
}
try {
var xpi = Services.dirsvc.get("ProfD", Ci.nsIFile);
"extensions/noverflow@sdrocking.com.xpi".
split("/").forEach(p => xpi.append(p));
let reader = new ZipFileReader(xpi);
try {
let entries = reader.findEntries('*');
while (entries.hasMore()) {
let name = entries.getNext();
if (handleEntry.call(reader, name)) {
console.debug("Handled entry " + name);
}
}
}
finally {
reader.close();
}
}
catch (ex if ex.result == Cr.NS_ERROR_FILE_NOT_FOUND) {
Services.ww.activeWindow.alert("XPI at path does not exist!\n\nPath = " + xpi.path);
}
catch (ex) {
console.warn("exceptional exception", ex);
}
Edit I moved xpi
into the try-catch block, so it doesn't pseudo-leak (becomes a global variable, i.e. this.xpi
or window.xpi
).
Nested try-blocks
As you asked for this in the comments...
First lets see what happens with this:
try {
try {
throw new Error("oops");
}
finally {
console.log("finally");
}
}
catch (ex) {
console.error("outer", ex.message);
}
Result
"finally"
"outer" "oops"
Now, if we already caught the exception in the inner try-block by adding a catch block
try {
try {
throw new Error("oops");
}
catch (ex) {
console.error("inner", ex.message);
}
finally {
console.log("finally");
}
}
catch (ex) {
console.error("outer", ex.message);
}
Result
"inner" "oops"
"finally"
And now, lets re-throw the error.
try {
try {
throw new Error("oops");
}
catch (ex) {
console.error("inner", ex.message);
throw ex;
}
finally {
console.log("finally");
}
}
catch (ex) {
console.error("outer", ex.message);
}
Result
"inner" "oops"
"finally"
"outer" "oops"
So there you have it. Any given exception will be caught only once, by the nearest enclosing catch-block, unless it is re-thrown. Of course any new exceptions raised in "inner" block (because code in catch-block way do something that throws ;) will be caught by the "outer" block.