Re: [IndexedDB] Reason for aborting transactions

On Tue, Feb 8, 2011 at 10:48 AM, Jeremy Orlow <jorlow@chromium.org> wrote:
> On Tue, Feb 8, 2011 at 10:38 AM, Jonas Sicking <jonas@sicking.cc> wrote:
>>
>> On Tue, Feb 8, 2011 at 9:16 AM, Jeremy Orlow <jorlow@chromium.org> wrote:
>> > On Tue, Feb 8, 2011 at 2:21 AM, Jonas Sicking <jonas@sicking.cc> wrote:
>> >>
>> >> On Mon, Feb 7, 2011 at 8:05 PM, Jeremy Orlow <jorlow@chromium.org>
>> >> wrote:
>> >> > On Mon, Feb 7, 2011 at 7:36 PM, Jonas Sicking <jonas@sicking.cc>
>> >> > wrote:
>> >> >>
>> >> >> On Fri, Jan 28, 2011 at 4:33 PM, Jeremy Orlow <jorlow@chromium.org>
>> >> >> wrote:
>> >> >> > We do that as well.
>> >> >> > What's the best way to do it API wise?  Do we need to add an
>> >> >> > IDBTransactionError object with error codes and such?
>> >> >>
>> >> >> I don't actually know. I can't think of a precedence. Usually you
>> >> >> use
>> >> >> different error codes for different errors, but here we want to
>> >> >> distinguish a particular type of error (aborts) into several sub
>> >> >> categories.
>> >> >
>> >> > I don't see how that's any different than what we're doing with the
>> >> > onerror
>> >> > error codes though?
>> >>
>> >> Hmm.. true.
>> >>
>> >> >> To make this more complicated, I actually think we're going to end
>> >> >> up
>> >> >> having to change a lot of error handling when things are all said
>> >> >> and
>> >> >> done. Error handling right now is sort of a mess since DOM
>> >> >> exceptions
>> >> >> are vastly different from JavaScript exceptions. Also DOM exceptions
>> >> >> have a messy situation of error codes overlapping making it very
>> >> >> easy
>> >> >> to confuse a IDBDatabaseException with a DOMException with an
>> >> >> overlapping error code.
>> >> >>
>> >> >> For details, see
>> >> >>
>> >> >>
>> >> >>
>> >> >> http://lists.w3.org/Archives/Public/public-script-coord/2010OctDec/0112.html
>> >> >>
>> >> >> So my gut feeling is that we'll have to revamp exceptions quite a
>> >> >> bit
>> >> >> before we unprefix our implementation. This is very unfortunate, but
>> >> >> shouldn't be as big deal of a deal as many other changes as most of
>> >> >> the time people don't have error handling code. Or at least not
>> >> >> error
>> >> >> handling code that differentiates the various errors.
>> >> >>
>> >> >> Unfortunately we can't make any changes to the spec here until
>> >> >> WebIDL
>> >> >> prescribes what the new exceptions should look like :(
>> >> >>
>> >> >> So to loop back to your original question, I think that the best way
>> >> >> to expose the different types of aborts is by adding a .reason (or
>> >> >> better named) property which returns a string or enum which
>> >> >> describes
>> >> >> the reason for the abort.
>> >> >
>> >> > Could we just add .abortCode, .abortReason, and constants for each
>> >> > code
>> >> > to
>> >> > IDBTransaction?
>> >>
>> >> Why both? How are they different. I'd just go with the former to align
>> >> with error codes.
>> >
>> > Sorry, I meant .abortMessage instead of .abortReason.  This would be
>> > much
>> > like normal error messages where we have a code that's standardized and
>> > easy
>> > for scripts to understand and then the message portion which is easy for
>> > humans to understand but more ad-hoc.
>> >
>> >>
>> >> > And maybe revisit in the future?
>> >>
>> >> Yes. I think we need to wait for webidl to solidify a bit here before
>> >> we do anything.
>> >
>> > I think we should put something in our spec in the mean time, but once
>> > WebIDL solidifies then we can revisit and try to match what's decided
>> > there.
>> >
>> > On Tue, Feb 8, 2011 at 8:07 AM, ben
>> > turner <bent.mozilla@gmail.com> wrote:
>> >>
>> >> Why not just expand our list of error codes to have multiple ABORT_
>> >> variants for each situation, and then always fire the "abort" event
>> >> with a slightly different errorCode?
>> >>
>> >> That seems far simpler IMO.
>> >
>> > If that is OK spec wise, I'm fine with it.  To be honest, hanging
>> > ABORT_BLAHs off IDBDatabaseException seems a bit odd though.
>>
>> I think at this point I've sort of lost track of what the proposal is.
>> Is it simply making abort events look like error events, but obviously
>> with .type set to "abort". And give them codes which live side-by-side
>> with the error codes?
>>
>> If so, that would be ok with me.
>
> I think that's what Ben was suggesting.  I was suggesting that it seemed
> kind of odd though, and I'd prefer the following:
> Add the following to IDBTransaction:
>   readonly attribute EXPLICIT_ABORT = 1
>   readonly attribute INTERNAL_ERROR_ABORT = 2
>   readonly attribute QUOTA_ERROR_ABORT=3
>   ... etc
>   readonly attribute abortMessage;
>   readonly attribute abortCode;
> And just set the message/code right before firing an abort event.

Yeah, I think Jeremy is right. If we are going to expose the abort
reason then this is way to do it.

However, the above is somewhat ugly. Do we really need this feature?
The only thing it seems like you can really make use of is the "quota
abort" reason. When that happens you can show to the user a message
like "Unable to synchronize emails since you didn't give me enough
quota. Knucklehead!". Maybe a cleaner solution could be designed that
only let you tell that condition from other aborts.

Did you have a use case in mind for the other types of aborts?

The one thing I could think of is being able to log errors and report
them back to the server. For example if you detect that you're getting
a lot of "user leaving page"-aborts, you might want to look into
making your transactions shorter-running, or putting up a "Saving,
please don't leave the page. Knucklehead!" dialog while the
transaction is running. And if you're getting a lot of
INTERNAL_ERROR_ABORT errors, then you could look into filing bugs
against a UA developer.

In any case, I still think we need to keep the ABORT_ERR code so that
we can fire an error event at the requests which hasn't finished yet
though. The spec does indeed contain the steps to abort a transaction.
It's currently section 4.3. Though it appears the links to it are
broken? And the second step contains a bug. It says "fire a error
event with code /code/", but should say "fire a error event with code
ABORT_ERR".

/ Jonas

Received on Wednesday, 9 February 2011 23:23:17 UTC