From ddb14f48bf4f827febe8359c1b19c600aa5aed6c Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Thu, 1 Oct 2020 14:31:08 +0200
Subject: [PATCH] we actually don't need to track write requests

as errors will bubble up to the txn
---
 src/matrix/storage/idb/Store.js       | 20 ++++++++++++-----
 src/matrix/storage/idb/Transaction.js | 31 +--------------------------
 2 files changed, 16 insertions(+), 35 deletions(-)

diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js
index cffd03e8..460d18f9 100644
--- a/src/matrix/storage/idb/Store.js
+++ b/src/matrix/storage/idb/Store.js
@@ -16,8 +16,6 @@ limitations under the License.
 
 import {QueryTarget} from "./QueryTarget.js";
 import {IDBRequestAttemptError} from "./error.js";
-import {reqAsPromise} from "./utils.js";
-import {StorageError} from "../common.js";
 
 class QueryTargetWrapper {
     constructor(qt) {
@@ -120,14 +118,26 @@ export class Store extends QueryTarget {
     }
 
     put(value) {
-        this._transaction._addWriteRequest(this._idbStore.put(value));
+        // If this request fails, the error will bubble up to the transaction and abort it,
+        // which is the behaviour we want. Therefore, it is ok to not create a promise for this
+        // request and await it.
+        // 
+        // Perhaps at some later point, we will want to handle an error (like ConstraintError) for
+        // individual write requests. In that case, we should add a method that returns a promise (e.g. putAndObserve)
+        // and call preventDefault on the event to prevent it from aborting the transaction
+        // 
+        // Note that this can still throw synchronously, like it does for TransactionInactiveError,
+        // see https://www.w3.org/TR/IndexedDB-2/#transaction-lifetime-concept
+        this._idbStore.put(value);
     }
 
     add(value) {
-        this._transaction._addWriteRequest(this._idbStore.add(value));
+        // ok to not monitor result of request, see comment in `put`.
+        this._idbStore.add(value);
     }
 
     delete(keyOrKeyRange) {
-        this._transaction._addWriteRequest(this._idbStore.delete(keyOrKeyRange));
+        // ok to not monitor result of request, see comment in `put`.
+        this._idbStore.delete(keyOrKeyRange);
     }
 }
diff --git a/src/matrix/storage/idb/Transaction.js b/src/matrix/storage/idb/Transaction.js
index 1b98515a..88e15186 100644
--- a/src/matrix/storage/idb/Transaction.js
+++ b/src/matrix/storage/idb/Transaction.js
@@ -14,7 +14,6 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import {IDBRequestError} from "./error.js";
 import {txnAsPromise} from "./utils.js";
 import {StorageError} from "../common.js";
 import {Store} from "./Store.js";
@@ -39,14 +38,6 @@ export class Transaction {
         this._txn = txn;
         this._allowedStoreNames = allowedStoreNames;
         this._stores = {};
-        this._writeRequests = null;
-    }
-
-    _addWriteRequest(request) {
-        if (!this._writeRequests) {
-            this._writeRequests = [];
-        }
-        this._writeRequests.push(request);
     }
 
     _idbStore(name) {
@@ -126,27 +117,7 @@ export class Transaction {
     }
 
     async complete() {
-        // check the write requests if we haven't failed yet
-        if (this._writeRequests) {
-            for (const request of this._writeRequests) {
-                if (request.error) {
-                    try {
-                        this.abort();
-                    } catch (err) {/* ignore abort error, although it would be useful to know if the other stuff got committed or not... */}
-                    return Promise.reject(new IDBRequestError(request, "Write request failed"));
-                }
-            }
-        }
-        const result = await txnAsPromise(this._txn);
-        // check the write requests if we haven't failed yet
-        if (this._writeRequests) {
-            for (const request of this._writeRequests) {
-                if (request.readyState !== "done") {
-                    return Promise.reject(new IDBRequestError(request, "Request is still pending after transaction finished"));
-                }
-            }
-        }
-        return result;
+        return txnAsPromise(this._txn);
     }
 
     abort() {