Unresolved Dexie bulkGet

[medmunds]

The code below fails when using fake-indexeddb, because at least one of the Dexie bulkGet operations never resolves:

import "fake-indexeddb/auto";
import Dexie from "dexie";

const db = new Dexie("test1");
db.version(1).stores({ table1: "++id" });
const table1 = db.table("table1");

const result = await Promise.all([
  table1.bulkGet([1]),
  table1.bulkGet([]),
  table1.bulkGet([3])
]);

I'm guessing this is a fake-indexeddb issue—or an interaction between it and Dexie—because the same Dexie code seems to work fine with Chrome and Firefox indexedDB.

Sandbox demonstrating problem: https://codesandbox.io/s/fakeidb-simultaneous-bulkget-w2j55?file=/src/index.test.ts. You should see tests failing with "Timeout - Async callback was not invoked within the 5000ms timeout specified by jest.setTimeout."

Same code running successfully with browser indexedDB: https://codesandbox.io/s/dexie-simultaneous-bulkget-test-qemz8?file=/src/index.test.ts

The bulkGet([]) with an empty key list is necessary to reproduce, and it must be preceded and followed by non-empty bulkGets. (Weird, right?) It doesn't seem to matter whether the bulkGets refer to records actually in the store or not.

Versions:
fake-indexeddb v3.1.1 (also 3.0.2 and 3.0.0; haven't tried others)
Dexie v3.0.1 (current latest; haven't tried others)

And in case they're relevant, I've reproed on:
node v10 or v12
typescript 3.9.7 targeting es2018, es2019, or es2020
jest v25 or v26
(I've included core-js in the sandbox, but it doesn't seem to matter; I believe TS is providing any missing core functions.)

[dumbmatter]

Thanks for the detailed bug report. I spent a little time playing around with it. Looks like each bulkGet is its own transaction, and each promise resolves when the transaction finishes. But in your test case, somehow the 3rd transaction never resolves.

The only difference I see in Dexie's code is that when no keys are given, it resolves the promise faster, because it doesn't have to wait for the async response from actually getting data. But I don't immediately know why that would be a problem. It still looks like the 1st is resolving, then the 2nd is resolving, and then the 3rd... never. Why would the 2nd resolving faster impact the 3rd? Strange. Gets into some hairy parts of fake-indexeddb, probably. I'll have to come back to this later.

[medmunds]

Thanks for the quick response. (And big thanks for fakeIndexedDB!)

Spent a little time in the debugger, and I think I see the problem:

• Database.processTransactions finds the next unfinished transaction, starts it, and then adds itself as a "complete" listener to continue the process with any remaining transactions
• If there are no remaining requests in the transaction, FDBTransaction._start immediately marks the transaction finished and dispatches the "complete" event
• Dexie's bulkGet([]) generates an FDBTransaction with no requests. This means the "complete" is dispatched before processTransactions has added its listener, so processTransactions is never run again to get to the remaining transactions.

I think that means the issue is better stated as: if there are ever transactions with no requests, any subsequent transactions may not be processed.

Not sure what the right fix would be. Maybe one of:

• In FDBTransation._start, delay the dispatch(complete) until the next cycle?
• In Database.processTransactions, attach the event listeners before executing next._start?
• In Database.processTransactions, check if executing next._start has left next finished, and if so immediately re-schedule processTransactions?

[dumbmatter]

Man, I'm jealous of your coworkers if this is the kind of bug reports and pull requests they routinely get from you! Really great stuff here.

I think you're right about the cause of the problem, and that the change you made in your PR is the solution least likely to cause other problems, since it's the most similar to how it works now.

I included this fix in v3.1.2.