12

I saw sync-promise posted on Reddit and got into a discussion with the author. We noticed some weird inconsistencies in the relationship between IndexedDB transactions and promises.

IndexedDB transactions auto-commit when all the onsuccess events finish. One complication is that you can't do anything asynchronous inside an onsuccess callback except do another operation on the same transaction. For example, you can't start an AJAX request in an onsuccess and then reuse the same transaction after the AJAX request returns some data.

What do promises have to do with it? As I understand it, promise resolution is supposed to always be asynchronous. This would imply that you can't use promises without auto-committing an IndexedDB transaction.

Here is an example of what I'm talking about:

var openRequest = indexedDB.open("library");

openRequest.onupgradeneeded = function() {
  // The database did not previously exist, so create object stores and indexes.
  var db = openRequest.result;
  var store = db.createObjectStore("books", {keyPath: "isbn"});
  var titleIndex = store.createIndex("by_title", "title", {unique: true});
  var authorIndex = store.createIndex("by_author", "author");

  // Populate with initial data.
  store.put({title: "Quarry Memories", author: "Fred", isbn: 123456});
  store.put({title: "Water Buffaloes", author: "Fred", isbn: 234567});
  store.put({title: "Bedrock Nights", author: "Barney", isbn: 345678});
};

function getByTitle(tx, title) {
  return new Promise(function(resolve, reject) {
    var store = tx.objectStore("books");
    var index = store.index("by_title");
    var request = index.get("Bedrock Nights");
    request.onsuccess = function() {
      var matching = request.result;
      if (matching !== undefined) {
        // A match was found.
        resolve(matching);
      } else {
        // No match was found.
        console.log('no match found');
      }
    };
  });
}

openRequest.onsuccess = function() {
  var db = openRequest.result;
  var tx = db.transaction("books", "readonly");
  getByTitle(tx, "Bedrock Nights").then(function(book) {
    console.log('First book', book.isbn, book.title, book.author);
    return getByTitle(tx, "Quarry Memories");
  }).then(function(book) {
    console.log('Second book', book.isbn, book.title, book.author);
    // With native promises this gives the error:
    // InvalidStateError: An attempt was made to use an object that is not, or is no longer, usable
    // With bluebird everything is fine
  });
};

(Full disclosure: demo was created by paldepind, not me!)

I've tried it in Chrome and Firefox. It fails in Firefox due to the transaction auto-committing, but it actually works in Chrome! Which behavior is correct? And if Firefox's behavior is correct, is it literally impossible to use "correct" promise implementations with IndexedDB transactions?

Another complication: If I load bluebird before running the above demo, it works in both Chrome and Firefox. Does this imply that bluebird is resolving promises synchronously? I thought it wasn't supposed to do that!

JSFiddle

Brett Zamir
  • 14,034
  • 6
  • 54
  • 77
dumbmatter
  • 9,351
  • 7
  • 41
  • 80
  • The promise constructor is always called synchronously (in bluebird and in native promises). Bluebird is just being bluebird - it's not resolving synchronously (thens are still executed at a next chain iteration. That said I don't understand the issue with IndexedDB and A+ promises very well. I'll see if I can ping some smart people. – Benjamin Gruenbaum Feb 07 '15 at 22:33

3 Answers3

10

