754350 - Device Storage - Clean up error strings

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Doug Turner (:dougt)]

Followup from bug 717103 - I am leaving the error strings for debugging for now.  We should clean up up and subclass DOMError (or something)

Comment 1

[Paul Dagnelie]

Attachment: idea sketch

I don't expect this to be exactly what was desired, but this was my first shot at creating something that might do most of what it needs to.  What other capabilities should this have?

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 642140 [details]
idea sketch

does this work?

Comment 3

[Paul Dagnelie]

I haven't tried to plug it into the code or anything, I just wanted to get an idea of what was being looked for.  If this looks okay, I will try to plug it in and make it work.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

mName should be set to one of the strings from

http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#error-names-0

or

http://dev.w3.org/2006/webapi/FileAPI/#dfn-error-codes

Or if you feel like non of them are matching we can make up new ones, but should follow the same pattern.

So for DEV_STORE_FILE_DOES_NOT_EXIST .name should be set to "NotFoundError"
For DEV_STORE_ERROR_PERMISSION_DENIED .name should be set to "SecurityError"
For DEV_STORE_ERROR_UNKNOWN .name should be set to "UnknownError"
etc.

For DEV_STORE_ERROR_NON_STRING_TYPE_UNSUPPORTED we should simply throw a TypeError exception, no need to create a DOMError at all. You can throw a TypeError by returning NS_ERROR_TYPE_ERR as nsresult.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I also don't think there's a need for .type if it doesn't contain any more information than what can be deduced from .name.

And is DeviceStorageFile a scriptable interface??

Comment 6

[Doug Turner (:dougt)]

DeviceStorageFile is not scriptable.  it is used only by our implementation.

you can remove all of the path handling in nsDeviceStorageError.  That was just for me.  Maybe make it #ifdef DEBUG.

Comment 7

[Paul Dagnelie]

Attachment: idea sketch v2

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

So if this is all that we are planning on exposing, then you could just create an existing DOMError. It already supports setting arbitrary values as .name.

The only reason we would need to subclass is if we want to expose additional information than the error type (which is exposed as .name). By subclassing we can add additional properties which would contain this additional information.

Comment 9

[[e-mail] [:onecyrenus]]

Got an "Unknown" error while trying to write to a file on the sdcard which was already present. 

Possibly we would want a separate error case for this scenario.

I would venture that we should check if a file exists prior to attempting to write, and if it does exist we through "already exists" error or something like that.

Comment 10

[[e-mail] [:onecyrenus]]

throw* 

ack.. no excuse for that one.

Comment 11

[Doug Turner (:dougt)]

dclarke, this isn't the bug you're looking for.  please file a separate one and assign it to me.  if it is blocking your work, please mark it as such.

Comment 12

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 13

[Doug Turner (:dougt)]

noming - we really should clean up the error names so apps can depend on them.

Comment 14

[Doug Turner (:dougt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/302400cbd111

Comment 15

[Ryan VanderMeulen [:RyanVM]]

One of the patches in this push caused Windows mochitest-2 failures. Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/91616129b892

https://tbpl.mozilla.org/php/getParsedLog.php?id=16083979&tree=Mozilla-Inbound

2054 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/devicestorage/test/test_dotdot.html | Error must be SecurityError

Comment 16

[Doug Turner (:dougt)]

On windows, we were returning a type error instead of a security error.  I fixed this, and pushed to try:

https://tbpl.mozilla.org/?tree=Try&rev=ea3a9ef38d71

Comment 17

[Doug Turner (:dougt)]

https://hg.mozilla.org/mozilla-central/rev/838eee8853ca
https://hg.mozilla.org/releases/mozilla-aurora/rev/3ab210d3f02a