Validate exception ordering against implementations #11

inexorabletash · 2015-05-18T19:42:19Z

The various "If .... throw ...." statements were left in order from previous iterations of the spec which asserted behavior but didn't define it as formal steps.

We need to verify that the steps match the implementations in the case of conflicts, and reorder if necessary. If implementations disagree, file implementation bugs as necessary.

NOTE: Checkboxes below indicate that at least 2 implementations and the spec are in alignment.

inexorabletash · 2015-06-01T22:17:21Z

IDBDatabase

Normal Methods

transaction
- WPT: upstream blink's idbdatabase-transaction-exception-order.html
- FF: InvalidStateError (closed/version) > InvalidAccessError (empty) > NotFoundError > TypeError (mode)
- Cr: InvalidStateError (closed/version) > InvalidAccessError (empty) > NotFoundError > TypeError (mode)

Schema Mutations

createObjectStore:
- WPT: upstream blink's idbdatabase-createObjectStore-exception-order.html
- FF: InvalidStateError > TransactionInactiveError > SyntaxError > ConstraintError > InvalidAccessError
- Cr: InvalidStateError > TransactionInactiveError > SyntaxError > ConstraintError > InvalidAccessError
deleteObjectStore
- WPT: upstream blink's idbdatabase-deleteObjectStore-exception-order.html
- FF: InvalidStateError > TransactionInactiveError > NotFoundError
- Cr: InvalidStateError > TransactionInactiveError > NotFoundError

inexorabletash · 2015-06-01T22:34:30Z

IDBObjectStore

Normal Methods

add/put/delete:
- WPT: upstream blink's idbobjectstore-add-put-exception-order.html
- WPT: upstream blink's idbobjectstore-delete-exception-order.html
- FF: InvalidStateError (deleted) > TransactionInactiveError > ReadOnlyError > DataError
- Cr: InvalidStateError (deleted) > TransactionInactiveError > ReadOnlyError > DataError
clear
- WPT: upstream blink's idbobjectstore-clear-exception-order.html
- FF: InvalidStateError (deleted) > TransactionInactiveError > ReadOnlyError
- Cr: InvalidStateError (deleted) > TransactionInactiveError > ReadOnlyError
get/getAll/getAllKeys/count/openCursor/openKeyCursor
- WPT: upstream blink's idbobjectstore-query-exception-order.html
- FF: InvalidStateError (deleted) > TransactionInactiveError > DataError
- Cr: InvalidStateError (deleted) > TransactionInactiveError > DataError

Schema Mutations

createIndex
- WPT: IndexedDB/idbobjectstore_createIndex14-exception_order.htm
- FF: InvalidStateError (mode) > InvalidStateError (deleted) > TransactionInactiveError > ConstraintError > SyntaxError > InvalidAccessError
- Cr: InvalidStateError (mode) > InvalidStateError (deleted) > TransactionInactiveError > ConstraintError > SyntaxError > InvalidAccessError
deleteIndex
- WPT: upstream blink's idbobjectstore-deleteIndex-exception-order.html
- FF: InvalidStateError (mode) > InvalidStateError (deleted) > TransactionInactiveError > NotFoundError
- Cr: InvalidStateError (mode) > InvalidStateError (deleted) > TransactionInactiveError > NotFoundError

inexorabletash · 2015-06-01T22:54:09Z

IDBIndex

get/getKey/getAll/getAllKeys/count/openCursor/openKeyCursor
- WPT: upstream blink's idbindex-query-exception-order.html
- FF: InvalidStateError (deleted) > TransactionInactiveError > DataError
- Cr: InvalidStateError (deleted) > TransactionInactiveError > DataError

inexorabletash · 2015-06-01T23:03:41Z

IDBCursor

