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

create/deleteObjectStore: InvalidStateError logic does not match impls/tests #192

Closed
inexorabletash opened this issue Apr 20, 2017 · 3 comments
Assignees
Milestone

Comments

@inexorabletash
Copy link
Member

Spec has:

Let transaction be the upgrade transaction associated with database. If one does not exist or it is finished, throw an "InvalidStateError" DOMException.

But in this case, a TransactionInactiveError is thrown instead in Chrome/Firefox and asserted as such in tests:

tx.abort();
db.createObjectStore('s'); // throws TransactionInactiveError

Noted by the ever vigilant @brettz9 over in web-platform-tests/wpt#5612 but it's the spec that's wrong here since impls/tests match.

Unfortunately, the "finished" notion for transactions is a bit squishy. Spec-wise we may be able to just drop the "or it is finished" bit from these methods and then tidy up the transaction lifecycle at some point in the future.

@inexorabletash inexorabletash added this to the V2 milestone Apr 20, 2017
@inexorabletash inexorabletash self-assigned this Apr 20, 2017
@inexorabletash
Copy link
Member Author

I think to fix this properly will require:

  • A database may have an associated upgrade transaction, initially null
  • Set database's associated upgrade transaction in the steps for running an upgrade transaction
  • In the steps to commit/abort a transaction, in the queued task just before we fire the event, we clear the database's associated upgrade transaction

This is because right after calling abort() we would consider the transaction "finished" but yet implementations still give TransactionInactiveError. Implementations still consider the database to have an associated upgrade transaction until the commit/abort actually fires.

The current "... the upgrade transaction associated with database..." text is squishy since a transaction always has an associated database even after it is finished. So that would invert to "... the database's associated upgrade transaction..."

@brettz9
Copy link
Contributor

brettz9 commented Apr 21, 2017

Btw one nit, FWIW, in regard to TransactionInactiveError in another context...

In "steps to asynchronously execute a request", the step "If transaction is not active, throw a "TransactionInactiveError" DOMException." can I think be an assertion as a check appears to always be made before running the steps.

@inexorabletash
Copy link
Member Author

Yep, that's correct.

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

No branches or pull requests

2 participants