From 1214974e85674e505b12e1530e0d36a4958581dc Mon Sep 17 00:00:00 2001 From: David Fahlander Date: Tue, 5 Apr 2016 17:00:10 +0200 Subject: [PATCH] Throw if not using Dexie.Promise from transaction scopes. This commit will hopefully help to find errors when using async / await or Promise.all() within transaction scopes. Before this commit, those things could be done and the code seemed to work perfectly! The problem was that the transaction was not used. Instead it was executing the stuff transactionless, (each operation did its own transaction), wich is both slow and error prone. --- .eslintrc.json => src/.eslintrc.json | 0 src/Dexie.js | 10 +++++++--- src/errors.js | 6 ++++-- test/.eslintrc.json | 17 +++++++++++++++++ test/tests-transaction.js | 17 +++++++++++++++++ 5 files changed, 45 insertions(+), 5 deletions(-) rename .eslintrc.json => src/.eslintrc.json (100%) create mode 100644 test/.eslintrc.json diff --git a/.eslintrc.json b/src/.eslintrc.json similarity index 100% rename from .eslintrc.json rename to src/.eslintrc.json diff --git a/src/Dexie.js b/src/Dexie.js index 3e39411a4..efa7cb6a7 100644 --- a/src/Dexie.js +++ b/src/Dexie.js @@ -892,9 +892,13 @@ export default function Dexie(dbName, options) { // Finally, call the scope function with our table and transaction arguments. Promise._rootExec(function() { returnValue = scopeFunc.apply(trans, tableArgs); // NOTE: returnValue is used in trans.on.complete() not as a returnValue to this func. - if (returnValue && typeof returnValue.next === 'function' && typeof returnValue.throw === 'function') { - // scopeFunc returned an iterable. Handle yield as await. - returnValue = awaitIterable(returnValue); + if (returnValue) { + if (typeof returnValue.next === 'function' && typeof returnValue.throw === 'function') { + // scopeFunc returned an iterable. Handle yield as await. + returnValue = awaitIterable(returnValue); + } else if (typeof returnValue.then === 'function' && (!returnValue.hasOwnProperty('_PSD'))) { + reject(stack(new exceptions.IncompatiblePromise())); + } } }); }); diff --git a/src/errors.js b/src/errors.js index f4db5c93b..ec3fbfd8a 100644 --- a/src/errors.js +++ b/src/errors.js @@ -14,7 +14,8 @@ var dexieErrorNames = [ 'SubTransaction', 'Unsupported', 'Internal', - 'DatabaseClosed' + 'DatabaseClosed', + 'IncompatiblePromise' ]; var idbDomErrorNames = [ @@ -38,7 +39,8 @@ var errorList = dexieErrorNames.concat(idbDomErrorNames); var defaultTexts = { VersionChanged: "Database version changed by other database connection", - DatabaseClosed: "Database has been closed" + DatabaseClosed: "Database has been closed", + IncompatiblePromise: "Incompatible Promise used in transaction scope. See http://tinyurl.com/znyqjqc" }; // diff --git a/test/.eslintrc.json b/test/.eslintrc.json new file mode 100644 index 000000000..586e7d174 --- /dev/null +++ b/test/.eslintrc.json @@ -0,0 +1,17 @@ +{ + "parserOptions": { + "ecmaVersion": 6, + "sourceType": "module", + "ecmaFeatures": { + } + }, + "env": { + "browser": true + }, + "rules": { + "no-undef": ["error"] + }, + "globals": { + "Promise": true + } +} diff --git a/test/tests-transaction.js b/test/tests-transaction.js index d48569652..bb6c298d9 100644 --- a/test/tests-transaction.js +++ b/test/tests-transaction.js @@ -2,6 +2,8 @@ import Dexie from 'dexie'; import {module, stop, start, asyncTest, equal, ok} from 'QUnit'; import {resetDatabase} from './dexie-unittest-utils'; +"use strict"; + var db = new Dexie("TestDBTrans"); db.version(1).stores({ users: "username", @@ -20,6 +22,21 @@ module("transaction", { } }); +asyncTest("Transaction should fail if returning non-Dexie Promise in transaction scope", function(){ + db.transaction('rw', db.users, function() { + return window.Promise.resolve().then(()=> { + ok(Dexie.currentTransaction == null, "Dexie.currentTransaction == null. If this assertion fails, don't weap. Rejoice and try to understand how the hell this could be possible."); + return db.users.add({ username: "foobar" }); + }).then(()=>{ + return db.users.add({ username: "barfoo" }); + }); + }).then (function(){ + ok(false, "Transaction should not commit because we were using a non-Dexie promise"); + }).catch ('IncompatiblePromiseError', function(e){ + ok(true, "Good. Should fail with 'IncompatiblePromiseError': " + e); + }).finally(start); +}); + asyncTest("empty transaction block", function () { db.transaction('rw', db.users, db.pets, function () { ok(true, "Entering transaction block but dont start any transaction");