advance
- WPT: upstream Blink's idbcursor-advance-exception-order.html
- FF: TypeError > TransactionInactiveError > InvalidStateError (deleted) > InvalidStateError (got)
- Cr: TypeError > TransactionInactiveError > InvalidStateError (deleted) > InvalidStateError (got)
continue
- WPT: upstream Blink's idbcursor-continue-exception-order.html
- FF: TransactionInactiveError > InvalidStateError (deleted) > InvalidStateError (got) > DataError (key) > DataError (order)
- Cr: TransactionInactiveError > InvalidStateError (deleted) > InvalidStateError (got) > DataError (key) > DataError (order)
continuePrimaryKey
- WPT: IndexedDB/idbcursor-continuePrimaryKey-exception-order.htm
- FF: TransactionInactiveError > InvalidStateError (deleted) > InvalidAccessError (source) > InvalidAccessError (direction) > InvalidStateError (got) > DataError (key) > DataError (order)
- Cr: TransactionInactiveError > InvalidStateError (deleted) > InvalidAccessError (source) > InvalidAccessError (direction) > InvalidStateError (got) > DataError (key) > DataError (order)
delete
- WPT: upstream blink's idbcursor-delete-exception-order.html
- FF: TransactionInactiveError > ReadOnlyError > InvalidStateError (deleted) > InvalidStateError (got) > InvalidStateError (key cursor)
- Cr: TransactionInactiveError > ReadOnlyError > InvalidStateError (deleted) > InvalidStateError (got) > InvalidStateError (key cursor)
update
- WPT: upstream blink's idbcursor-update-exception-order.html
- FF: TransactionInactiveError > ReadOnlyError > InvalidStateError (deleted) > InvalidStateError (got) > InvalidStateError (key cursor) > DataError (key)
- Cr: TransactionInactiveError > ReadOnlyError > InvalidStateError (deleted) > InvalidStateError (got) > InvalidStateError (key cursor) > DataError (key)

inexorabletash · 2015-06-01T23:06:53Z

IDBTransaction

abort
- FF: InvalidStateError (finished)
- Cr: InvalidStateError (finished)
objectStore
- WPT: upstream blink's idbtransaction-objectStore-exception-order.html
- FF: InvalidStateError (finished) > NotFoundError
- Cr: InvalidStateError (finished) > NotFoundError

…/Cr - #11

odinho · 2015-06-16T07:42:20Z

Great. At one point we should also get some tests on these things to make it simpler to implement and follow. :)

inexorabletash · 2015-06-16T18:13:04Z

@velmont - Yes please! Writing tests for these is a bit intimidating, since some cases involve upgrades, many involve async behavior (e.g. trying to use a transaction in a non-IDB callback but before the transaction has committed), etc. And, of course, this is all about combinations of the above. Ugh!

I documented the Blink and Gecko behavior via code inspection, since that was easy.

If anyone wants to help with writing testharness.js-based tests for these please let me know and I'll assist as best I can. I'm also happy to tweak the Chrome ordering; the cursor APIs are particularly funky, but until we have tests I don't want to change anything.

inexorabletash · 2015-06-25T00:08:05Z

Updated notes above for FF based on code inspection, following comments by @benturner #23

…ls. Issue #11

inexorabletash · 2016-09-23T08:05:07Z

Need to update Chrome behavior descriptions following https://chromium.googlesource.com/chromium/src/+/dce205414aa1bde4125c3129fef24a8dc8ad32cc

inexorabletash · 2016-09-23T08:07:58Z

And tests there can be upstreamed to wpt

inexorabletash · 2016-10-12T17:13:58Z

Following a change yesterday in Chrome, FF and Cr match in all cases. \o/ But we still need tests - there are a bunch to upstream from blink (see WPT:... lines above) - mostly for "v2" stuff or where we have changed to align recently.

beidson · 2016-10-21T15:46:51Z

Order of exceptions thrown from renaming object stores and indexes differs

For IDBIndex, the name attribute’s setter runs these steps:
...
4. If transaction is not an upgrade transaction, throw an InvalidStateError.
5. If transaction is not active, throw a TransactionInactiveError.
6. If index or index’s object store has been deleted, throw an InvalidStateError.
7. If index’s name is equal to name, terminate these steps.
8. If an index named name already exists in index’s object store, throw a ConstraintError.
...

For IDBObjectStore, the name attribute's setter runs these steps:
...
4. If store has been deleted, throw an InvalidStateError.
5. If transaction is not an upgrade transaction, throw an InvalidStateError.
6. If transaction is not active, throw a TransactionInactiveError.
7. If store’s name is equal to name, terminate these steps.
8. If an object store named name already exists in store’s database, throw a ConstraintError.
...

The "if the is deleted..." exception is in different places for each of these, but should be in the same place.

I'm implementing these in WebKit now and really don't want to follow the spec on this. Could we pick one or the other?

inexorabletash · 2016-10-21T16:08:57Z

@bevis-tseng and @pwnall - what do you think about making the changes @beidson proposes? i.e. swap the order in one of the two methods?

pwnall · 2016-10-23T19:43:15Z

I'm fine with either order.

While building apps, I used to get thrown off by random errors while debugging my code, so I hope we can converge on some ordering. I can change Chrome to adopt any ordering Firefox and Safari would like to use.

bevis-tseng · 2016-10-24T02:04:08Z

