dexie / Dexie.js

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

toArray returns array with null element #2022

Open StephanBijzitter opened 1 week ago

StephanBijzitter commented 1 week ago

Dexie 3.2.7

Given the following code:

        dexie.queue
            .where('userId').equals(authenticatedUserId)
            .limit(100)
            .toArray().then((entries) => entries.map(({id, action, timestamp, resourceTableName}) => ({
                id: id ?? -1,
                action,
                timestamp,
                resourceTableName
            })))

I get the following error (only for one user, for everyone else it works perfectly fine)

TypeError: Cannot destructure property 'id' of 'e' as it is null.

(The build process moves the destructuring assignment to the function body, replacing the function params with an object called e)

So that means the array entries is [null], But I have trouble reproducing this as dexie won't let me insert null items into the queue table. How can this happen, and how do I get rid of these null items?

dfahlander commented 1 week ago

It is possible to insert null values into indexedDB but they would never be returned when querying on an index like this code shows. I would say that it is impossible that this could happen unless there would be a bug in the indexedDB implementation that I never heard of. Are you sure the error comes from this part of the code?

StephanBijzitter commented 1 week ago

Yeah, similar code* is present in some other parts of the application and the stacktraces all point to either the destructuring of the null entry or accessing the id property on a null entry.

  • one being for example the same query, but using forEach instead of toArray

The fact that it crashes isn't really a big issue to me, but I would need to somehow remove the null entries from the table as I also got a count on that and it's (probably?) being inflated, messing up quite a bit of logic. Removing the table itself and re-creating it is not an option as this is essentially the only table that is critical in our entire application >.<

Other example, that does not result in the error but simply never enters the while-loop's body:

        while (entry = await dexie.queue
            .where(['userId', 'timestamp'])
            .between([userId, 0], [userId, Date.now()], true, true)
            .first()) {

Which makes sense, as that would equate to while (null) {}

StephanBijzitter commented 1 week ago

I think..... this should work to clear out the queue table. right?

        while ((entry = await dexie.queue
            .where(['userId', 'timestamp'])
            .between([userId, minKey], [userId, maxKey], true, true)
            .first()) !== undefined
        ) {
            if (entry === null) {
                captureException(new Error('Encountered null entry in queue during upload'));
                await dexie.queue
                    .where(['userId', 'timestamp'])
                    .between([userId, minKey], [userId, maxKey], true, true)
                    .limit(1)
                    .modify((value, ref) => {
                        if (value === null) { // Just to be absolutely sure that we're never deleting customer data
                            delete (ref as Partial<{value: QueueEntry;}>).value;
                        }
                    });
                continue;
            }
            // actual logic with entry
dfahlander commented 1 week ago

Yes, but I'm still confused that a null entry could get there. Is the query performed in a liveQuery() If so, this could indicate a bug in Dexie's cache (which is only active in liveQuery contexts) - and if that would be the case, removing the null entries wouldn't lead anywhere. A better workaround if so, would be to filter them out from the result.

In the case of indexeddb bug, the code you show should delete the entry.

StephanBijzitter commented 1 week ago

The place where we show a count of entries in the queue relies on dexie-observable with a simple count, so it doesn't crash on this as the entries themselves aren't used. The part I just showed you is executed onClick.

I'll report back once I know more :-)