Closed Bug 1407778 Opened 7 years ago Closed 6 years ago

Sqlite.jsm: Set `status` on the error if `openAsyncDatabase` or `asyncClone` fails

Categories

(Toolkit :: Storage, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: lina, Assigned: zacknoyes2)

References

Details

Attachments

(1 file)

We currently include the status in the error message in https://searchfox.org/mozilla-central/rev/31606bbabc50b08895d843b9f5f3da938ccdfbbf/toolkit/modules/Sqlite.jsm#948,1029, and I think it would be useful to expose as a property on the error, too.

Callers could use this to handle specific errors (for example, remove and replace a database only if `openConnection` fails with `NS_ERROR_FILE_CORRUPTED`, and rethrow other errors) without parsing the message string.
SGTM
Priority: -- → P3
Attached patch 1407778.patchSplinter Review
Attachment #8944603 - Flags: review?(kit)
Comment on attachment 8944603 [details] [diff] [review]
1407778.patch

Thanks, Zack and William! Forwarding the review to Mak.
Attachment #8944603 - Flags: review?(kit) → review?(mak77)
Assignee: nobody → zacknoyes2
Status: NEW → ASSIGNED
Comment on attachment 8944603 [details] [diff] [review]
1407778.patch

Review of attachment 8944603 [details] [diff] [review]:
-----------------------------------------------------------------

The commit message should probably say "where" we are adding an error property (to Sqlite.jsm::openConnection)

::: toolkit/modules/tests/xpcshell/test_sqlite.js
@@ +101,5 @@
> +  let dest = OS.Path.join(OS.Constants.Path.profileDir, "corrupt.sqlite");
> +  Assert.ok(!(await OS.File.exists(dest)), "Database file should not exist yet");
> +
> +  await OS.File.copy(src, dest);
> +  let path = OS.Path.join(OS.Constants.Path.profileDir, "corrupt.sqlite");

Is not dest already pointing to the right path? Maybe worth renaming dest to path and just keep using that.
Attachment #8944603 - Flags: review?(mak77) → review+
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f3552d61627
Added `status` Error property on Sqlite.openConnection. r=mak
https://hg.mozilla.org/mozilla-central/rev/8f3552d61627
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.