Device Storage - Clean up error strings




7 years ago
7 years ago


(Reporter: dougt, Assigned: dougt)



Firefox Tracking Flags

(blocking-basecamp:-, firefox18 fixed, firefox19 fixed)



(1 attachment, 2 obsolete attachments)



7 years ago
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

7 years ago
Posted file idea sketch (obsolete) —
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?
Attachment #642140 - Flags: feedback?(doug.turner)


7 years ago
Attachment #642140 - Attachment is patch: false


7 years ago
OS: Mac OS X → All
Hardware: x86 → All

Comment 2

7 years ago
Comment on attachment 642140 [details]
idea sketch

does this work?

Comment 3

7 years ago
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.
OS: All → Mac OS X
Hardware: All → x86
mName should be set to one of the strings from


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"

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.
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

7 years ago
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

7 years ago
Posted patch idea sketch v2 (obsolete) — Splinter Review
Attachment #642140 - Attachment is obsolete: true
Attachment #642140 - Flags: feedback?(doug.turner)
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.
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.

ack.. no excuse for that one.

Comment 11

7 years ago
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

7 years ago
Posted patch patch v.1Splinter Review
Attachment #643666 - Attachment is obsolete: true
Attachment #670539 - Flags: review?(jonas)

Comment 13

7 years ago
noming - we really should clean up the error names so apps can depend on them.
blocking-basecamp: --- → ?
One of the patches in this push caused Windows mochitest-2 failures. Backed out.

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

Comment 16

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


7 years ago
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.