Re: [Bug 11375] New: [IndexedDB] Error codes need to be assigned new numbers

On Wed, Dec 15, 2010 at 3:42 AM, Jonas Sicking <jonas@sicking.cc> wrote:

> > Speaking of which, we use UNKNOWN_ERR for a bunch of other
> > internal consistency issues.  Is this OK by everyone, should we use
> another,
> > or should we create a new one?  (Ideally these issues will be few and far
> > between as we make things more robust.)
>
> Is this things like disk IO errors and the like?
>

Yeah and other "impossible" conditions.


>  > We also use UNKNOWN_ERR for when things are not yet implemented.  Any
> > concerns?
>
> Ideal is if you can leave out the function entirely if you don't
> implement it yet.
>

An example is autoIncrement.  There isn't really any way to "leave it out"
because we otherwise support creating object stores and silently ignoring it
means that we don't behave the way a user would expect.

If there were mature IndexedDB implementations, we would have blocked
releasing anything until we were feature complete, but until then I think
it's better for everyone that we release early and often so we can get
feedback.

When this isn't possible, I would say that you should throw something
> different from what could be legitimately thrown from the function. I
> know gecko has a special exception we throw from various places when
> functionality isn't implemented.
>
> I don't think we should put anything in the spec about this as the
> spec should specify that everything should be implemented :)
>

I agree.  Wasn't asking because I wanted to add anything to the spec.


>  > What error code should we use for IDBCursor.update/delete when the
> cursor is
> > not currently on an item
>
> It's currently specced to throw NOT_ALLOWED_ERR. I think this is
> consistent with other uses of that exception.
>

I see now.  In the text at the top it does actually say this clearly, but in
the table below it only talks about hitting the end of results.


> > (or that item has been deleted)?
>
> I brought up this a while back in bug 11257. Note that you can't throw
> since you can't synchronously know if an item has been deleted.
>
> IMHO the simplest thing is to just treat IDBCursor.update the same as
> IDBObjectStore.put and IDBCursor.delete as IDBObjectStore.delete. I
> can't think of a use case for deleting and then wanting to ensure that
> IDBCursor.update doesn't create a new entry, so we might as well keep
> things simple and reuse the implementation and spec logic.
>
> In short, I think the spec is fine in this area.
>
> > TRANSIENT_ERR doesn't seem to be used anywhere in the spec.  Should it be
> > removed?
>
> Yes, along with RECOVERABLE_ERR, NON_TRANSIENT_ERR and DEADLOCK_ERR.
>
> We should also remove the .message property. DOMException doesn't have
> that.
>
> > As for the numbering: does anyone object to me just starting from 1 and
> > going sequentially?  I.e. does anyone have a problem with them all
> getting
> > new numbers, or should I keep the numbers the same when possible.  (i.e.
> > only UNKNOWN_ERR, RECOVERABLE_ERR, TRANSIENT_ERR, TIMEOUT_ERR,
> DEADLOCK_ERR
> > would change number, but the ordering of those on the page would change.)
> > I intend to tackle this early next week unless there are major areas of
> > contention.
>
> Sounds great. The only possible thing that we might want to do
> differently is that we might want to get rid of IDBDatabaseException
> entirely and just add values to DOMException. I don't have an opinion
> on this but I'm currently checking with JS developers what is easiest
> for them. (In general I'm not a fan of how exceptions in the DOM are
> so different from JS-exceptions).
>

Other APIs have their own exception classes.  What's the benefit of putting
our exceptions in DOMException?  The downside is that other specs need to
coordinate with our exception codes.  Unless there's a pretty compelling
reason to do this, I don't think we should.

J