jakearchibald / idb

IndexedDB, but with promises
https://www.npmjs.com/package/idb
ISC License
6.2k stars 348 forks source link

Open Is Still Successful If Upgrade Throws Exception #294

Open bcwhite-code opened 1 year ago

bcwhite-code commented 1 year ago

If openDB() causes an upgrade() but that upgrade fails with an exception, it will still call the .then() clause rather than the .catch() clause.

For example, the following code seems okay at first glance:

        openDB(DB_NAME, DB_VERSION, {
            upgrade(db, oldVersion, newVersion, transaction, event) {
                if (oldVersion > 0 && oldVersion != newVersion) {
                    db.deleteObjectStore(DB_TILE_STORE)
                    oldVersion = 0
                }
                if (oldVersion == 0) {
                    db.createObjectStore(DB_TILE_STORE)
                }
            },
        }).then((db) => {
            this.scrimDb = db  // Success
        }).catch((err) => {
            this.scrimDb = null  // Failure
            Toaster.show("Failed to open storage.  Please reload the page.")
        })

However, if for some reason there is no existing DB_TILE_STORE storage then deleteObjectStore() will throw an exception yet the code will still reach the "success" case and set this.scrimDB = db. (This came to my attention because I originally did not have the oldVerision>0 condition and so the delete failed for any new user.)

This is perhaps just an effect of the underlying implementation but it's unexpected nonetheless. I've since put a try/catch around the delete to properly deal with the issue but it would have been easier to track down the problem had the openDB call failed instead of an attempted save later on.

I didn't check if the update was considered successful (i.e. the internal database version number is updated) or not.

BTW, thank you for this adapter! It makes things nice and clean.