jakearchibald / idb

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

error handling (what's the best way ?) #163

Closed novaknole closed 4 years ago

novaknole commented 4 years ago

Hi Jake.

I create my own class called utility where I have functions. Let me clarify two functions of that class.

async writeData(storeName, val, key) {
    const tx = this.db.transaction(storeName, 'readwrite');
    const store = tx.objectStore(storeName);
    store.put(val);
    return await tx.done;
  }

  async readAllData(storeName) {
    const tx = this.db.transaction(storeName, 'readonly');
    const store = tx.objectStore(storeName);
    return await store.getAll();
  }

Now, I use that class in my service worker or in my app. I import this class and use these methods .

Let's say something like this.

import IndexeddB from './utility.js'

let data = IndexedDB.readAllData('articles');

I am curious about this. If you look at the functions itself, do I have everything set up correctly? I mean I am not using onsuccess and onerror on the request. So return await tx.done (is it fair to say that this will automatically throw if there's an error and it won't throw an error if there's not one, to put this in a better way, where I use those functions, if I put them in try/catch block, it's all good and I am doing it right?

jakearchibald commented 4 years ago
async writeData(storeName, val, key) {
    const tx = this.db.transaction(storeName, 'readwrite');
    const store = tx.objectStore(storeName);
    store.put(val);
    return await tx.done;
  }

Fwiw, you can just do:

await db.put(storeName, val, key);

And:

const data = await db.getAll(storeName);

See https://github.com/jakearchibald/idb#shortcuts-to-getset-from-an-object-store.

I am curious about this. If you look at the functions itself, do I have everything set up correctly?

Yep, it's all correct, it's just avoiding some of the shortcuts.

I mean I am not using onsuccess and onerror on the request.

You don't need to 😄

So return await tx.done (is it fair to say that this will automatically throw if there's an error and it won't throw an error if there's not one, to put this in a better way, where I use those functions, if I put them in try/catch block, it's all good and I am doing it right?

Yep, although you don't really need the await there. See this for details https://jakearchibald.com/2017/await-vs-return-vs-return-await/.

Using the shortcut above:

try {
  const data = await db.getAll(storeName);
  console.log(data);
} catch (err) {
  console.error(err);
}

I'm closing this issue, but feel free to ask follow-up questions.

novaknole commented 4 years ago

Hi @jakearchibald , Long time, no see. I will basically post some questions here regarding the library.

As you said to me earlier, I will use the shortcuts.. I have this function in my helper class

async putData(storeName, val, key) {
    (key) ? await this.db.put(storeName, val)
          : await this.db.put(storeName, val, key);
  }

1) I am wondering if there's some kind of other way to do what i am doing there( Cleaner way). Because This function should work for all cases, and sometimes there's no need to pass key , this is why I do what I do in there. Could you think of a cleaner way or recommend leaving it as it is?

2) As we know, Indexeddb is transactional. Let's say that I wanted to add 3 things in indexeddb. and in my other class, I have the code :

var indexeddbHelper = new indexeddbHelper();
await indexeddbHelper.putData('objectstore1', {nice:123});
await indexeddbHelper.putData('objectstore1', {nice:456});
await indexeddbHelper.putData('objectstore2', {nice:999});

Is this gonna be transactional now? I mean I want all 3 to get stored or none of them. Not sure if this is the right way of doing it .

3) I have a json returned from axios request. I want to store it somewhere so that I can access it in my service worker. So What I am doing is I store that json with the custom key current. Seems like no complex searching is necessary because in service worker, i'd simply use get function of your library and get that json directly. Another approach would be to still use Cache API , but putting this in cache means that i have to create a Request as cache only accepts requests as key. then use serialize/deserialize and I'd still be able to work with this. Which way do you think is more worth? Would you still use indexeddb ?

jakearchibald commented 4 years ago
async putData(storeName, val, key) {
    (key) ? await this.db.put(storeName, val)
          : await this.db.put(storeName, val, key);
  }

I am wondering if there's some kind of other way to do what i am doing there( Cleaner way).

It seems a bit redundant to have one method that just calls another with the same args.

I guess putData is on a class/obj? Like this:

class Whatever {
  db = database;

  async putData(storeName, val, key) {
    key
      ? await this.db.put(storeName, val)
      : await this.db.put(storeName, val, key);
  }
}

Since the function is the same, you can just bind it:

class Whatever {
  db = database;
  putData = this.db.put.bind(this.db);
}

