1573828 - Ensure that IndexedDB.jsm transaction.promiseComplete is rejected with an error when transaction.abort() has been called

Status: RESOLVED FIXED

(Toolkit :: Async Tooling, defect, P1)

Description

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

As part of Bug 1542181 we added in the transaction wrapper defined in IndexedDB.jsm an onabort handler which rejects transaction.error.

Based on the discussion from the kinto PR that is going to apply a similar change to kinto.js (https://github.com/Kinto/kinto.js/pull/1044) I took a look to the error rejected when a transaction is aborted because of a quota exceeded issues and when the transaction is aborted using the transaction.abort() method and it seems that:

• when the quota exceeded issue is triggered (as described in this phabricator comment), transaction.onabort is being called as expected and transaction.error is QuotaExceededError as expected

• when transaction.abort() is explicitly called, transaction.onabort is being called as expected but transaction.error is not defined (nor is evt.target.error from what I saw)

We should look into this and ensure that on an aborted transaction the transaction wrapper is going to always reject promiseComplete with an error object.

needinfo :asuth to double-check in which conditions the transaction.error is not defined for an aborted transaction and when it is supposed to be defined

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

The "abort a transaction" algorithm (https://w3c.github.io/IndexedDB/#abort-transaction) takes arguments transaction and error, where error is allowed to be null. The abort() method (https://w3c.github.io/IndexedDB/#dom-idbtransaction-abort) is defined to invoke "abort a transaction" with a null error. This is the only case in which a null error will be provided, per spec, at the current time. In all other cases we expect to see an AbortError, QuotaExceededError, UnknownError (if something happens in commit and we don't have a better error), or the propagated error of performing an underlying operation (ex: https://w3c.github.io/IndexedDB/#ref-for-abort-a-transaction%E2%91%A6).

Given that, it might make sense to normalize the null to an indication that the abort was triggered manually.

Comment 2

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

Attachment: Bug 1573828 - Ensure IndexedDB.jsm transaction.promiseComplete is rejected with an error object. r?asuth!

Comment 3

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

Marking it as P1 (as it is related to Bug 1542181, which has been uplifted to 69, and so it would be good to ask an uplift to 69 for this change too asap).

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8301877895ed
Ensure IndexedDB.jsm transaction.promiseComplete is rejected with an error object. r=asuth

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/8301877895ed