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

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.

> 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.

> 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.  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.

> This might be somewhat confusing for developers?
> 
> One option might be to only fire an error event on the transaction if 
> the transaction is failing not due to a failed request, but due to 
> failure committing the transaction, or if the transaction is aborted.
> But there's still the risk that people would set up an error listener 
> on the transaction and only expect commit errors, and be surprised 
> about the onerror getting called during the bubbling phase when an 
> "error" event is fired at a request.
> 
> 
> Separately, I had thought that we should make transaction.error 
> indicate a ABORT_ERR if transaction.abort() is called. But I think I 
> like your idea of leaving it null better since there might not 
> actually have been an error, but rather the change or read wasn't desired any more.
> 
> 
> So, to sum things up. I definitely think we should do the following:
> 
> Change the steps to "fire an error event" such that when it invokes 
> the "steps for aborting a transaction", it forwards whatever 
> error-code caused the error event to be dispatched in the first place.

Agree

> 
> In the "steps for aborting a transaction", add an additional step 
> between the current steps 1 and 2 which says something like "Set 
> transaction's error to <var>error</var>".
> 

Agree

> In all locations when we invoke the "steps for aborting a transaction"
> or the steps for "fire an error event", make sure that we indicate 
> which error code should be used. For example, I'm not sure what code 
> we should use when an exception is thrown from a "success" handler, or from an "error"
> handler where .preventDefault was not called.
> 

Agree

> 
> Additionally we might want to change the "steps for aborting a transaction"
> such that it fires an "error" event at the transaction before firing the "abort"
> event. Or maybe only doing so if the "steps for aborting a 
> transaction" were not called as a result of step 3 in the steps to "fire a error event".

Based on our previous comments, we don't agree with this statement.

> 
> / Jonas

Israel

Received on Thursday, 13 October 2011 17:58:14 UTC