Replacing via Table.put() can result in null value

[gregor]

I have no exact steps to reproduce this but again and again we see this issue with different tables in our app:

In a few tables we rely on replacing full items instead of granularly updating them for convenience. According to the docs this is a supported procedure as an exisiting item in the table with the same primary will be replaced by this. It seems like this is not error prone though. Under some circumstances the item is not replaced by the new one. Instead you end up with a table entry for the primary key with a null value. This obviously leads to a number of data inconsistencies.

[dfahlander]

What method is being used? Collection.modify() or Table.put() ? What OS/Browser? If you use Collection.modify(callbackFn), and set this.value = somethingNew, make sure that this "somethingNew" var is not changed over time. For example, if you set it to a closure var from a for loop, the var could have changed when your callback is called. So make sure to set this.value = somethingThatCannotChangeOverTime.

[gregor]

We use Table.put(). One code path, that to my understanding should never result in a null within the database but where it shows:

https://github.com/wireapp/wire-webapp/blob/dev/app/script/event/NotificationService.js#L101-L109

 /**
   * Save last notifications id from storage.
   * @param {string} notification_id - Notification ID to be stored
   * @returns {Promise} Resolves with the stored record
   */
  save_last_notification_id_to_db(notification_id) {
    return this.storage_service.save(z.storage.StorageService.OBJECT_STORE.AMPLIFY, NotificationService.CONFIG.PRIMARY_KEY_LAST_NOTIFICATION, {value: notification_id});
  }
};

We always forward an object of {value: NEW_VALUE}. This can never be null. The worst that can happen is {value: undefined} meaning {} be passed on to:

https://github.com/wireapp/wire-webapp/blob/dev/app/script/storage/StorageService.js#L393-L407

  /**
   * Saves objects in the local database.
   *
   * @param {string} store_name - Name of object store where to save the object
   * @param {string} primary_key - Primary key which should be used to store the object
   * @param {Object} entity - Data to store in object store
   * @returns {Promise} Resolves with the primary key of the persisted object
   */
  save(store_name, primary_key, entity) {
    return this.db[store_name].put(entity, primary_key)
      .catch((error) => {
        this.logger.error(`Failed to put '${primary_key}' into store '${store_name}'`, error);
        throw error;
      });
  }

[dfahlander]

I assume you have defined your table schemas has having non-inbound keys, as you are providing the primary key as a second argument to put().

Following the Dexie.js code on what could possibly happen leads to two different paths. If your database has not registered any creating hooks or updating hooks you will end up in a simple call to IDBObjectStore.prototype.put(obj, key). If that is the case, the error lies within the browser implentation of indexedDB (you are using electron (=chromium), right?).

If you are utilizing hooks, the Dexie code does more complex things. Among others, it will clone your object prior to updating it.

So question is whether you are utilizing hooks or not?

If the bug lies in dexie, you might test using [bulkPut](http://dexie.org/docs/Table/Table.bulkPut()) in place of put() to see whether you face the same bug there (just to narrow down the issue).

[StabbarN]

We are also seeing this. We use Table.put(), no hooks and non-inbound keys. The put is invoked in a rather large transaction (4-10 affected tables).

So far we have seen it on iPads with iOS 9 and 10 with a shortcut to homescreen (thus UIWebView). Starting the webpage on an iPad with UIWebView is very common in our user group. On iPads we use indexeddbshim ( https://github.com/axemclion/IndexedDBShim )

It also seems to occur when user closes the app while the transaction is ongoing.

Probably something wrong with the implementation of iPad's uiwebview or indexeddbshim.

[dfahlander]

Thanks @magnus-staberg. As no hooks are being used, this has to be an IndexedDB implementation bug. In your case, you are using IndexeddbShim. It would be very interesting to know whether @gregor is also using IndexeddbShim.

[gregor]

@dfahlander No, we are not using any shims. Plain Chromium in Electron is what's driving Wire desktop applications.

[dfahlander]

@gregor is Wire app listening to dexie hooks?

[dfahlander]

Just mentioning it (even though it could not be the issue here): The object passed to Table.put() must not be changed over time, as the actual call to IDBObjectStore.put() may take place asynchronically. In the wire code supplied by @gregor, the object is entered explicitely so that could not be the case there.

It seems we are talking of two separate issues here with the same symptom. Regarding Electron, we have seen instability issues before when an app is upgraded (#271) detected by @bennyn at Wire. In this case, it seemed like two Electron processes competed for holding the IndexedDB storage file. Generally, if the underlying IDB implementation is instable, things like this could likely happen.

Regarding @magnus-staberg's issue, it could be a problem with IndexedDBShim. Dexie only calls IDBObjectStore.put() (when not using hooks). This can be proved by just looking at the Dexie.js source where the actual put() operation takes place (when no hooks are involved): Dexie 1.5 / Dexie 2.0 . Even if the contents of the object passed to Table.put() would be changed after the call, it can impossibly become null.

[gregor]

No, we do not use any dexie hooks within the Wire implementation. Since finding the upgrade related issues you mentioned, our Electron code as evolved and should not opening the main browser window in the second instace when upgrading preventing the previous issues we saw.

[j-mew-s]

Hi @dfahlander ,

First off thanks for maintaining Dexie, it has done a great job for us so far! I've recently started seeing some unusual data causing issues for our users. When we load up data from IndexedDB into our app, we migrate it if necessary. Something like:

await db
      .table('mytable')
      .toCollection()
      .modify((data, ref) => migrateData(data, ref))

Our migrateData() function would then read some of the current state, and do any modification as necessary before returning. However, when reading current state, like data['someProperty'], we get an error:

Cannot read property 'someProperty' of null

So it appears somehow modify is supplying a null to our function. The weird thing is, I tried to set up some data to let me test a fix (using the native IndexedDB API), but this store uses inline keys (an id field), so adding a null value to this store should never work anyway (as far as I understand).

Have you seen this kind of behaviour before? Does this point to an issue in Chromium's indexedDB implementation? (Our web app only supports Chromium-based browsers). In which case should I just filter out nulls and not worry about the cause?

We aren't using any shims and don't use any Dexie hooks. Happy to raise as a separate issue if it seems distinct - this issue is quite old after all.