418406 - Make network error constants accessible via Components.results

Status: RESOLVED FIXED

(Core :: XPConnect, enhancement)

Description

[Wladimir Palant]

The error constants that are accessible via Components.results are defined in xpc.msg, this file currently lists the common errors (nsError.h, including file errors) and the XPC errors (nsIXPConnect.h). However, network errors (nsNetError.h) are not listed. This makes interpreting network errors from JavaScript unnecessarily hard - one has to hardcode constants and rely on them never changing. Also, exception messages contain only the nsresult code but no exception name.

Comment 1

[Wladimir Palant]

Attachment: Proposed patch

The description strings are mostly copied from nsNetError.h - with exception of a few strings that had to be shortened down as well as FTP and cache errors that are currently undocumented.

Comment 2

[Ben Bucksch (:BenB)]

With "unnecessarily hard", Waldimir means this:
http://mxr.mozilla.org/seamonkey/source/extensions/sroaming/resources/content/transfer/utility.js#288
which I absolutely hated and costed me a lot of time, but it was the only way to translate network errors.

Comment 3

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

What's the story here with regards to late l10n changes?

Comment 4

[Wladimir Palant]

This file isn't translated, error messages are always in English (and usually not visible to end users).

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 304233 [details] [diff] [review]
Proposed patch

Thanks for doing this! Just some comments on the strings:

+XPC_MSG_DEF(NS_ERROR_CONNECTION_REFUSED             , "The connection attempt failed")

The message here really must say "The connection was refused", because "failed" is is too unspecific

+XPC_MSG_DEF(NS_ERROR_PROXY_CONNECTION_REFUSED       , "The connection attempt to a proxy failed")

same here

+XPC_MSG_DEF(NS_ERROR_NET_TIMEOUT                    , "The connection was lost due to a timeout error")

Hmm, the rather more common occurrence for this error is when the connect() timed out.

+XPC_MSG_DEF(NS_ERROR_NOT_RESUMABLE                  , "This request is not resumable, but it was tried to resume it")

no, you can get the error without trying to resume. for example if you access the entityID of a nonresumable channel.

+XPC_MSG_DEF(NS_ERROR_FTP_PWD                        , "FTP error while retriving current directory")

retrieving (also in the next one)

+XPC_MSG_DEF(NS_ERROR_UNKNOWN_HOST                   , "The lookup of a hostname failed")

s/a/the/ ?

+XPC_MSG_DEF(NS_ERROR_UNKNOWN_PROXY_HOST             , "The lookup of a proxy hostname failed")

here too

+XPC_MSG_DEF(NS_ERROR_CACHE_ENTRY_DOOMED             , "Cache entry still exists but is already doomed")

why not just make this "Cache entry has been doomed". The cache entry doesn't really exist anymore when it has been doomed.

+XPC_MSG_DEF(NS_ERROR_DOCUMENT_NOT_CACHED            , "Document could not be fetched from the cache")

Document does not exist in the cache.

Comment 6

[Ben Bucksch (:BenB)]

> "The connection was refused"

This could be misunderstood by a user to mean that the password is wrong etc.. "Refused" smells a lot like policy/police.

I'd say "The network connection was refused by the server"
Or do we routinely get this when there's a firewall blocking outgoing connections? If so, drop "the server", but still add "network".

> "Cache entry still exists but is already doomed"

Please replace "doomed". My only association is a) the game "Doom" and b) voodoo, not even I know what this means in the context of cache.
Is "expired" correct? "declared as outdated"?

Comment 7

[Wladimir Palant]

(In reply to comment #5)
> +XPC_MSG_DEF(NS_ERROR_NET_TIMEOUT                    , "The connection was lost
> due to a timeout error")
> 
> Hmm, the rather more common occurrence for this error is when the connect()
> timed out.

Looking at netError.dtd, this is error is called "The connection has timed out" - I think we can reuse that description.

> +XPC_MSG_DEF(NS_ERROR_NOT_RESUMABLE                  , "This request is not
> resumable, but it was tried to resume it")
> 
> no, you can get the error without trying to resume. for example if you access
> the entityID of a nonresumable channel.

Ok, I added "or to request resume-specific data" that was in the original description of this error.

(In reply to comment #6)
Ben, these strings (and especially the ones concerning cache) are meant for developers and not for the user. TomTom HOME can "translate" them before showing them to the user, and it should be doing that.

Comment 8

[Wladimir Palant]

Attachment: Patch v2

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #6)
> Please replace "doomed". My only association is a) the game "Doom" and b)
> voodoo, not even I know what this means in the context of cache.
> Is "expired" correct? "declared as outdated"?

Especially the doomed message, but also the others, is unlikely to be seen by users. It's a technical term of our cache implementation... there's a doom() function, when you call it the entry is doomed. That basically deletes the entry.

Comment 10

[Damon Sicore (:damons)]

Comment on attachment 308262 [details] [diff] [review]
Patch v2

a1.9+=damons

Comment 11

[Reed Loden [:reed]]

Checking in js/src/xpconnect/src/xpc.msg;
/cvsroot/mozilla/js/src/xpconnect/src/xpc.msg,v  <--  xpc.msg
new revision: 1.27; previous revision: 1.26
done
Checking in js/src/xpconnect/src/xpcexception.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcexception.cpp,v  <--  xpcexception.cpp
new revision: 1.36; previous revision: 1.35
done