Re: [indexeddb] Implicit Transaction Request associated with failed transactions

On Thu, Oct 13, 2011 at 10:57 AM, Israel Hilerio <israelh@microsoft.com> wrote:
> On Monday, October 10, 2011 10:10 PM, Jonas Sicking wrote:
>> On Thu, Oct 6, 2011 at 3:30 PM, Israel Hilerio <israelh@microsoft.com> wrote:
>> > On Tuesday, October 04, 2011 3:01 AM, Jonas Sicking wrote:
>> >> On Mon, Oct 3, 2011 at 7:59 PM, Jonas Sicking <jonas@sicking.cc> wrote:
>> >> > On Mon, Sep 12, 2011 at 2:53 PM, Israel Hilerio
>> >> > <israelh@microsoft.com>
>> >> wrote:
>> >> >> Based on previous conversations, it seems we've agreed that
>> >> >> there are
>> >> situations in which a transaction could failed independent of
>> >> explicit requests (i.e. QUOTA_ERR, TIMEOUT_ERR).  We believe that
>> >> this can be represented as an implicit request that is being
>> >> triggered by a transaction.  We would like to add this concept to
>> >> the spec.  The benefit of doing this is that it will allow
>> >> developers to detect the error code associated with a direct
>> >> transaction failure.  This is
>> how we see the concept being used:
>> >> >>
>> >> >> trans.onerror = function (e) {
>> >> >> //eventTarget is mapped to an implicit transaction that was
>> >> >> created behind the scenes to track the transaction
>> >> >>
>> >> >>  if (e.eventTarget.errorCode === TIMEOUT_ERR) {
>> >> >>    // you know the transaction error because of a timeout
>> >> >> problem
>> >> >>  }
>> >> >>  else if (e.eventTarget.errorCode === TIMEOUT_ERR) {
>> >> >>   // you know the transaction error because of a quota problem
>> >> >>  }
>> >> >> }
>> >> >>
>> >> >> Our assumption is that the error came not from an explicit
>> >> >> request but
>> >> from the transaction itself.  The way it is today, the
>> >> e.eventTarget will not exists (will be undefined) because the error
>> >> was not generated from an explicit request.  Today, eventTargets
>> >> are only populated from explicit requests.
>> >> >
>> >> > Good catch!
>> >> >
>> >> > We had a long thread about this a while back with the subject
>> >> > "[IndexedDB] Reason for aborting transactions". But it seems to
>> >> > have fizzled with no real conclusion as to changing the spec. In
>> >> > part that seems to have been my fault pushing back at exposing
>> >> > the reason for a aborted transaction.
>> >> >
>> >> > I think I was wrong :-)
>> >> >
>> >> > I think I would prefer adding a .errorCode on IDBTransaction
>> >> > through (or .errorName or .error or whatever we'll end up changing it to).
>> >> > This seems more clear than creating a implicit request object.
>> >> > It'll also make it easy to find the error if you're outside the
>> >> > error handler. With the implicit request, you have no way of
>> >> > getting to the request, and thus the error code, from code
>> >> > outside the error handler, such from code that looks at the
>> >> > transaction after it has
>> run.
>> >> >
>> >> > And the code above would work exactly as is!
>> >> >
>> >> > Let me know what you think?
>> >>
>> >> In detail, here is what I suggest:
>> >>
>> >> 1. Add a .errorCode (or .errorName/.error) property on
>> >> IDBTransaction/IDBTransactionSync.
>> >> 2. The property default to 0 (or empty string/null) 3. In the
>> >> "Steps for aborting a transaction" add a new step between the
>> >> current steps
>> >> 1 and 2 which says something like "set the errorCode property of
>> >> <var>transaction</var> to <var>code</var>".
>> >>
>> >> This way the reason for the abort is available (through the
>> >> transaction) while firing the "error" event on all still pending
>> >> requests in step 2. The reason is also available while firing the
>> >> "abort" event on the transaction itself.
>> >>
>> >> / Jonas
>> >
>> > Independent on how we handler error, we like this approach!  This is
>> > our
>> interpretation of the impact it will have on the overall feature.
>> >
>> > SCENARIO #1:
>> > Whenever there is an error on a request, the error value associated
>> > with the
>> request will be assigned to the transaction error value.
>> > The error value in the transaction will be available on the
>> IDBTransaction.onerror and IDBTransaction.onabort handlers.
>> >
>> > SCENARIO #2:
>> > Whenever there is an error associated with the transaction (e.g.
>> > QUOTA or
>> TIMEOUT ), the error value associated with the failure (e.g. QUOTA or
>> TIMEOUT) will be assigned to the transaction error value.  The error
>> value in the transaction will be available on the
>> IDBTransaction.onerror and IDBTransaction.onabort handlers.
>> >
>> > SCENARIO #3:
>> > A developer uses the IDBTransaction.abort() method to cancel the
>> transaction.  No error will be assigned to the transaction error
>> value. The error value will be 0 (or empty string/null) when the
>> IDBTransaction.onabort handler is called.
>> >
>> > SCENARIO #4 (to be complete):
>> > Whenever there is an error on a request, the error value associated
>> > with the
>> request will be assigned to the transaction error value. However, if
>> the
>> event.preventDefault() method is called on the request, the only
>> handler that will be called will be IDBTransaction.onerror and the
>> error value will be available in the transaction.  This implies that
>> the value of the first transaction event error that is not cancelled
>> or prevented from executing its default behavior will be value that
>> will be contained by the error on the transaction when the IDBTransaction.onabort handler is called.  See example below:
>> >
>> > request1 ==> fires onerror with event.target errorCode == DATA_ERR
>> > //request1 will preventDefault on the error event transaction ==>
>> > fires onerror with this.errorCode = event.target.errorCode ==
>> > DATA_ERR
>> >
>> > request2 ==> fires onerror with event.target errorCode ==
>> > CONSTRAINT_ERR
>> > //request2 will let the error continue on its default path
>> > transaction ==> fires onerror with this.errorCode =
>> > event.target.errorCode == CONSTRAINT_ERR //the error sent to the
>> > transaction onerror handler will be passed to the onabort error
>> > event
>> >
>> > request3 ==> fires onerror with event.target.errorCode == ABORT_ERR
>> > transaction ==> fires onerror with this.errorCode =
>> > event.target.errorCode == ABORT_ERR transaction ==> fires onabort
>> > with this.errorCode =  event.target.errorCode == CONSTRAINT_ERR
>> >
>> > SCENARIO #5 (to be complete):
>> > Whenever there is an error on a request, the error value associated
>> > with the
>> request will be assigned to the transaction error value.
>> > However, if the event. stopPropagation() method is called on the
>> > request,
>> the only handler that will be called will be IDBTransaction.onabort
>> and the error value will be available in the transaction.
>> >
>> > Do you agree?
>>
>> I *think* I was envisioning something slightly different.
>>
>> First off the below assumes that we go with the .error approach,
>> rather than .errorCode or .errorName, per the decision in a separate thread.
>>
>> Also, note that there's a distinction between "Fire an 'error' event
>> on transaction" and "transaction.onerror runs". When an error event is
>> fired on a request, the following things happens:
>>
>> 1. All capturing listeners registered on the database for the "error"
>> event runs.
>> 2. All capturing listeners registered on the transaction for the
>> "error" event runs.
>> 3. All event listeners registered on the request for the "error" event
>> runs (bubbling and capturing alike). This includes calling
>> request.onerror if it's non- null.
>> 4. All bubbling listeners registered on the transaction for the
>> "error" event runs. This includes calling transaction.onerror if it's non-null.
>> 5. All bubbling listeners registered on the database for the "error"
>> event runs. This includes calling db.onerror if it's non-null.
>>
>> This is defined by the DOM Events spec, so not really something we
>> have control over in the IndexedDB spec. We *could* choose to make the
>> error event non-bubbling, in which case step 4 and 5 would not happen.
>> But I don't think we should make that change.
>>
>> So please be clear when talking about these things if you're talking
>> about firing an "error" event on the transaction, or running
>> transaction.onerror, since they are two different things.
>>
>
> To clarify, we weren't suggesting that onerror handlers be called directly without following the proper "error" event model specified in the DOM Events spec.

Cool. It's just easier when making concrete proposals to express
things in terms of when an event is fired, and what object it's fired
on. When the various on* handlers are called is a direct result of
that.

>> So, with that background, here is my thinking:
>>
>> I don't think we should set IDBTransaction.error just because a single
>> request failed. This since the error handler for the request might
>> still call
>> event.preventDefault() in which case things will progress just fine.
>> (I'm not sure if this is what you were proposing, I just wanted to be clear).
>>
>> Only if an error for a request causes the transaction to be aborted
>> (i.e. if event.preventDefault was not called on the error event during
>> dispatch) should we set transaction.error.
>>
>> The goal being that as long as the transaction isn't aborted,
>> transaction.error should remain null.
>>
>
> Your explanation about not setting the error attribute when the IDBTransaction.onerror handler is called makes sense.  This implies that once the initial abort event is fired, the IDBTransaction.error attribute will never change.

Cool.

>> Additionally, as things currently stand, no "error" event is fired at
>> transactions when they are aborted. Only a "abort" event. The only
>> time event listeners registered on a transaction "error" events are
>> called, is during the capture/bubble phase when an "error" event is
>> fired on a request belonging to that transaction.
>>
>> I have mixed feelings about if I think that's good or not. On one hand
>> it does make sense that if you want to do error handling, you only
>> need to listen to the "error" event to capture all types of errors,
>> both ones fired on transactions and ones fired on requests.
>>
>> On the other hand, if we fire "error" events when a transaction is
>> aborted, that means that transaction.onerror is called both for errors
>> related to requests, which by the time transaction.onerror is called
>> might already have been handled, as well as errors fired at the
>> transaction itself due to for example running out of quota.
>>
>> So for example if a transaction is aborted using transaction.abort(),
>> then transaction.onerror will first be called once for each still
>> pending request as each request is aborted, then transaction.onerror
>> would be called for the actual abort of the transaction.
>>
>
> We don't believe it is a good idea to fire an error event when the IDBTransaction.abort method is called.

Good point. I agree with that.

> The firing of error events on the transaction should only be of two types: propagation error events or transaction error events.
>
> This should allow devs the ability to handle data error objects inside their IDBRequest.onerror handler and transaction commit error on their IDBTransaction.onerror handler.  The only difference is that QuotaError and TimeoutError would wouldn't be cancellable and will always bubble.

Not quite sure what you mean by "propagation error events". Do you
mean events that are fired on a request, but where the
transaction.onerror handler is called during the event's bubble phase?

If so I think this sounds good. However there's still the matter of
defining when a "transaction error event" should be fired.
Transactions currently fail to commit in at least the following
circumstances:

A) When transaction.abort() is explicitly called
B) When a .put or .add request fails due to a 'unique' constraint
violation in an index, and the result "error" event *isn't* canceled
using event.preventDefault()
C) When an exception is thrown during a event handler for a "error" event
D) When an exception is thrown during a event handler for a "success" event
E) When a index is created with a 'unique' constraint on an
objectStore which already has data which violates the constraint.
F) When a request fails due to database error (for example IO error)
and the resulting "error" event *isn't* canceled using
event.preventDefault()
G) When a transaction fails to commit due to a quota error
H) When a transaction fails to commit due to a IO error
I) When a transaction fails to commit due to a timeout error

I've probably missed some places too.

In which of these occasions do you think we should fire an "error"
event on the transaction before firing the "abort" event? And note
that in *all* of the above cases we will fire a "abort" event on the
transaction.

>From the discussion so far, it sounds like you *don't* want to fire an
"error" event for at least for situation A, which makes sense to me.

Whatever situations we decide to fire an "error" event in, I'd like
there to be some sort of consistency. I'd also like to start by
looking at use cases rather than just at random pick situations that
seem good.

So, if anyone think we should fire error events targeted at the
transaction in any of the above situations, please state use cases,
and which of the above situations you think should generate an error
event.

Additionally, I'm not sure that we need to worry about I) above. I)
only happens when there are timeouts specified, which only is possible
in the synchronous API. But in the synchronous API we never fire any
events, but simply let IDBDatabaseSync.transaction throw an exception.

/ Jonas

Received on Friday, 14 October 2011 21:33:37 UTC