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

RESOLVED FIXED in Firefox 60

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: lina, Assigned: zacknoyes2)

Tracking

(Blocks 1 bug)

unspecified
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

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
Assignee

Comment 2

a year ago
Posted 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+

Comment 5

a year ago
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f3552d61627
Added `status` Error property on Sqlite.openConnection. r=mak

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8f3552d61627
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.