[CLOSED] NativeFileSystem APIs pass inconsistent types to error handlers

[core-ai-bot]

Issue by peterflynn
Monday Nov 05, 2012 at 22:49 GMT
Originally opened as adobe/brackets#2057

---

The error callbacks passed to various NativeFileSystem APIs are sometimes passed an integer error code, vs. sometimes passed a FileError object containing such an error code. Sometimes even different codepaths through the same method are inconsistent here.

The spec is unambiguous on which one is correct: "the err argument to the ErrorCallback MUST be a DOMError object..."

• FileEntry.createWriter(), Entry.getMetadata(), and requestNativeFileSystem() all pass a number insteead of a FileError
• DirectoryEntry.getFile() and getDirectory() pass a number _or_a FileError, depending on case
• DirectoryReader.readEntries() is similarly inconsistent
• showOpenDialog(), albeit nonstandard, passes a number

[core-ai-bot]

Comment by pthiess
Tuesday Nov 06, 2012 at 00:55 GMT

---

Reviewed

[core-ai-bot]

Comment by jbalsas
Sunday Nov 18, 2012 at 11:35 GMT

---

Hi,

I was looking at this, and it looks like the error callbacks are all being called with a FileError. What seems to be wrong is the jsdoc in _nativeToFileError:

/**
 * Converts a brackets.fs.ERR_* error code to a FileError.* error code
 *`@`param {number} nativeErr
 *`@`return {number}
 */
 _nativeToFileError: function (nativeErr) { 
   var error;
   [...]
   return new NativeFileSystem.FileError(error);   
 }

@peterflynn Is it possible that this caused the confussion, or am I missing something?

[core-ai-bot]

Comment by jbalsas
Sunday Nov 18, 2012 at 11:36 GMT

---

Another thing is that looking at the specs there are some difference between the draft originally used in the implementation of NativeFileSystem and the current one

In the old one, ErrorCallback was supposed to receive a FileError, but that interface seems to have been deprecated in the currents specs and it expects a DomError instead.

Mozilla already dropped FileError as of Firefox 13.0, though I haven't been able to find any information regarding plans for this on Chrome.

I guess this is one of the perks of implementing working drafts... should we update NativeFileSystem already to align with the new specifications?

[core-ai-bot]

Comment by jasonsanjose
Tuesday Dec 11, 2012 at 22:09 GMT

---

FBNC@peterflynn

[core-ai-bot]

Comment by peterflynn
Thursday Dec 13, 2012 at 00:29 GMT

---

@jbalsas: sorry about my mistake in the original bug, and thanks for the cleanup in pull #2318!

@jasonsanjose: I noticed that resolveNativeFileSystemPath() doesn't follow Chema's new conventions, though. It calls the old conversion function (which no longer exists afaict) and doesn't new up a NativeFileError to wrap the error code in... Shall one of us fix that real quick before closing this?

[core-ai-bot]

Comment by jbalsas
Thursday Dec 13, 2012 at 00:34 GMT

---

@peterflynn About resolveNativeFileSystemPath, I also noticed it just now while updating the comments for adobe/brackets#2063. I fixed it along with the last batch of changes, so you could pick it up from there. That's of course unless you'd want it right away, in case that pull request takes some time to get through.

[core-ai-bot]

Comment by peterflynn
Saturday Dec 15, 2012 at 02:22 GMT

---

Closing now that #2063 has also landed