Closed Bug 929274 Opened 12 years ago Closed 12 years ago

InvalidStateError when accessing transaction.error after aborted transaction

Categories

(Core :: Storage: IndexedDB, defect)

Product:
Core
Component:
Storage: IndexedDB
Version:
24 Branch
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla32

People

(Reporter: spamdaemon, Assigned: bent.mozilla)

Details

Attachments

(2 files, 4 obsolete files)

transactionTest.js
1.22 KB, text/javascript
Details
Patch v4
12.93 KB, patch
khuey
: review+
Details | Diff | Splinter Review
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 v4 — — Splinter 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: 12 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.

Attachment

General

Creator:
Created:
Updated:
Size: