rezonant / dibello

A high-level API on top of IndexedDB
MIT License
3 stars 1 forks source link

Why other async operations are disallowed within transaction? #3

Closed rafis closed 6 years ago

rafis commented 6 years ago

Help me please to understand why the following code does not work?

describe('Database with foreign key relations', function() {

    it("transaction should not break by third-party asynchronous operations like delay", async function() {

        const db = await Database.open('test123', {
            version: 2,
            migrations: {
                "1": function(schema) {
                    schema.createStore('trees')
                        .id('id')
                        .key('color')
                        .field('history');
                },
                "2": function(schema) {
                    schema.createStore('apples')
                        .id('id')
                        .foreign('tree_id', 'trees.id')
                        .field('size');
                }
            }
        });

        await db.transact('readwrite', async function(trees, apples) {

            const result = await apples.find({ size: 'average' });
            const item = await result.next();
            let apple = item.value;

            if (!apple) {
                let tree = {
                    id: '1',
                    color: 'red'
                };
                await trees.persist(tree);
                apple = {
                    id: '1',
                    tree_id: tree.id,
                    size: 'average'
                };
            }

            function delay(ms) {
                return new Promise(resolve => setTimeout(resolve, ms));
            }

            await delay(1); // this breaks the test

            await apples.persist(apple);

        });

    });

});

Without await delay(1); the test works.

rezonant commented 6 years ago

Interesting, and unfortunate, but this looks like an artifact of how IndexedDB works:

https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB#Adding_data_to_the_database

Now that you have a transaction you need to understand its lifetime. Transactions are tied very closely to the event loop. If you make a transaction and return to the event loop without using it then the transaction will become inactive. The only way to keep the transaction active is to make a request on it. When the request is finished you'll get a DOM event and, assuming that the request succeeded, you'll have another opportunity to extend the transaction during that callback. If you return to the event loop without extending the transaction then it will become inactive, and so on. As long as there are pending requests the transaction remains active. Transaction lifetimes are really very simple but it might take a little time to get used to. A few more examples will help, too. If you start seeing TRANSACTION_INACTIVE_ERR error codes then you've messed something up.

After looking at your issue here I worried that the find() example which uses an inner query may not work correctly, but I think it still would because the async operation would be an IndexedDB one, and so the transaction would not become inactive due to that.

But any other async operation would cause the transaction to close. The best way to handle this would be to initiate another transaction if you need to perform an asynchronous operation.

Perhaps there could be some functionality we could add to Dibello to make this a smoother process, like perhaps introducing a Dibello-specific transaction injectable which would be capable of starting a new transaction without re-establishing your stores/injectables (ie apples, trees). But I'm not sure if this is the direction we should go in, since it sort of modifies the definition of what a transaction is in Dibello versus what IndexedDB does underneath the covers.

For now you would have to do:

...
await delay(1);
db.transact('readwrite', async (apples) => apples.persist(apple));
rafis commented 6 years ago

Thank you. Now I see, IndexedDB is not perfect as I thought. I do not understand why they have decided to bind IndexedDB to the DOM, DOM event system. Sometimes I think that perhaps using localStorage is better choice for me.

Did you get these errors while developing the library and debugging its code? Because I'm getting 2-3 types of similar error while debugging inside db.transact(). But if I don't have breakpoints inside transaction logic then I'm receiving the errors more rarely, so it is doable.

I haven't look deeply into tests and I don't have much experience developing javascript tests, but using IndexedDB mock disturbs me, such a feeling that using mocks are not testing the system holistically. I will try to look deeper if I will have time.