1476268 - Fix uncaught storage.local IDBTransaction error on explicit IDBTransaction abort

Status: RESOLVED FIXED

(WebExtensions :: Storage, enhancement, P1)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

When an IDBTransaction is explicitly aborted [1] the transaction's promise is going to be an uncaught rejected error (also, the error property of the IDBTransaction is actually going to be null [2] when the transaction is explicitly aborted, and we are already raising the original error that is making the "set" method to reject the transaction and so we should catch and ignore the rejection on transaction's promise in that case).

Also related to that IDB transaction, the ExtensionStorageIDB's set method should wait explicitly that the transaction has been completed before returning (by explicitly await on the promise returned by the Transaction's promiseComplete method [3]).

[1]: using the abort() method, as we do inside the ExtensionStorageIDB's set method when we get an exception while storing a set of values inside the storage.local IndexedDB backend: https://searchfox.org/mozilla-central/rev/b0275bc977ad7fda615ef34b822bba938f2b16fd/toolkit/components/extensions/ExtensionStorageIDB.jsm#92
[2]: as also mentioned in the docs related to IDBTransaction.error on MDN https://developer.mozilla.org/en-US/docs/Web/API/IDBTransaction/error
[3]: https://searchfox.org/mozilla-central/rev/b0275bc977ad7fda615ef34b822bba938f2b16fd/toolkit/modules/IndexedDB.jsm#252-254

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1476268 - Fix uncaught storage.local IDBTransaction error on explicit IDBTransaction abort.

Review commit: https://reviewboard.mozilla.org/r/257498/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/257498/

Comment 2

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8992621 [details]
Bug 1476268 - Fix uncaught storage.local IDBTransaction error on explicit IDBTransaction abort.

https://reviewboard.mozilla.org/r/257498/#review264416

::: toolkit/components/extensions/ExtensionStorageIDB.jsm:222
(Diff revision 1)
> +          // We can ignore this rejection because we are aborting the transaction explicitly,
> +          // and so the transaction.error would be actually null, and we are already rejecting
> +          // the original error below.

We ignore this rejection because we are explicitly aborting the transaction, the transaction.error will be null, and we throw the original error below.

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8992621 [details]
Bug 1476268 - Fix uncaught storage.local IDBTransaction error on explicit IDBTransaction abort.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257498/diff/1-2/

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8992621 [details]
Bug 1476268 - Fix uncaught storage.local IDBTransaction error on explicit IDBTransaction abort.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257498/diff/2-3/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b6028970a75
Fix uncaught storage.local IDBTransaction error on explicit IDBTransaction abort. r=mixedpuppy

Comment 6

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/5b6028970a75