Closed Bug 929274 Opened 6 years ago Closed 6 years ago

InvalidStateError when accessing transaction.error after aborted transaction

Categories

(Core :: Storage: IndexedDB, defect)

24 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: spamdaemon, Assigned: bent.mozilla)

Details

Attachments

(2 files, 4 obsolete files)

Attached file transactionTest.js
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 2013091200

Steps to reproduce:

1. Create a store
2. Add an element with a key, e.g. "FOO"
3. Create a transaction, call it tx
4. Add the element again with the same key to trigger a duplicate error
5. Access tx.error in the transaction's onerror handler


Actual results:

[20:46:45.468] InvalidStateError: An attempt was made to use an object that is not, or is no longer, usable @ https://localhost:8080/transactionTest.js:20



Expected results:

I'm not sure what to expect, but I did not expect an exception to be raised in this case.
Chrome simply reports tx.error===null, but I'm thinking that some kind of DOM error should be produced.
Attachment #820078 - Attachment mime type: application/javascript → text/javascript
This does indeed look like we're not following the spec. According to my reading we should return null as long as the transaction is open, and should *never* let it throw an exception.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch changes.patch (obsolete) — Splinter Review
I think this is better.
Assignee: nobody → bent.mozilla
Status: NEW → ASSIGNED
Attachment #820254 - Flags: review?(Jan.Varga)
Attached patch Patch, v1 (obsolete) — Splinter Review
With a better test.
Attachment #820254 - Attachment is obsolete: true
Attachment #820254 - Flags: review?(Jan.Varga)
Attachment #820267 - Flags: review?(Jan.Varga)
Comment on attachment 820267 [details] [diff] [review]
Patch, v1

Review of attachment 820267 [details] [diff] [review]:
-----------------------------------------------------------------

looks good, no nits :)
Attachment #820267 - Flags: review?(Jan.Varga) → review+
Attached patch Patch, v2 (obsolete) — Splinter Review
Here is a new patch. Passed unit tests locally.
Attachment #820267 - Attachment is obsolete: true
Attached patch Patch v3 (obsolete) — Splinter Review
Updated patch.
Attachment #8423483 - Attachment is obsolete: true
Comment on attachment 8424187 [details] [diff] [review]
Patch v3

Review of attachment 8424187 [details] [diff] [review]:
-----------------------------------------------------------------

Carrying jan's review forward, test changes look good to me!
Attachment #8424187 - Flags: review+
Shihua, did you get this pushed to try? Can you paste the link to the tbpl run here? Thanks!
Flags: needinfo?(szheng)
(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #10)
> Shihua, did you get this pushed to try? Can you paste the link to the tbpl
> run here? Thanks!

https://tbpl.mozilla.org/?tree=Try&rev=a2f13887b7b4
Flags: needinfo?(szheng)
Attached patch Patch v4Splinter Review
Disable mochitest for b2g desktop.
Try run: https://tbpl.mozilla.org/?tree=Try&rev=8c9f26a8cea4
Attachment #8424187 - Attachment is obsolete: true
Try run passed. Need checkin?
Flags: needinfo?(bent.mozilla)
https://hg.mozilla.org/mozilla-central/rev/e4e5b0502f93
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.