Catch and emit error from idb.openDB

[jpobley]

Firefox Private Browsing disables IndexedDb by default, which causes an error. We need to catch and emit that error so clients using IndexedDbProvider can do something about it.

It turns out there is a workaround, but if we don't catch the error, we can't suggest it.

[jpobley]

I should also point out that one can also await and catch the error via indexedDbProvider.whenSynced, so this PR isn't totally necessary.

[dmonad]

Thanks for this @jpobley!

I think that whenSynced should indeed be a rejected promise when the initialization fails for some reason. It is also idiomatic to throw an error as you do.

Could you please do a small modification and change this to:

this.whenSynced = new Promise(..)
this.whenSynced.catch(err => { this.emit('error', ..) })

Otherwise this.whenSynced is resolved because the error is caught.

This issue is fairly preventable. The example should probably show that you only create a y-indexeddb provider when IndexedDB is supported and enabled. Not sure how one can check for that.

[jpobley]

Cool. Updated. I wasn't entirely sure what you meant by your request, so lemme know if I misunderstood it.

This issue is fairly preventable. The example should probably show that you only create a y-indexeddb provider when IndexedDB is supported and enabled. Not sure how one can check for that.

Yeah, you'd think there would be a flag set or something, but I haven't found anything. One idea off the cuff might be updating lib0/indexeddb.js to have an isEnabled() export that resembles:

import {uuidv4} from './random.js'

export const isEnabled = (function checkIdbEnabled() {
    let idbEnabled;

    const name = `lib0-idb-enabled-check-${uuidv4()}`;
    const request = indexedDB.open(name)
    request.onerror = e => idbEnabled = false
    request.onsuccess = e => {
        e.target.result.close()
        indexedDB.deleteDatabase(name)
        idbEnabled = true
    }

    return () => idbEnabled
})()

[dmonad]

What is this change about?

[jpobley]

Ah, I guess I misunderstood your ask. Above, you requested this change:

this.whenSynced = new Promise(..)
this.whenSynced.catch(err => { this.emit('error', ..) })

Which I interpreted to mean that you wanted this.whenSynced to be a new Promise instead of the Promise returned from this._db.

What would you like me to do instead?

[dmonad]

Ah, got it. Don't take my above comment too literally. The second part this.whenSynced.catch(err => { this.emit('error', ..) }) is fine. The first part adds five extra lines of code with no apparent benefit. Please rework the PR to include a minimal amount of changes.