dexie / Dexie.js

A Minimalistic Wrapper for IndexedDB
https://dexie.org
Apache License 2.0
11.43k stars 641 forks source link

`TypeError: Cannot read properties of undefined (reading 'table')` - `PSD.trans` became `undefined` during execution inside a middleware #1709

Open FelixZY opened 1 year ago

FelixZY commented 1 year ago

(Rewrite of original submission for brevity and to include an error message which may help future googlers)

I have a Dexie instance with multiple custom middlewares. In some circumstances, I've run into the very cryptic TypeError: Cannot read properties of undefined (reading 'table'), which seems to be caused by PSD.trans becoming undefined at the following location:

https://github.com/dexie/Dexie.js/blob/c0b5573397136772ee0586e042aa4569fd237cc0/src/hooks/hooks-middleware.ts#L34


Original text:

I had a poorly written seed method which produced ~100 entries in ~7 separate, unawaited, bulkAdd calls (each for separate tables). These calls all passed through some middleware looking something like this:

const result = await table.mutate(req); // req is DBCoreAddRequest | DBCorePutRequest
const inserted = await table.getMany({
  keys: result.results!,
  trans: req.trans,
});
await core.table("otherTable").mutate({
  type: "add",
  trans: req.trans,
  values: inserted.map(it => ({obj: it})),
});
return result;

What I ran into was that PSD.trans would be undefined at the following location - but only when calling core.table("otherTable").mutate().

https://github.com/dexie/Dexie.js/blob/c0b5573397136772ee0586e042aa4569fd237cc0/src/hooks/hooks-middleware.ts#L34

This caused the first two statements (table.mutate(req) and table.getMany()) to work as expected but the third (core.table("otherTable").mutate()) to fail due to PSD.trans being undefined.

After rewriting my seed method with proper awaits and another method of generating data the problem went away. I'm posting this in the hopes that a more experienced maintainer may have an idea of how PSD.trans could become undefined. I'm thinking it's a bug since the error did cause my transaction to complete "partially" (since the first table.mutate(req) statement persisted data but the final core.table("otherTable").mutate() did not) but I'm also open to the possibility that it could just be my code that is bad :sweat_smile:

FelixZY commented 1 year ago

I've since run into this issue a few more times, not only in poorly written async code, and it typically causes ugly workarounds as I do not understand the root cause. It's unfortunately hard for me to provide reproduction examples.

@dfahlander is there any intuition on PSD.trans being undefined at this location you could provide?

https://github.com/dexie/Dexie.js/blob/c0b5573397136772ee0586e042aa4569fd237cc0/src/hooks/hooks-middleware.ts#L34

FelixZY commented 1 year ago

Further details from ongoing debugging session:

This seems to be triggered by a subtle interaction between two middlewares. I have middleware like this (still working on that minimal reproduction example):

new Dexie("myDatabase", {
  addons: [
    (db) => db.use(middleware1),
    (db) => db.use(middleware2),
  ]
});

It seems that if middleware1 uses async/await style promises, certain implementations (still trying to pin which down) of middleware2 can cause PSD.trans to become undefined:

Does not work

  db.use({
    stack: "dbcore",
    name: "middleware1",
    create: (core) => ({
      ...core,
      table(tableName) {
        const table = core.table(tableName);
        return {
          ...table,
          mutate: async (req) => await table.mutate(req),
        };
      },
    }),
  });

Does work:

  db.use({
    stack: "dbcore",
    name: "middleware1",
    create: (core) => ({
      ...core,
      table(tableName) {
        const table = core.table(tableName);
        return {
          ...table,
          mutate: (req) => table.mutate(req),
        };
      },
    }),
  });
FelixZY commented 1 year ago

There is indeed some difference when comparing the return value of table.mutate to await table.mutate.

mutate: (req) => {
  const returnVal = table.mutate(req)
  return returnVal;
},

Screenshot from 2023-06-15 21-01-08


mutate: async (req) => {
  const returnVal = await table.mutate(req)
  return returnVal;
},

Screenshot from 2023-06-15 21-02-47

dfahlander commented 1 year ago

