-1

In javascript, I have a class here to handle requests. I want to make sure I have all the error handling set up ok. I want to open the connection to the db and close it on each request. I have this code so far

export default class IndexedDBStorage {

    #name:string;

    constructor(name:string) {
        this.#name = name;
    }

    private async GetDB():Promise<IDBDatabase> {
        return new Promise((resolve, reject) => {

            // https://javascript.info/indexeddb
            const request = window.indexedDB.open(this.#name, 1);

            request.onupgradeneeded = () => {
                //this.#db = request.result;
                if (!request.result.objectStoreNames.contains(this.#name)) {
                    request.result.createObjectStore(this.#name, {keyPath: 'id', autoIncrement:true});
                }
            };

            request.onerror = () => {
                reject("Why didn't you allow my web app to use IndexedDB?! " + request.error?.code);
            };

            request.onsuccess = () => {
                //request.result.onerror = () => {
                    // Generic error handler for all errors targeted at this database's requests!
                //    reject("Database error: " + request.error?.code);
                //};

                resolve(request.result);
            };
        });
    }

    async GetAllItems<T>():Promise<Array<T>> {
        return new Promise((resolve, reject) => {
            this.GetDB().then(db => {
                const transaction = db.transaction(this.#name);
                const store = transaction.objectStore(this.#name);
                const datarequest = store.getAll();
                db.close();
                datarequest.onsuccess = function() {
                    resolve(datarequest.result);
                };
                datarequest.onerror = function() {
                    reject("Error " + datarequest.error);
                }; 
            }).catch(err => {
                console.error(err);
            });
        });
    }
}

Also wanted to know, what is the difference between this part inside the GetDB function. This is the onerror event of the db itself, which is setup in the onsuccess event.

            //request.result.onerror = () => {
                // Generic error handler for all errors targeted at this database's requests!
            //    reject("Database error: " + request.error?.code);
            //};

And this part

            datarequest.onerror = function() {
                reject("Error " + datarequest.error);
            }; 

?

Thanks

EDIT:

export default class IndexedDBStorage {

    #name:string;

    constructor(name:string) {
        this.#name = name;
    }

    private async GetDB():Promise<IDBDatabase> {
        return new Promise((resolve, reject) => {

            const request = window.indexedDB.open(this.#name, 1);

            request.onerror = (event: Event) => {
                // this function catches all errors
                // all errors in database, transaction or request levels should be caught and handled there
                const errorEvent = event as Event & {error:string};
                console.error(errorEvent.error);
                alert(errorEvent.error);
                reject(errorEvent.error);
            };

            request.onblocked = function () {
                // this event shouldn't trigger if we handle onversionchange correctly
                // it means that there's another open connection to the same database
                // and it wasn't closed after db.onversionchange triggered for it
                const message = 'Database is currently blocked, page needs to reload, click ok to reload.';
                alert(message);
                window.location.reload();
                //console.log(message);
                //reject(message);
            };

            request.onupgradeneeded = (event: IDBVersionChangeEvent) => {
                const db = request.result;
                console.log('idb onupgradeneeded firing');
                switch(event.oldVersion) { // existing db version
                    case 0:
                      // version 0 means that the client had no database
                      // perform initialization
                      console.log('Database currently at version 0 which means that the client had no database');
                      console.log(`Upgrading to version ${db.version}`);
                      request.result.createObjectStore(this.#name, {keyPath: 'id', autoIncrement:true});
                    case 1:
                      // client had version 1
                      // update
                      console.log('Database currently at version 1 which means that the database already exists');
                }
            };

            request.onsuccess = () => {
                const db = request.result;
                db.onversionchange = function() {
                    db.close();
                    const message = "Database is outdated, page needs to reload, click ok to reload.";
                    alert(message);
                    window.location.reload();
                };
                resolve(db);
            };
        });
    }

    async GetAllItems<T>():Promise<Array<T>> {
        return new Promise((resolve, reject) => {
            this.GetDB().then(db => {
                const transaction = db.transaction(this.#name);
                const store = transaction.objectStore(this.#name);
                const datarequest = store.getAll();
                db.close();
                datarequest.onsuccess = function() {
                    resolve(datarequest.result);
                };
            }).catch(err => {
                reject(err);
            });
        });
    }
}
omega
  • 40,311
  • 81
  • 251
  • 474
  • 2
    Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! Create a `new Promise` only for each request, but use promise chaining otherwise. – Bergi Apr 05 '23 at 02:07
  • 1
    Actually this is a good example for the problems with that _"Promise constructor antipattern"_: The `GetAllItems` will never settle (neither resolve nor reject), if `this.GetDB()` rejects. – kca Apr 05 '23 at 16:36
  • @Bergi How can I fix that then? I don't really understand from your link how to fix this. – omega Apr 06 '23 at 20:48
  • 1
    Basically `return this.getDb().then(db => { return new Promise((resolve, reject) => { … }); }).catch(…)`. Although you probably [should drop the `.catch(console.error)`](https://stackoverflow.com/q/50896442/1048572) and if `getDb()` returns a database that always needs to be closed, you may want to use the [disposer pattern](https://stackoverflow.com/q/28915677/1048572). – Bergi Apr 06 '23 at 21:31

1 Answers1

3
  • requests can error, when a request errors it acquires an error property with a value
  • transactions can error, but the error will only be accessible from the one of the erring requests in the transaction
  • there is no big difference between the request to open the database and a request to get/put/add/etc. technically one is type IDBOpenRequest and one is type IDBRequest, i think IDBOpenRequest extends IDBRequest
  • if handling the transaction error, access the error via the event, because event.target will point to the request that erred, so event.target.error, aka request.error, is the error object, where as event is just the event and not the error itself
  • you can wrap the IDBOpenRequest in a promise, and you can wrap a request or transaction in a promise
  • generally avoid wrapping requests in promises except for the open request, always wrap the outer transaction, because of possible issues with promise microtasks and how indexeddb transactions time out in the event loop
  • because you catch the promise rejection in GetAllItems and the callback you are using for the catch is a void function, you are effectively suppressing the error and returning a promise that always resolves to either an array or undefined
  • you are not considering the blocked event that can happen in the open request, where essentially the promise sits unsettled indefinitely (until the database is unblocked because the concurrent version change transactions complete), so this code is not always going to work so well
  • i suggest not listening for errors that bubble up to the database, always listen for errors at the request level (except for when wrapping a transaction in a promise, in that case listen at transaction level but access the error via event.target.error, which points to the request in the transaction that erred), i highly suggest just ignoring the bubbling feature of indexeddb entirely
Josh
  • 17,834
  • 7
  • 50
  • 68