Improve Error Categorization and Formatting #86
Conversation
ghost
assigned 
|
@sashei @mozilla-lockbox/product-integrity This should be ready to look at. There is one change from the spec in #64, which is that if no separate message is included, then only the reason is used for the error message:
I don't think there's any practical way to PI verify short of analyzing the updated unit-tests. |
test/items-test.js
Outdated
| assert.strictEqual(err.reason, DataStoreError.INVALID_ITEM); | ||
| assert.deepEqual(err.details, { | ||
| "entry.username": "string.max" | ||
| }); | ||
| } | ||
| }); | ||
| it("fails to prepare an item with excessive entry.psssword", () => { | ||
| it("fails to prepare an item with excessive entry.passsword", () => { |
There was a problem hiding this comment.
I'm guessing this is meant to be entry.password?
lib/util/errors.js
Outdated
| * @member {string} DataStoreError.CRYPTO | ||
| */ | ||
| /** | ||
| * An operation requires network connectivety, but there is none. |
| reason = message; | ||
| message = ""; | ||
| constructor(reason, message) { | ||
| if (-1 === REASONS.indexOf(reason)) { |
There was a problem hiding this comment.
Not sure if we want to use Array#includes() versus indexOf() === -1. On that note, I'm not sure if we have any list of minimum browser support that we are planning, or if we should run this library through something like Babel to make it ES5 compliant (and minify it, etc).
Also, not sure if we should log any REASON misses. Like, if somebody typed DataStoreError.GENERIC_ERRROR instead of DataStoreError.GENERIC_ERROR.
There was a problem hiding this comment.
I can take a look at Array#includes(), but I've been hesitant to use APIs that I haven't had personal experience with on other platforms ... notably iOS WebKit (which is the target for lockbox-ios).
As far as minifying ... prove to me the size is an actual problem to deal with here, and not something that the consumers are better equiped for.
|
thinking on it more, logging a potentially bad error reason isn't that useful. As a developer, one ought to be testing the right error. Further, there's no good way to have it emit during the dev lifecycle but not emit in the user lifecycle. |
Fixes #64