Or a more explicit option:

class Whatever {
  db = database;

  putData(...args) {
    return this.db.put(...args);
  }
}
var indexeddbHelper = new indexeddbHelper();
await indexeddbHelper.putData('objectstore1', {nice:123});
await indexeddbHelper.putData('objectstore1', {nice:456});
await indexeddbHelper.putData('objectstore2', {nice:999});

Is this gonna be transactional now? I mean I want all 3 to get stored or none of them.

No, the shortcut methods on db are for things that only have one operation. If you want multiple operations to be part of the transaction, you need to create a transaction:

const tx = db.transaction(['objectstore1', 'objectstore2'], 'readwrite');
tx.objectStore('objectstore1').put({ nice: 123 });
tx.objectStore('objectstore1').put({ nice: 456 });
tx.objectStore('objectstore2').put({ nice: 999 });
await tx.done;

Without this, it has no way of knowing which operations are part of which transaction.

I have a json returned from axios request. I want to store it somewhere so that I can access it in my service worker. So What I am doing is I store that json with the custom key current. Seems like no complex searching is necessary because in service worker, i'd simply use get function of your library and get that json directly. Another approach would be to still use Cache API , but putting this in cache means that i have to create a Request as cache only accepts requests as key. then use serialize/deserialize and I'd still be able to work with this. Which way do you think is more worth? Would you still use indexeddb ?

IDB sounds like a better option here.

novaknole commented 4 years ago

@jakearchibald I think it's clear now...

As you said, I have the following class:

class IndexedDB {

  constructor(version = idbConfig.version){
    this.db = null;
    this.version = version
  }

  async openDB(){
    let database;
    this.db = database = await openDB(idbConfig.database, this.version, {
      // if the local version is less than specified here, call upgrade func.
      upgrade(db) {
        // check if db contains table or not
        if(!db.objectStoreNames.contains(idbConfig.stores.config.name)){
          db.createObjectStore(idbConfig.stores.config.name, { autoIncrement: false } )
        }
      },

      blocking(){
        database.close()
      }
    })
  }

  putData(...args) {
    return this.db.put(...args);
  }
}

export default new IndexedDB();

Because I use webpack , it means that this class is singleton for my application code(wheverever I use it) and it's different for my service worker since the build for service worker is done by different vendor other than webpack.

Whenever I want to use indexeddb, I import this class in that file and then write something like this:

await IndexedDB.openDB()    
await IndexedDB.putData(config.app.indexeddb.stores.config.name, response.data, config.app.indexeddb.stores.config.cms_data); 

So, this code gets called everytime user refreshes the page... because data most of the time will be the same, it just replaces the same data with the same data. Nothing fancy.

1) Because I openDB the database, should I also close it after having used it? in this case , below putData, maybe i should have closing database code? or it doesn't matter at all (because everytime i refresh the page, it closes automatically? ) Any recommendations?

2) in my sw.js, I open it in the activate handler and then i use its functions in different events. Should I close it after having used it in one of the events of service worker ? but if I close it, then it means that for each event where I need it, I have to open it again.

jakearchibald commented 4 years ago
  1. Because I openDB the database, should I also close it after having used it?

If you don't need it again on this page (or, not for a long time), you can.

  1. in my sw.js, I open it in the activate handler and then i use its functions in different events. Should I close it after having used it in one of the events of service worker ? but if I close it, then it means that for each event where I need it, I have to open it again.

The service worker closes when it isn't in use. So, when you get a 'push' or 'fetch' event, it isn't guaranteed that the 'activate' event was also called within this context.

Fwiw, I wouldn't use a class in this case (what's the point of a class with just one instance), and instead I'd have data get/set method check to see if there's an active connection to the database, and if there isn't one, create one.

novaknole commented 4 years ago

@jakearchibald Makes sense. I will still use the class, but I won't depend on the activate handler. Thanks for all the information. Really do appreciate.

Sorry if my questions are getting out of hands ! just wanted to also know your opinion on something else. Still related to indexeddb but i guess this is more of a personal preference. I even made the bounty (+350) for that question as it seems, I am not getting any answers.

https://stackoverflow.com/questions/61559039/how-to-get-jwt-token-in-service-worker

novaknole commented 4 years ago

@jakearchibald have you taken a look at this ? if I don't ask too much ....

jakearchibald commented 4 years ago

I'm not really familiar enough with the push API, sorry.