This is probably due to the difference between microtasks and tasks ("macrotasks"). Firefox has never had a standards-complaint promise implementation that uses microtasks, whereas Chrome, Bluebird, and others correctly use microtasks. You can see this in how a microtask (which executes "sooner" than a macrotask, but still async) falls inside the transaction boundary, whereas a macrotask (e.g. from Firefox's promises) does not.

So, this is a Firefox bug.

Domenic
  • 110,262
  • 41
  • 219
  • 271
  • Thanks for the information! Do you have any reference to this being a Firefox bug? I found [this related discussion from last year](http://lists.w3.org/Archives/Public/public-webapps/2014AprJun/0811.html) and it doesn't seem to reach a resolution. If anything, it seems that most people are saying that Chrome's current behavior is wrong. Maybe I'm misreading things. – dumbmatter Feb 08 '15 at 06:41
  • 4
    This is fixed in Firefox as of version 60. See [bug 1193394](https://bugzilla.mozilla.org/show_bug.cgi?id=1193394). – brianskold Jun 19 '18 at 23:12
8

Ok, so I've once again taken a deep dive into the IndexedDB, the DOM and the HTML specification. I really need to get this right for SyncedDB since it relies heavily on promises inside transactions.

The crux of the problem is whether or not the delayed execution of the onFulfilled and the onRejected callbacks to then that Promises/A+ compliant must exhibit will trigger an IndexedDB transaction commit.

The IndexedDB rules for a transactions lifetime are actually pretty straight forward when you extract them from the specification and line them up:

This roughly translates to:

  • When you create a transaction you can place as many request against it as you wish.
  • From then on new request can only be made inside event handlers for another requests success or error event listener.
  • When all requests has been executed and no new requests placed the transaction will commit.

The question then becomes: If a promise is fulfilled inside a request's success or error event listener will its onFulfilled callbacks be invoked before the IndexedDB sets the transaction as inactive again? I.e. will onFullfilled callbacks be called as part of step 3 in firing a success event?

The step dispatches an event and IndexedDB uses DOM events so the actual operation performed is beyond the IndexedDB specification. The steps for dispatching an event is, instead, specified here in the DOM specification. Going over the steps it becomes clear that at no point is a microtask (which would call the promise callbacks) checkpoint performed. So the initial conclusion is that the transaction will be closed before any onFulfilled callbacks will be invoked.

However, if we attach the event listeners by specifying an onsuccess attribute on the request object things gets more hairy. In that case we are not simply adding an event listener as per the DOM specification. We are instead setting an event handler IDL attribute as defined in the HTML specification.

When we do that the callback is not added directly to the list of event listeners. It is instead "wrapped" inside the the event handlers processing algorithm. This algorithm performs the following important operations:

  1. In step 3 it runs the jump to code entry-point algorithm.
  2. This then performs the steps to clean up after running a callback which
  3. Finally, this performs a microtask checkpoint. Which means that your promise callbacks will be invoked before the transaction is marked as inactive! Hurrah!

This is good news! But it is weird how the answer depends on whether or not you listen for the success event by using addEventListener or set a onsuccess event handler. If you do the former the transaction should be inactive when your promise's onFulfilled callbacks is invoked and if you do the later it should still be active.

I was, however not able to reproduce the difference in existing browsers. With native promises Firefox fails at the example code no matter what and Chrome succeeds even when using addEventListener. It is possible that I've overlooked or misunderstood something in the specifications.

As a final note Bluebird promises will close transactions in Internet Explorer 11. This is due to the scheduling that Bluebird uses in IE. My synchronized promise implementation works inside transactions in IE.

paldepind
  • 4,680
  • 3
  • 31
  • 36
  • You don't need a separate synchronous promise implementation, you can make a copy of bluebird synchronous by calling `Promise.setScheduler(function(fn){fn();})` – Esailija Feb 09 '15 at 12:33
  • You don't even have to do that. Bluebird has a synchronized build. But SyncPromise is targeted at library implementors. And depending on a pretty huge full featured promise library is undesirable in such a use case. – paldepind Feb 09 '15 at 12:59
5

You are correct: Promises are resolved asynchronously, and IndexedDB has some synchronous requirements. While other answers point out that native promises may work correctly with IndexedDB in certain versions of certain browsers, it is likely as a practical matter that you will have to deal with the issue of it not working in some of the browsers you're targeting.

Using a synchronous promise implementation instead, however, is a horrible idea. Promises are asynchronous for very good reasons, and you are introducing needless chaos and potential for bugs if you make them synchronous instead.

There is a fairly straightforward workaround, however: use a Promise library that provides a way to explicitly flush its callback queue, and an IndexedDB wrapper that flushes the promise callback queue after invoking event callbacks.

From the Promises/A+ point of view, there isn't any difference between the handlers being called at the end of the event, or at the beginning of the next tick cycle -- they are still being called after all the code that set up the callbacks has finished, which is the important part of Promise asynchrony.

This allows you to use promises that are asynchronous, in the sense of meeting all the Promises/A+ guarantees, but which still ensure that the IndexedDB transaction isn't closed. So you still get all the benefits of callbacks not happening "all at once".

The catch of course is that you need libraries that support this, and not every Promise implementation exposes a way to specify a scheduler or to flush its callback queue. Likewise, I'm not aware of any open source IndexedDB wrappers that have support for this.

If you are writing your own IndexedDB wrapper with Promsies, though, it would be good for it to use an appropriate Promise implementation, and flush its callback queue accordingly. One easy option would be to embed one of the many "micropromise" implementations that are only 100 lines or so of Javascript, and modify it as needed. Alternately, using one of the larger mainstream Promise libraries with custom scheduling support would be doable.

Do not use a synchronous promise library, the synchronous Bluebird build, or a synchronous scheduler. If you do that, you might as well abandon promises altogether and use straight callbacks.

Follow-up note: one commenter suggests that a synchronous promise is as safe as flushing a callback queue. But they are wrong. Horribly, horribly wrong. You can reason about a single event handler well enough to say "there isn't any other code running here; it's okay to invoke the callbacks now". To make a similar analysis with synchronous promises requires a complete understanding of how everything calls everything else... which is precisely opposite from the reason you want promises in the first place.

In the specific sync-promise implementation, the sync-promise author claims that their promise library is now "safe", and doesn't "release Zalgo". They are once again wrong: it isn't safe, and does release Zalgo. The author apparently did not actually understand the article about "releasing Zalgo", and has successfully reimplemented jQuery promises, which are widely considered horribly broken for a number of reasons, including their Zalgo-ness.

Synchronous promises are simply not safe, no matter your implementation.

PJ Eby
  • 8,760
  • 5
  • 23
  • 19
  • Thanks! Can you be a little more explicit for a newb like me? Taking Bluebird as an example... [it does let you specify a scheduler](https://github.com/petkaantonov/bluebird/blob/master/API.md#promisesetschedulerfunction-scheduler---void). Is that sufficient to ensure an IDB transaction will not be closed? If so, what is the implementation (paldepind below says [the default one](https://github.com/petkaantonov/bluebird/blob/master/src/schedule.js) doesn't work for IE11)? If not, what additional control would be needed, and is there an example of a library that provides that functionality? – dumbmatter Feb 10 '15 at 20:21
  • If you use a synchronous promise implementation in a controlled restricted manner I'd say it is far from a horrible idea. You introduce _exactly_ the same hazards in your code if you begin manually flushing the callback queue. Neither are optimal solutions. But, in the case where you're writing an IndexedDB wrapper you'd have to flush almost every single time you resolve a promise. Missed one? You've just introduce an error! Thus your solution actually increases both the amount of code and the potential for bugs. – paldepind Feb 11 '15 at 07:17
  • My [synchronous promise implementation](https://github.com/paldepind/sync-promise) library now includes a safety mechanism that makes it _safer_ than manually flushing the callback queue. – paldepind Feb 19 '15 at 13:44
  • You are wrong. It is not safer, it is fundamentally broken. Re-read the article on Zalgo, because you didn't understand it. Your library releases Zalgo because the `.then()` method can either call the callback "now", or "later". That is the *precise definition* of "releasing Zalgo"! In contrast, flushing a callback queue at a known point ensures that callbacks are always called "later", never "now". *That* is Zalgo-safe, your library is not, Q.E.D. your library is *not* safer than early-flushing the callback queue of a proper Promises/A+ implementation. – PJ Eby Apr 30 '15 at 06:22
  • 1
    @pjeby You're missing the point. It is not that SyncPromise should not release Zalgo (it does). The point is that it ensures that code using SyncPromise internally does not release Zalgo. – paldepind Jun 24 '15 at 12:24