Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using for-await loop results in PrematureCommitError #774

Open
dfahlander opened this issue Nov 15, 2018 · 8 comments
Open

Using for-await loop results in PrematureCommitError #774

dfahlander opened this issue Nov 15, 2018 · 8 comments

Comments

@dfahlander
Copy link
Collaborator

This is a breakout from #317

Original Message by by @dperetti:

For the records, I'm facing very weird PrematureCommitErrors in Electron 3.04 since I switched to using Babel 7. I'm able to reproduce it in my app but it involves very specific and non intuitive conditions, so I'm not sure I can reproduce it in a minimal example.
However, I traced my code and came to the conclusion it didn't not match the caveats mentioned here.
It rather looked like some sort of leak.
Since I had upgraded to Babel 7 right before and thought it was related, I tried to compile using the @babel/plugin-proposal-async-generator-functions plugin and it worked again!
So my understanding is that Babel 7 uses native Promises with Electron, and that they are not ready for prime time...

UPDATE:
Here is a tricky one !
I get PrematureCommitErrors when using for await on an empty iterable.

   // items = []
   for await (const s of items)) {
       await this.db.items.put(new ItemEntry({/* ... */}))
   }
   // ... more calls 
   await this.db.otherItems.put(new OtherItem({/* ... */))
}
// PrematureCommitError here when the transaction exits
@dfahlander
Copy link
Collaborator Author

@dperetti What type of async iterable is items? It is already known that no other async operations are permitted within a IndexedDB transaction, except for async tasks that will be resolve within the same micro tick, such as await Promise.resolve() or await Promise.all([]).

If items for example, represents a true async iterable, it would not survive an IDBTransaction according to spec.

@dperetti
Copy link

In my case it's just a plain Array (not even async).

@dfahlander
Copy link
Collaborator Author

dfahlander commented Nov 15, 2018

Ok, that's interesting. Is your for-await-loop the first thing that happens in your transaction scope?

Can you test what happens if you put a single line before the for..await loop:

db.transaction('rw', db.otherItems, async ()=>{
  await db.otherItems.count(); // Add this line before. Does that make the transaction survive?
  for await (const item of items) {
    await db.otherItems.put(...);
  }
}).catch( err => console.error(err));

@dperetti
Copy link

Nope it doesn't (see last example).
But, here is more crazy stuff as I'm trying to extract a minimal example from my real use case.
It turns out the number of times for await is invoked matters!
This is 100% reproductible in my project:

    // Two empty for await uses: works.
    await this.db.transaction('rw',
      this.db.items,
      async () => {
        try {
          for await (const dummy of []) {
          }
          for await (const dummy of []) {
          }
          await this.db.items.put(new Item(123))
     } catch...
    // Three empty for await uses: doesn't work !
    await this.db.transaction('rw',
      this.db.items,
      async () => {
        try {
          for await (const dummy of []) {
          }
          for await (const dummy of []) {
          }
          for await (const dummy of []) {
          }
          await this.db.items.put(new Item(123))
     } catch...
    // Doesn't work.
    await this.db.transaction('rw',
      this.db.items,
      async () => {
        try {
          await this.db.items.count()
          for await (const dummy of []) {
          }
          for await (const dummy of []) {
          }
          for await (const dummy of []) {
          }
          await this.db.items.put(new Item(123))
     } catch...

@dperetti
Copy link

dperetti commented Nov 15, 2018

Great news: this is reproducible in a fiddle !
Please look at line 29.
Run → PrematureCommitError: Transaction committed too early.
Remove a for await invokation.
Run → Works.

https://jsfiddle.net/b9vyc7du/

Error in Chrome & Firefox... but seems to work in Safari !

@dfahlander
Copy link
Collaborator Author

dfahlander commented Nov 15, 2018

Okay, thanks much!!

It seems to be a Dexie issue related to keeping zones between native await calls. If I am correct, you would not get the same issue if you'd transpile down the code to ES5 or ES6.

A workaround if you want to use for-await loops with native awaits, is to use the transaction instance instead of relying on the zone to access the transaction:

db.transaction('rw', db.friends, async tx => {
   for await (var a of [1,2,3,4,5,6]) {}
   for await (var a of [1,2,3,4,5,6]) {}
   for await (var a of [1,2,3,4,5,6]) {}
   await tx.friends.add({name: "blah", age: 41})
});

@dperetti
Copy link

dperetti commented Nov 15, 2018

Cool, using the transaction instance works and it's pretty clean ! But making Flowtype happy with it will be a new challenge :-).

@dfahlander
Copy link
Collaborator Author

I know, that argument is not part of the d.ts file in dexie@2.x even though it works. Dexie@3.x has it though also in its type definition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants