W3C home > Mailing lists > Public > public-webapps@w3.org > January to March 2012

Re: [indexeddb] Missing TransactionInactiveError Exception type for count and index methods

From: Jonas Sicking <jonas@sicking.cc>
Date: Tue, 24 Jan 2012 17:38:36 -0800
Message-ID: <CA+c2ei8Zoxmg0QdkyAvE6PEc__gcea8jXerbNff2DRp5mBNMMA@mail.gmail.com>
To: Israel Hilerio <israelh@microsoft.com>
Cc: "jsbell@chromium.org" <jsbell@chromium.org>, "public-webapps@w3.org" <public-webapps@w3.org>, Adam Herchenroether <aherchen@microsoft.com>, Victor Ngo <vicngo@microsoft.com>
On Tue, Jan 24, 2012 at 2:42 PM, Israel Hilerio <israelh@microsoft.com> wrote:
> On Tuesday, January 24, 2012 2:15 PM, Jonas Sicking wrote:
>> On Tue, Jan 24, 2012 at 12:48 PM, Jonas Sicking <jonas@sicking.cc> wrote:
>> > On Tue, Jan 24, 2012 at 12:33 PM, Israel Hilerio <israelh@microsoft.com>
>> wrote:
>> >> On Tuesday, January 24, 2012 12:12 PM, Jonas Sicking wrote:
>> >>> On Tue, Jan 24, 2012 at 10:08 AM, Israel Hilerio
>> >>> <israelh@microsoft.com>
>> >>> wrote:
>> >>> >> >> In addition, the index method in IDBObjectStore uses
>> >>> >> >> InvalidStateError to convey two different meanings: the object
>> >>> >> >> has been removed or deleted and the transaction being used
>> finished.
>> >>> >> >> It seems that it would be better to separate these into:
>> >>> >> >> * InvalidStateError when the source object has been removed or
>> >>> deleted.
>> >>> >> >> * TransactionInactiveError when the transaction being used is
>> inactive.
>> >>> >> >>
>> >>> >> >> What do you think?  I can open a bug if we agree this is the
>> >>> >> >> desired behavior.
>> >>> >> >
>> >>> >> >
>> >>> >> > Did this come out of the discussion here:
>> >>> >> >
>> >>> >> > http://lists.w3.org/Archives/Public/public-
>> webapps/2011OctDec/1589.
>> >>> >> > htm
>> >>> >> > l
>> >>> >> >
>> >>> >> > If so, the rationale for which exception type to use is
>> >>> >> > included, although no-one on the thread was deeply averse to the
>> alternative.
>> >>> >> > If it's a different issue can give a more specific example?
>> >>> >>
>> >>> >> Right. I think InvalidStateErr is better, for the reason detailed
>> >>> >> in the above referenced email.
>> >>> >>
>> >>> >> I agree we're using the same exception for two error conditions,
>> >>> >> but I'm not terribly worried that this will make debugging harder for
>> authors.
>> >>> >>
>> >>> >> But I don't feel strongly so if there's a good reason I'm ok with
>> >>> >> changing things.
>> >>> >>
>> >>> >> / Jonas
>> >>> >>
>> >>> >
>> >>> > I agree that InvalidStateErr makes sense here.  The issue we're
>> >>> > presenting is
>> >>> the use of one exception for two error conditions.
>> >>> > We just want to remove the ambiguity and cleanup of the language.
>> >>> > We
>> >>> have this issue in the IDBObjectStore.index and
>> >>> IDBTransaction.objectStore methods.
>> >>> >
>> >>> > One alternative could be to just leave "InvalidStateError" and
>> >>> > remove the
>> >>> "or" clause from the description.
>> >>> > That would leave us with:
>> >>> >
>> >>> > 1. InvalidStateError - Occurs if a request is made on a source
>> >>> > object that
>> >>> has been deleted or removed.
>> >>> >
>> >>> > Alternatively, we could add one more exception to capture the "or"
>> clause:
>> >>> >
>> >>> > 2. TransactionInactiveError - Occurs if the transaction the object
>> >>> > store
>> >>> belongs to has finished.
>> >>> >
>> >>> > I'm okay with only doing #1, if you all agree.  This simplifies
>> >>> > things and
>> >>> captures the idea stated in the reference email.  Let me know what you
>> think.
>> >>>
>> >>> Hmm.. I think I'm not fully following you. Did you intend for #1 to
>> >>> change normative behavior, or to be an editorial clarification?
>> >>>
>> >>> Simply removing the part after the "or" seems to result in a
>> >>> normative change since nowhere would we say that an exception should
>> >>> be thrown if
>> >>> index() is called after a transaction has finished. I.e. removing it
>> >>> would mean that index() would have to return an IDBIndex instance
>> >>> even when called after the transaction has finished.
>> >>>
>> >>> Maybe the solution is to change the text to something like: "Thrown
>> >>> if the function is called on a source object that has been deleted.
>> >>> Also thrown if the transaction the object store belongs to has finished."
>> >>>
>> >>> / Jonas
>> >>
>> >> Sorry for the confusion.  My assumption for #1 was that it would be
>> >> okay not to throw an exception if a developer were to make a call to
>> >> IDBObjectStore.index and IDBTransaction.objectStore when there is no
>> transaction as long as any requests from those objects would throw
>> TransactionInactiveError.
>> >> This would leave TransactionInactiveError to be thrown by methods that
>> return IDBRequests only.
>> >>
>> >> In Alternative #2, we were proposing to add TransactionInactiveError
>> >> to the exception list to avoid the exception overloading.  It just seems
>> weird to overload InvalidStateError with multiple definitions.
>> >>
>> >> Should we just do #2, then?
>> >
>> > Hmm.. the throwing for index() when called was after the transaction
>> > was ended was done on request from you guys as I recall it.
>> >
>> > As mentioned in the discussion at [1] I think that
>> > TransactionInactiveError is the wrong exception to throw here.
>> >
>> > l really don't understand the problem of throwing the same exception
>> > for two different error conditions. For example IDBObjectStore.put and
>> > IDBObjectStore.add throws a DataError for 7 different error conditions
>> > as enumerated in the bullet list at [2]. And IDBCursor.update throws
>> > InvalidStateError under two different conditions [3]. And any function
>> > which uses structured clones throw DataCloneError for two related but
>> > different error conditions [4].
>> >
>> > So I don't see why we couldn't simply keep things as they are now?
>> >
>> > [1]
>> > http://lists.w3.org/Archives/Public/public-webapps/2011OctDec/1589.htm
>> > l [2]
>> > http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBObj
>> > ectStore-put-IDBRequest-any-value-any-key
>> > [3]
>> > http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBCur
>> > sor-update-IDBRequest-any-value [4]
>> > http://dev.w3.org/html5/spec/Overview.html#structured-clone
>>
>> A few other examples of functions which throw the same exception for
>> multiple types of errors:
>>
>> EventTarget.dispatchEvent [5] throws InvalidStateError for two different
>> error conditions. Node.insertBefore throws HierarchyRequestError for 7
>> different error conditions, as do Node.appendChild and many other DOM
>> mutating methods.
>> XMLHttpRequest.send throws InvalidStateError for three different error
>> conditions (though two of them are very similar). The same method also
>> throws a NetworkError for multiple types of network errors.
>>
>> So I really don't think we should worry too much about throwing the same
>> exception for multiple types of errors (especially InvalidStateError). Instead
>> we should focus on throwing the most applicable error in each error
>> conditions.
>>
>> / Jonas
>>
>>
>> [5] http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-
>> eventtarget-dispatchevent
>
> Jonas,
>
> Thanks for providing these examples.  Perhaps another way of stating my point is that we have two
> Exception types (i.e. InvalidStateError and TransactionInactiveError) which are used for the same purpose on the same API set.  The issue is that, as a developer  I need to remember that InvalidStateError could mean the same as a TransactionInactiveError, it just depends the method  I'm invoking.  This seems odd to me.
>
> If you feel so strongly about this, then I'm okay leaving it the way it is.  I was trying to
> clear a possible confusion point that one of my testers brought up.

Ah, I think I understand what you mean better.

I don't however fully think that it's true that "[developers] need to
remember that InvalidStateError could mean the same as a
TransactionInactiveError". The purpose of the events are somewhat
different. As mentioned in the description Eliot wrote for
InvalidStateError (which is quite excellent by the way!),
TransactionInactiveError is a more specific type state error which
applies to functions which can only be called when a transaction is
active.

This is why I don't think it quite applies here since we (as I
understand it) allow the index() function to be called even when the
transaction is inactive.

Another point is that these types of exceptions are there mostly for
developers to catch bugs. I.e. if someone calls objectStore.index()
after transaction is finished the exception should hopefully bubble
all the way out and an error is logged in their developer console. So
the main purpose of the different exceptions is to provide
documentation to the developer so that they have an easier time honing
in on what went wrong.

So there's really very little need to remember which exception is
thrown when. It's mostly there as documentation when something goes
wrong.

/ Jonas
Received on Wednesday, 25 January 2012 01:39:40 GMT

This archive was generated by hypermail 2.3.1 : Tuesday, 26 March 2013 18:49:50 GMT