FYI.
These were done in identical order in FF for both IDBObjectStore/IDBIndex [1][2]:
4. If transaction is not an upgrade transaction or if the (store|index) was deleted, throw an InvalidStateError.
5. If transaction is not active, throw a TransactionInactiveError.
6. If the name is identical, terminate these steps.
...

[1] http://searchfox.org/mozilla-central/rev/84075be5067b68dc6cb3b89f999645650e68c05b/dom/indexedDB/IDBObjectStore.cpp#2205-2219
[2] http://searchfox.org/mozilla-central/rev/84075be5067b68dc6cb3b89f999645650e68c05b/dom/indexedDB/IDBIndex.cpp#159-172

beidson · 2016-10-24T02:22:56Z

WebKit now matches Gecko's ordering.
I suggest we codify that.

beidson · 2016-10-24T16:55:40Z

For reference:
https://trac.webkit.org/browser/trunk/Source/WebCore/Modules/indexeddb/IDBIndex.cpp#L82
https://trac.webkit.org/browser/trunk/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp#L98

inexorabletash · 2017-01-13T21:32:05Z

WebKit now matches Gecko's ordering.
I suggest we codify that.

Somehow I missed that comment. Yay!

So far as I know WebKit/Gecko/Blink all match and those match the spec. If that's not the case please submit a web-platform-test case!

inexorabletash mentioned this issue May 21, 2015

Import issues from Bugzilla #15

Closed

14 tasks

inexorabletash added a commit that referenced this issue Jun 1, 2015

Tweak exception ordering for IDBTransaction.objectStore() to match FF…

abbfb43

…/Cr - #11

inexorabletash added a commit that referenced this issue Jun 25, 2015

Reorder exception priority for deleteIndex() to match Blink/Gecko imp…

6e6ed33

…ls. Issue #11

inexorabletash added feature request needs-tests labels Aug 10, 2015

This was referenced Jan 8, 2016

Various IndexedDB tests expect the wrong exception web-platform-tests/wpt#2455

Closed

Various IndexedDB tests expect the wrong exception for aborted transactions web-platform-tests/wpt#2456

Closed

inexorabletash mentioned this issue Apr 23, 2016

(More) Various IndexedDB tests expect the wrong exception web-platform-tests/wpt#2888

Closed

inexorabletash added the interop label Jun 24, 2016

inexorabletash added needs-pr and removed feature request labels Sep 23, 2016

inexorabletash added this to the V2 milestone Sep 23, 2016

inexorabletash mentioned this issue Oct 21, 2016

Order of exceptions thrown from renaming object stores and indexes differs #102

Closed

inexorabletash closed this as completed Jan 13, 2017

This was referenced Apr 4, 2017

IndexedDB IDBDatabase.transaction exception order issue (mode check should precede store check) web-platform-tests/wpt#5313

Closed

Align IDBDatabase.transaction() exception precedence with tests/implementations #188

Merged

xfq mentioned this issue Oct 11, 2017

Implementation report #213

Closed

Validate exception ordering against implementations #11

Validate exception ordering against implementations #11

Comments

inexorabletash commented May 18, 2015

inexorabletash commented Jun 1, 2015 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

IDBDatabase

Uh oh!

inexorabletash commented Jun 1, 2015 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

IDBObjectStore

Uh oh!

inexorabletash commented Jun 1, 2015 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

IDBIndex

Uh oh!

inexorabletash commented Jun 1, 2015 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

IDBCursor

Uh oh!

inexorabletash commented Jun 1, 2015 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

IDBTransaction

Uh oh!

odinho commented Jun 16, 2015

Uh oh!

inexorabletash commented Jun 16, 2015

Uh oh!

inexorabletash commented Jun 25, 2015

Uh oh!

inexorabletash commented Sep 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

inexorabletash commented Sep 23, 2016

Uh oh!

inexorabletash commented Oct 12, 2016

Uh oh!

beidson commented Oct 21, 2016

Uh oh!

inexorabletash commented Oct 21, 2016

Uh oh!

pwnall commented Oct 23, 2016

Uh oh!

bevis-tseng commented Oct 24, 2016

Uh oh!

beidson commented Oct 24, 2016

Uh oh!

beidson commented Oct 24, 2016

Uh oh!

inexorabletash commented Jan 13, 2017

Uh oh!

inexorabletash commented Jun 1, 2015 •

edited

Loading

inexorabletash commented Jun 1, 2015 •

edited

Loading

inexorabletash commented Jun 1, 2015 •

edited

Loading

inexorabletash commented Jun 1, 2015 •

edited

Loading

inexorabletash commented Jun 1, 2015 •

edited

Loading

inexorabletash commented Sep 23, 2016 •

edited

Loading