done promise semantics is confusing and may cause uncaught promise rejections

[golergka]

Is your feature request related to a problem? Please describe.

The way that done promise currently works causes uncaught errors for us. Let's take this piece of code as an example:

try {
  const tx = db.transaction(storeName, "readwrite");
  await tx.store.add(value);
  await tx.done;
} catch (error) {
  console.error(error); // we expect the errors to be caught here
}

From the documentation and just naive intuition about how the code works, I would expect that if add operation fails, I would catch the error. However, when I trigger this failure in Jest, I get this:

 FAIL  src/background/idb/storage.test.ts
  ● storage › IngestionStore › should not add duplicate events and log an error

    AbortError: AbortError

      at FDBTransaction.error (../../../node_modules/idb/build/index.cjs:70:32)
      at invokeEventListeners (../../../node_modules/fake-indexeddb/build/cjs/lib/FakeEventTarget.js:25:23)
      at FDBRequest.dispatchEvent (../../../node_modules/fake-indexeddb/build/cjs/lib/FakeEventTarget.js:97:11)
      at FDBTransaction._start (../../../node_modules/fake-indexeddb/build/cjs/FDBTransaction.js:179:19)
      at ../../../node_modules/fake-indexeddb/build/cjs/lib/Database.js:31:16
      at run (../../../node_modules/setimmediate/setImmediate.js:40:13)
      at runIfPresent (../../../node_modules/setimmediate/setImmediate.js:69:21)
      at Timeout.task [as _onTimeout] (../../../node_modules/jsdom/lib/jsdom/browser/Window.js:552:19)

This happens because done promise fails before the promise returned by .add(), which happens before the await tx.done line — and as a result, at this point in time, the done promise doesn't have any catch() handlers, which causes promise rejection to bubble up.

As a fix, I changed all of our usage to this:

try {
  const tx = db.transaction(storeName, "readwrite");
  await Promise.all([tx.store.add(value), tx.done])
} catch (error) {
  console.error(error); // we expect the errors to be caught here
}

This solves the problem. However, when I shared this with the team, everybody found this very confusing and unintuitive, and I think it makes bad developer experience. Moreover, this is not mentioned anywhere in the docs.

Describe the solution you'd like

It happens because done promise is created immediately when transaction gets created:

idb/src/wrap-idb-value.ts

Line 95 in eb2fc14

transactionDoneMap.set(tx, done);

Instead of this, I would suggest to change done from property to a method, and create (or retrieve from cache) this promise only when method is called. This, of course, would be a breaking change, but I think it would greatly improve DX and prevent the worst kind of errors: hidden errors throwing outside of the normal execution stack, which most developers don't test for.

[golergka]

Just for the record, I encountered it while mocking indexed-db with fake-indexeddb. It's theoretically possible this issue only surfaces with this mock and not real implementation because of different order of events. However, order of events is not part of the official IndexedDB specification anyway, so it shouldn't be relied on for correctness and DX.

[bogacg]

Moreover, this is not mentioned anywhere in the docs.

How about this section in the README.md?

I was looking for something as described in this question and saw yours first.

It seems everyone's expectation slightly differs from what we have here, including mine, but root cause may be due to indexedDB's awkwardness. Can't wait for its next version.