idb icon indicating copy to clipboard operation
idb copied to clipboard

Abort unfinished transaction on Drop

Open ShaddyDC opened this issue 3 years ago • 9 comments

It would be convenient if a transaction that wasn't explicitly flushed with done() or commit() instead automatically called abort() when dropped.

This would allow users to more easily bail with the ? operator without leaving uncommited, half-finished transactions around.

ShaddyDC avatar Nov 22 '22 15:11 ShaddyDC

I agree with the intention but indexed db transaction behavior is dependent a lot on each browser's implementation.

Specification for abort() says that all the pending requests will be aborted (which does not mean all the requests made in that transaction). Depending on the browser implementation and user code, transaction may get auto committed even before calling commit(). I'm not sure if there's an easy way to handle this.

Worth reading: https://stackoverflow.com/questions/27326698/indexeddb-transaction-auto-commit-behavior-in-edge-cases

This will definitely require some testing/trial-and-error on each browser to see the behavior.

devashishdxt avatar Nov 25 '22 11:11 devashishdxt

I wasn't aware of that behaviour, but it does make sense. Thanks for the link.

If I understand the answers from a linked question correctly, transactions may commit as soon as the callbacks for all requests finish. Given the way the crate waits for each request to finish before returning control to a caller, I can basically never assume that 2 consecutive actions on an object store don't have a commit occur between them? That's unfortunate.

Without the ability to resume execution from the callbacks then, I have to wonder how useful it even is to expose the transaction layer to a user that can only make limited use of it. Though if it does usually work as expected, maybe that's reason enough. I haven't done any testing yet.

A possible, albeit hacky workaround would be to explicitly not return from the callbacks, until the next request is started or the transaction is otherwise explicitly finished by the user. That might go against the spirit of being a light wrapper, but it would keep the transaction alive, and dropping would work as expected. However, given that transactions should not be kept active while doing other works such as web requests, it might cause other issues for unsuspecting users.

I'm just spitballing. You've clearly got a much better idea than me about how the internals work to judge what appoach might work. :)

ShaddyDC avatar Nov 25 '22 13:11 ShaddyDC

@ShaddyDC If someone wants fine grained control over callbacks, they can always use idb-sys crate which exposes raw indexed db API without async/await.

devashishdxt avatar Nov 26 '22 03:11 devashishdxt

I have to wonder how useful it even is to expose the transaction layer to a user that can only make limited use of it.

Most of the times, it works as expected. There're also tests for testing abort() of transaction. But, because indexed db implementation can choose to auto-commit a transaction (because of various reasons like doing IO in event loop), it is unreliable and does not guarantee that the transaction will be aborted.

Also, I'm not sure how to handle abort failures if we call abort() in Drop implementation.

devashishdxt avatar Nov 26 '22 03:11 devashishdxt

That makes sense. Should I close the issue or would you prefer to keep it open in case you or anybody else has any good ideas for it in the future?

ShaddyDC avatar Nov 26 '22 12:11 ShaddyDC

I think eventually this needs to be resolved. We can keep it open for now.

devashishdxt avatar Nov 26 '22 12:11 devashishdxt

Hmm… maybe an idea: what if Transaction were not a type, but a function that takes a Future?

The usage would then be something like (pseudo-code for clarity):

db.transaction(&["store"], ReadWrite, |transaction| async move {
    transaction.get("store", "key").await?;
    transaction.put("store", "key", "value").await?;
}).await?;

Then, the various transaction functions would:

  • submit a request to the IdbTransaction
  • make the success and error callbacks both poll the provided future exactly once, to let it make progress
  • assert that polling the provided future once results to either the future completing (thus committing the transaction), or a new request being submitted to transaction. If the assertion fails, it aborts the transaction before actually panicking

WDYT? This is still a bit hand-wavy, but I'm thinking it could make sense to have something like that :)

Ekleog avatar Jan 12 '24 20:01 Ekleog

Well, I gave it some more thought, and then decided to go ahead and implement it. It's now available at https://crates.io/crates/indexed-db ; though I have not implemented cursors yet — I'll first integrate it with my personal projects, check that the APIs are reasonably easy to use, and implement cursors after that :)

Ekleog avatar Jan 14 '24 02:01 Ekleog

I can confirm that the API of indexed-db (as of version 0.2.2, I had to make some changes after trying to use it a bit more) seems to work fine for me, and AFAICT prevents any invalid use of transaction that'd cause it to auto-commit.

Hope this helps :)

Ekleog avatar Jan 14 '24 19:01 Ekleog