1

I have a function that is used to add a record to the IndexDb database:

async function addAsync(storeName, object) {
    return new Promise((res, rej) => {
        // openDatabaseAsync() is another reusable method to open the db.  That works fine.
        openDatabaseAsync().then(db => {
            var store = openObjectStore(db, storeName, 'readwrite');
            var addResult = store.add(JSON.parse(object));
            addResult.onsuccess = res;
            addResult.onerror = (e) => {
                console.log("addResult Error");
                throw e;
            };
        }).catch(e => {
            // Error from "throw e;" above NOT GETTING CAUGHT HERE!
            console.error("addAsync ERROR > ", e, storeName, object);
            rej(e);
        });
    })
}

If I try to add a duplicate key, then I expect:

addResult.onerror = (e) => {
    console.log("addResult Error");
    throw e;
}

to capture that. It does.

But then, I also expect my

.catch(e => {
    // Error from "throw e;" above NOT GETTING CAUGHT HERE!
    console.error("addAsync ERROR > ", e, storeName, object);
    rej(e);
})

to catch that error. But instead I get an "uncaught" log.

Console output:

addResult Error
Uncaught Event {isTrusted: true, type: "error", target: IDBRequest, currentTarget: IDBRequest, eventPhase: 2, …}

Does that final .catch only handle exceptions from the openDatabaseAsync call? I would have thought now as it is chained to the .then.

In summary, here's what I would expect from the above code:

  • If openDatabaseAsync() fails then I'm not catching that so the error would be sent to the caller of addAsync().
  • If .then fails then I expect the .catch to catch it, log the error and then reject the promise meaning that the called of addAsync() would need to handle that.

However, I would have thought that I should get the log from the line:

console.error("addAsync ERROR > ", e, storeName, object);

before the reject is sent back to the caller of addAsync(), which may be unhandled at that point.

Neil W
  • 2,112
  • 1
  • 14
  • 20
  • The thing that's uncaught would be the `addAsync().catch()` thing, since you're rejecting in the promise that gets returned from `addAsync()`. Do you have a catch on the result from that call? – TKoL Mar 01 '21 at 13:25
  • THanks @TKoL. The answer to your question is "No". It's called by Blazor JSInterop but that's something I can move on to. But, to your point, I don't get the log `"addAsync ERROR > ..."` so that tells me that the `throw e;` is not getting caught by the `.catch`, which is where I'm confused at the moment. I can move on to external handling from the caller of `addAsync()` subsequently. – Neil W Mar 01 '21 at 13:29

1 Answers1

3

Your approach would benefit form a larger overhaul.

  • Generally, don't write a function as async when it's not also using await.
  • Don't use new Promise() for an operation that returns a promise, such as openDatabaseAsync() does. Return that promise, or switch to async/await.
  • It would be useful to wrap IndexedDB operations so that they follow promise semantics.

On the example of IDBRequest:

function promisifyIDBRequest(idbr) {
    return new Promise( (resolve, reject) => {
        idbr.onsuccess = () => resolve(idbr.result);
        idbr.onerror = (e) => reject(e.target.error);
    });
}

Now you can do this:

async function addAsync(storeName, object) {
    const db = await openDatabaseAsync();
    const store = openObjectStore(db, storeName, 'readwrite');
    return promisifyIDBRequest(store.add(JSON.parse(object)));
}

Add a try/catch block if you want to handle errors inside of addAsync().

It's worth checking out existing solutions that wrap the entire IndexedDB interface with promise semantics, such as https://github.com/jakearchibald/idb.


FWIW, the promise-chain variant of the above function would look like this:

function addAsync(storeName, object) {
    return openDatabaseAsync().then( (db) => {
        const store = openObjectStore(db, storeName, 'readwrite');
        return promisifyIDBRequest(store.add(JSON.parse(object)));
    });
}

both variants return a promise for an IDBRequest result.

Tomalak
  • 306,836
  • 62
  • 485
  • 598
  • Thank you, indeed! Some food for thought in there. One question. That appears to be a generic "promisifier". Should I therefore be sending the function rather than the result, i.e. `return await promisifyIDBRequest(() => store.add(JSON.parse(object)));`? – Neil W Mar 01 '21 at 13:54
  • Or that first code block should be: `idbr.onsuccess = () => resolve(idbr.result)`? Then that promisifyIDBRequest function is a generic handler for all IDB requests that have "onsuccess" and "onerror" properties? – Neil W Mar 01 '21 at 13:58
  • @NeilW No, `promisifyIDBRequest()` expects as input what `store.add()` returns, and yes, it could be a generic promisifier for anything that has the `onsuccess`/`onerror` interface, but in this particular case, the `onsuccess` handler expects `irbr.result` for promise resolution, so it's tied into that as well. – Tomalak Mar 01 '21 at 14:02
  • @NeilW Of course I had stupid typos in my `promisifyIDBRequest()` function, I trust you've seen and corrected them on your own by now. :) – Tomalak Mar 01 '21 at 14:48
  • 1
    I have. And many thanks again. Apart from tidying up my IndexedDb interactions, the reason I was trying to do what I was digging here was because I was not getting JS error info back to .NET via JSInterop. Another key learning is that when rejecting a promise from .onerror, I am better off rejecting (e.target.error) and not just (e). Otherwise the .NET Exception.Message is just `[object Event]`! Thanks again. – Neil W Mar 01 '21 at 16:00
  • @NeilW Good to hear you've sorted it all out! – Tomalak Mar 01 '21 at 16:13
  • ...also, the `return await` wasn't right, I've you still have that in your code, convert it to a plain `return`. – Tomalak Mar 01 '21 at 16:15
  • Re your last comment about `return await`. I can understand that it is redundant on the basis that the caller of `addAsync` will `await` it. To me ... Your original code means they are awaiting the result of `addAsync`. Your revised code means they are essentially awaiting the result of `promisifyIDBRequest`. Have I understood correctly? If so, I understand the redundancy of the extra await, but is there any bigger fallout of leaving the extra await in there? – Neil W Mar 01 '21 at 16:42
  • 1
    @NeilW Yes, it's redundant. `return await somePromise();` and `return somePromise()` are essentially the same operation, but `return await` adds one extra step to the overall operation. There even is an ESLint rule for that: [`no-return-await`](https://eslint.org/docs/rules/no-return-await). It can become useful as soon as you add that `try/catch` block in `addAsync`, nicely outlined in https://jakearchibald.com/2017/await-vs-return-vs-return-await/. – Tomalak Mar 01 '21 at 20:23
  • @NeilW Oh, and on a different note, the `JSON.parse()` in `addAsync()` is a bit of a wart. It would be useful to move that outside. `store.add()` accepts any value, accepting only JSON-parseable strings in the wrapper function is a needles limitation. – Tomalak Mar 01 '21 at 22:45
  • Thanks for your continued advice. I've removed the redundant awaits now.. And regarding the `JSON.parse()`. Scenario is that `addAsync()` is being called from C# Blazor WASM via JSInterop, so my C# IndexedDb abstraction takes a C# type, serializes it and then calls `JS.addAsync()`. When retrieving, it calls a `JS.getByKeyAsync()` method (which stringifies the data out of the IndexedDb) and then C# abstraction deserializes it back into the C# type. Can't remember why I ended up `JSON.Parse()` on way in and `JSON.Stringify()` on way out. Maybe coz of keyPath? Will have a play. – Neil W Mar 02 '21 at 13:06
  • @NeilW Yeah, it was just a random thought, you have it well in hand anyway. – Tomalak Mar 02 '21 at 15:10