[CLOSED] NativeFileError for error callbacks in NativeFileSystem (#2057)

[core-ai-bot]

Issue by jbalsas
Sunday Dec 09, 2012 at 20:21 GMT
Originally opened as adobe/brackets#2318

---

This is a possible fix for #2057. It introduces a new NativeFileError implementing DOMError to substitute the current FileError as the specs dictate.

Important changes

• There's no need anymore for the FileError polyfill
• NativeFileSystem.FileError is moved to its own module NativeFileError. I thougth it would make more sense, and avoids dependencies with NativeFileSystem when only wanting to check error name values.
• _nativeToFileError has been modified to just convert a brackets.fs error code to a NativeFileError error name. Hopefully this clears out any confusion in NativeFileSystem by making explicit all the error constructors in the same way.
• DOMError has a name attribute where FileError had code.

Notes

• There's no longer a dependency with the browser implementation of FileError, nor the need to use a polyfill for the in-browser branch.
• Errors that don't have a custom message are more descriptive when printed on dialog errors (i.e. instead of showing error.4 you now see something like error. NotReadableError)
• The attribute change between DOMError and FileError may break error handling in existing extensions. We could add a code attribute in our NativeFileError class to ensure backward compatibility. I've done a quick search though, and only three extensions seem to be using error.code right now.
• I've run all the unit tests and smoke tests and everything seems to be working fine so I hope I didn't miss anything, but this change affects several files, so it could be a good idea to split it into several pull requests if you don't feel it's safe enough.

---

jbalsas included the following code: https://github.com/adobe/brackets/pull/2318/commits

[core-ai-bot]

Comment by jasonsanjose
Monday Dec 10, 2012 at 19:54 GMT

---

Initial review complete.

What were the 3 extensions that use error.code? I'd like to contact the authors when this change lands.

[core-ai-bot]

Comment by jbalsas
Monday Dec 10, 2012 at 20:52 GMT

---

I've updated the name and its references and pushed the changes.

The three extensions I've identified are:

• App Cache Buddy: (main.js line 91)
• CanIUse: (main.js line 143)
• brackets-snippets: (main.js line 214, line 284)

[core-ai-bot]

Comment by jasonsanjose
Tuesday Dec 11, 2012 at 18:00 GMT

---

@jbalsas Need to merge with master

[core-ai-bot]

Comment by jbalsas
Tuesday Dec 11, 2012 at 18:24 GMT

---

Done! ;)

[core-ai-bot]

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

---

Confirmed unit tests are passing on mac. Merging.

[core-ai-bot]

Comment by jasonsanjose
Wednesday Dec 12, 2012 at 21:57 GMT

---

@davidderaedt,@cfjedimaster and@jrowny: Please see the changes described above.