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

Default transactions to abort on drop #32

Closed
Ekleog opened this issue Jan 12, 2024 · 3 comments
Closed

Default transactions to abort on drop #32

Ekleog opened this issue Jan 12, 2024 · 3 comments
Assignees
Labels
Released This issue's resolution is now available in a published release

Comments

@Ekleog
Copy link

Ekleog commented Jan 12, 2024

Hey!

Here's an idea for indexed-db-futures v0.5: what if transactions aborted on drop, instead of committing on drop?

This'd make them farther away from the indexed-db standard, for sure, but the indexed-db standard is based on callbacks, which are pretty different from futures anyway. And, in Rust, it's very easy to miss one early-return point, to make sure that returning Err from a function aborts the transaction.

TL;DR:

fn foo() {
    let transaction = [...];
    transaction.add_key_val(...).unwrap().await.unwrap();
    do_some_check(&transaction).await?;
    Ok(())
}

This will (AFAICT) commit the transaction if do_some_check were to return an Err. The behavior I'd expect from the code if just reading it intuitively, would be for the transaction to be aborted.

In order to get the behavior I'd instinctively expect, I need to use the following code, which is quite a bit less pleasant to both write and read:

fn foo() {
    let transaction = [...];
    transaction.add_key_val(...).unwrap().await.unwrap();
    if let Err(e) = do_some_check(&transaction).await {
        transaction.abort().unwrap();
        return Err(e);
    }
    Ok(())
}

WDYT about adding a commit(self) function to transaction, that'd commit it (ie. just drop it), and to have IdbTransaction's Drop implementation abort the transaction if it has not been explicitly committed?

Anyway, I'm just starting using indexed-db-futures, but it already seems much, much more usable than the web-sys version. So, thank you! :D

@Ekleog
Copy link
Author

Ekleog commented Jan 14, 2024

After some more investigation, I learned that this would be a nontrivial feature.

So I decided to go ahead and implement it, and this resulted in the indexed-db crate, that has a misuse-resistant API with transactions that always abort upon errors and never commit early.

Do you want to keep this issue open to track this feature inside indexed_db_futures, or should we close it? :)

@Alorel
Copy link
Owner

Alorel commented Jul 26, 2024

Hey, thanks for bringing this up and sorry I'd abandoned this crate for so long. Let's keep this issue open please - I'll want to do some refactors around the crate and it'll be helpful to have this issue for reference

Alorel added a commit that referenced this issue Oct 27, 2024
Closes #32

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
@Alorel Alorel self-assigned this Oct 27, 2024
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #32

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #32

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #32, #39

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #25, #32, #39

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #25, #32, #39

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #25, #32, #39

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #25, #32, #39

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #25, #32, #39

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #25, #32, #39

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
Alorel added a commit that referenced this issue Oct 27, 2024
Closes #25, #32, #39

BREAKING CHANGE: API redesigned for serde & Stream support & msrv bumped to 1.75. See [MIGRATING/v0.6.0.md](https://github.com/Alorel/rust-indexed-db/blob/v0.6.0/MIGRATING/v0.6.0.md) for details.
@Alorel Alorel closed this as completed Oct 27, 2024
@Alorel Alorel added the Released This issue's resolution is now available in a published release label Oct 27, 2024
@Alorel
Copy link
Owner

Alorel commented Oct 27, 2024

This issue has been included in v0.6.0 🎉

- Your friendly neighbourhood 🤖 semantic release bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released This issue's resolution is now available in a published release
Projects
None yet
Development

No branches or pull requests

2 participants