ok good you found it. That explains it. The mechanisms of supporting async contexts across native await calls are not propagated in the middle stack but in transactions and live queries only. That explains it.

dfahlander commented 1 year ago

Middlewares need either to be promise based or transpiled down to es2016 currently.

FelixZY commented 1 year ago

Reproduction code achieved:

import Dexie, {
  DBCore,
  DBCoreMutateRequest,
  DBCoreMutateResponse,
  DBCoreTable,
  Table,
} from "dexie";
import { IDBKeyRange, indexedDB } from "fake-indexeddb";

// Configure IndexedDB mock
Dexie.dependencies.indexedDB = indexedDB;
Dexie.dependencies.IDBKeyRange = IDBKeyRange;

describe("Dexie Issue 1709 Reproduction", () => {
  async function cascadeDelete(
    core: DBCore,
    req: DBCoreMutateRequest,
    table: DBCoreTable
  ): Promise<DBCoreMutateResponse> {
    if (req.type !== "delete") return await table.mutate(req);

    if (table.name === "foreigns") {
      const childrenTable = core.table("children");

      for (let key of req.keys) {
        const { result: childKeys } = await childrenTable.query({
          trans: req.trans,
          query: {
            index: childrenTable.schema.getIndexByKeyPath("fId")!,
            range: {
              type: 1, // DBCoreRangeType.Equal
              lower: key,
              upper: key,
            },
          },
        });

        // The recursive call appears to be key
        await cascadeDelete(
          core,
          {
            type: "delete",
            trans: req.trans,
            keys: childKeys,
          },
          childrenTable
        );
      }
    }

    return await table.mutate(req);
  }

  test("Reproduce", async () => {
    const db = new Dexie("myDb", {
      addons: [
        (db) =>
          db.use({
            stack: "dbcore",
            name: "mutateUsingPromise",
            create: (core) => ({
              ...core,
              table(tableName) {
                const table = core.table(tableName);
                return {
                  ...table,
                  // remove async/await and the code runs without error
                  mutate: async (req) => await table.mutate(req),
                };
              },
            }),
          }),
        (db) =>
          db.use({
            stack: "dbcore",
            name: "cascadeDelete",
            create: (core) => ({
              ...core,
              transaction: (_, mode) =>
                core.transaction(["foreigns", "children"], mode),
              table: (tableName) => {
                const table = core.table(tableName);
                return {
                  ...table,
                  mutate: (req) => cascadeDelete(core, req, table),
                };
              },
            }),
          }),
      ],
    }) as Dexie & {
      foreigns: Table<{
        id: number;
      }>;
      children: Table<{
        id: number;
        fId: number;
      }>;
    };

    db.version(1).stores({
      foreigns: "id",
      children: "id, fId",
    });

    await db.foreigns.bulkAdd([
      {
        id: 1,
      },
      {
        id: 2,
      },
    ]);
    await db.children.bulkAdd([
      {
        id: 1,
        fId: 1,
      },
      {
        id: 2,
        fId: 1,
      },
      {
        id: 3,
        fId: 2,
      },
    ]);

    await expect(db.foreigns.count()).resolves.toBe(2);
    await expect(db.children.count()).resolves.toBe(3);

    await expect(db.foreigns.delete(1)).resolves.not.toThrow();
    await expect(db.foreigns.count()).resolves.toBe(1);
    await expect(db.children.count()).resolves.toBe(1);

    await expect(db.foreigns.delete(2)).resolves.not.toThrow();
    await expect(db.foreigns.count()).resolves.toBe(0);
    await expect(db.children.count()).resolves.toBe(0);
  });
});
dfahlander commented 1 year ago

I keep this issue open until the documentation of middlewares is updated with this limitation. Notice that native awaits are supported for user code - the limitation of avoiding native await goes for middlewares and hooks.

FelixZY commented 1 year ago

I wonder if the typescript types can be modified in some way to allow detection of mistakes like this - e.g. via https://typescript-eslint.io/rules/await-thenable/ :thinking:

FelixZY commented 1 year ago

For reference (and I find this a bit scary :sweat_smile: )

async (req) => await table.mutate(req) // does NOT work
async (req) => table.mutate(req) // OK
(req) => table.mutate(req) // OK