Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-05 Thread Andrei Popescu
On Sat, Jul 3, 2010 at 1:52 AM, Andrei Popescu andr...@google.com wrote:
 On Fri, Jul 2, 2010 at 9:45 PM, Jonas Sicking jo...@sicking.cc wrote:
 On Fri, Jul 2, 2010 at 1:02 PM, Andrei Popescu andr...@google.com wrote:
 On Fri, Jul 2, 2010 at 10:43 AM, Jonas Sicking jo...@sicking.cc wrote:
 Filed http://www.w3.org/Bugs/Public/show_bug.cgi?id=10064

 Fixed. Please have a look, in case I missed or got anything wrong. Thanks!

 For add and put you should not throw DATA_ERR if the objectStore has a
 key generator.


 Oh, I thought I added a sentence about that. Will fix on Monday.


Fixed.

Andrei



Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-02 Thread Jeremy Orlow
Ok.

Want to write up a proposal for how to implement this (presumably in a bug)?

J

On Fri, Jul 2, 2010 at 6:38 AM, ben turner b...@mozilla.com wrote:

 I would also point out that throwing exceptions at the call site makes
 debugging much easier in my opinion. Our error events currently don't
 include information like filename and line number where the failing
 request was generated (though I think we should add that eventually).
 Exceptions are much easier to track down, diagnose, and fix.

 -Ben

 On Thu, Jul 1, 2010 at 2:03 AM, Jonas Sicking jo...@sicking.cc wrote:
  On Wed, Jun 30, 2010 at 8:16 PM, Jeremy Orlow jor...@chromium.org
 wrote:
  I've thought about this more and have some additional doubts inline.
  On Thu, Jul 1, 2010 at 11:55 AM, Jonas Sicking jo...@sicking.cc
 wrote:
 
  On Wed, Jun 30, 2010 at 6:42 PM, Jeremy Orlow jor...@chromium.org
 wrote:
   On Thu, Jul 1, 2010 at 11:17 AM, Jonas Sicking jo...@sicking.cc
 wrote:
  
   Hi All,
  
   Currently the IndexedDB specification is silent on what should
 happen
   if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
   IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
   transaction. There are two possible ways we can handle this:
  
   1. We can throw an exception.
   2. We can return a IDBRequest object and asynchronously fire a
 'error'
   event on this object.
  
   The advantage of 1 is that we pretty much know that this was an
 error
   due to a bug in the web page,
 
  I don't see why this is compelling.  Many of the other errors that you
 still
  propose we fire via the callback are due to bugs in the page.
 
 
   and we can always know this
   synchronously without having to consult the database.
 
  So?  Just because we can doesn't mean we should.
 
 
   Throwing an
   error means that all the existing infrastructure for error handling
   with automatically kick in. For example any higher-level try/catch
   constructs will have an opportunity to catch the error.
   Implementations generally report uncaught exceptions to an error
 log.
   The browser will fire an 'error' event on the window which the page
   can use for further logging. Firing an error event on the other hand
   does not allow the browser to automatically log the error in a
 console
   as the page hasn't yet gotten a chance to handle it.
 
  Sure, but this doesn't help the majority of error conditions in
 IndexedDB.
   It also ignores the cost of handling errors in 2 different ways.
 
  I'm arguing that people generally won't want to check for all types of
  errors, such as writing in a READ_ONLY transaction. Compare to the
  Node.appendChild function which throws for a number of conditions, yet
  I've never seen anyone put a try around it and check for
  HIERARCHY_REQUEST_ERR.
 
  Additionally, anything you do can likely in theory throw exceptions.
  For example if you specify the wrong number of arguments or call a
  function that doesn't exist, this will result in an exception. However
  you rarely see people wrap try around calls to try to handle this
  type of bugs.
 
  Another example is the line:
 
  db.objectStore(myObjectStore).get(53)
 
  can result in errors both being thrown and being reported as an error
  event. If the objectStore myObjectStore doesn't exist, then the
  .objectStore() call will return null and the js engine will throw due
  to trying to call a function on a null value.
 
  Error checking is mostly useful when there are ways you can actually
  handle the error. I think generally if someone calls .put during a
  READ_ONLY transaction, they simply have a bug in their program and
  there is little they can do in terms of handling that, short of
  simply logging it.
 
   The advantage of 2 is that this is consistent with other error
   conditions, such as writing duplicate keys, disk errors during
 writing
   the database to disk, internal errors in the database, etc.
 
  The other problem is that users then need 2 sets of error handling
 routines
  for each call.  Given how difficult it is to get web developers to do
 any
  error checking, requiring 2 types of checks seems like a big downside.
 
  Actually, this is somewhat faulty math. With exceptions you can put a
  try/catch high up in a call stack to catch errors from multiple
  different locations. This will catch all errors thrown from all
  function calls inside the try. Compare that to error events, which has
  to be manually installed at every single request site. From this point
  of view, the more errors we move to being reported as exceptions, the
  more easily we're making it for developers to catch more errors.
 
  Javascript has a error reporting mechanism, exceptions. We should try
  to make good use of it.
 
   While consistency, and only needing to check for errors one way, is
   certainly good arguments, I would argue that people won't need to
   check for calling-add-on-read-only-transactions. For properly
 written
   code it's not an 

Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-02 Thread ben turner
I would also point out that throwing exceptions at the call site makes
debugging much easier in my opinion. Our error events currently don't
include information like filename and line number where the failing
request was generated (though I think we should add that eventually).
Exceptions are much easier to track down, diagnose, and fix.

-Ben

On Thu, Jul 1, 2010 at 2:03 AM, Jonas Sicking jo...@sicking.cc wrote:
 On Wed, Jun 30, 2010 at 8:16 PM, Jeremy Orlow jor...@chromium.org wrote:
 I've thought about this more and have some additional doubts inline.
 On Thu, Jul 1, 2010 at 11:55 AM, Jonas Sicking jo...@sicking.cc wrote:

 On Wed, Jun 30, 2010 at 6:42 PM, Jeremy Orlow jor...@chromium.org wrote:
  On Thu, Jul 1, 2010 at 11:17 AM, Jonas Sicking jo...@sicking.cc wrote:
 
  Hi All,
 
  Currently the IndexedDB specification is silent on what should happen
  if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
  IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
  transaction. There are two possible ways we can handle this:
 
  1. We can throw an exception.
  2. We can return a IDBRequest object and asynchronously fire a 'error'
  event on this object.
 
  The advantage of 1 is that we pretty much know that this was an error
  due to a bug in the web page,

 I don't see why this is compelling.  Many of the other errors that you still
 propose we fire via the callback are due to bugs in the page.


  and we can always know this
  synchronously without having to consult the database.

 So?  Just because we can doesn't mean we should.


  Throwing an
  error means that all the existing infrastructure for error handling
  with automatically kick in. For example any higher-level try/catch
  constructs will have an opportunity to catch the error.
  Implementations generally report uncaught exceptions to an error log..
  The browser will fire an 'error' event on the window which the page
  can use for further logging. Firing an error event on the other hand
  does not allow the browser to automatically log the error in a console
  as the page hasn't yet gotten a chance to handle it.

 Sure, but this doesn't help the majority of error conditions in IndexedDB.
  It also ignores the cost of handling errors in 2 different ways.

 I'm arguing that people generally won't want to check for all types of
 errors, such as writing in a READ_ONLY transaction. Compare to the
 Node.appendChild function which throws for a number of conditions, yet
 I've never seen anyone put a try around it and check for
 HIERARCHY_REQUEST_ERR.

 Additionally, anything you do can likely in theory throw exceptions.
 For example if you specify the wrong number of arguments or call a
 function that doesn't exist, this will result in an exception. However
 you rarely see people wrap try around calls to try to handle this
 type of bugs.

 Another example is the line:

 db.objectStore(myObjectStore).get(53)

 can result in errors both being thrown and being reported as an error
 event. If the objectStore myObjectStore doesn't exist, then the
 .objectStore() call will return null and the js engine will throw due
 to trying to call a function on a null value.

 Error checking is mostly useful when there are ways you can actually
 handle the error. I think generally if someone calls .put during a
 READ_ONLY transaction, they simply have a bug in their program and
 there is little they can do in terms of handling that, short of
 simply logging it.

  The advantage of 2 is that this is consistent with other error
  conditions, such as writing duplicate keys, disk errors during writing
  the database to disk, internal errors in the database, etc.

 The other problem is that users then need 2 sets of error handling routines
 for each call.  Given how difficult it is to get web developers to do any
 error checking, requiring 2 types of checks seems like a big downside.

 Actually, this is somewhat faulty math. With exceptions you can put a
 try/catch high up in a call stack to catch errors from multiple
 different locations. This will catch all errors thrown from all
 function calls inside the try. Compare that to error events, which has
 to be manually installed at every single request site. From this point
 of view, the more errors we move to being reported as exceptions, the
 more easily we're making it for developers to catch more errors.

 Javascript has a error reporting mechanism, exceptions. We should try
 to make good use of it.

  While consistency, and only needing to check for errors one way, is
  certainly good arguments, I would argue that people won't need to
  check for calling-add-on-read-only-transactions. For properly written
  code it's not an error that will occur, and thus there is no need to
  check for it. In fact, you probably are generally better off letting
  the exception bubble all the way up and get logged or caught by
  generic error handlers.

 These are awfully bold assumptions.  Simply not catching 

Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-02 Thread ben turner
I would also point out that throwing exceptions at the call site makes
debugging much easier in my opinion. Our error events currently don't
include information like filename and line number where the failing
request was generated (though I think we should add that eventually).
Exceptions are much easier to track down, diagnose, and fix.

-Ben

On Thu, Jul 1, 2010 at 2:03 AM, Jonas Sicking jo...@sicking.cc wrote:
 On Wed, Jun 30, 2010 at 8:16 PM, Jeremy Orlow jor...@chromium.org wrote:
 I've thought about this more and have some additional doubts inline.
 On Thu, Jul 1, 2010 at 11:55 AM, Jonas Sicking jo...@sicking.cc wrote:

 On Wed, Jun 30, 2010 at 6:42 PM, Jeremy Orlow jor...@chromium.org wrote:
  On Thu, Jul 1, 2010 at 11:17 AM, Jonas Sicking jo...@sicking.cc wrote:
 
  Hi All,
 
  Currently the IndexedDB specification is silent on what should happen
  if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
  IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
  transaction. There are two possible ways we can handle this:
 
  1. We can throw an exception.
  2. We can return a IDBRequest object and asynchronously fire a 'error'
  event on this object.
 
  The advantage of 1 is that we pretty much know that this was an error
  due to a bug in the web page,

 I don't see why this is compelling.  Many of the other errors that you still
 propose we fire via the callback are due to bugs in the page.


  and we can always know this
  synchronously without having to consult the database.

 So?  Just because we can doesn't mean we should.


  Throwing an
  error means that all the existing infrastructure for error handling
  with automatically kick in. For example any higher-level try/catch
  constructs will have an opportunity to catch the error.
  Implementations generally report uncaught exceptions to an error log..
  The browser will fire an 'error' event on the window which the page
  can use for further logging. Firing an error event on the other hand
  does not allow the browser to automatically log the error in a console
  as the page hasn't yet gotten a chance to handle it.

 Sure, but this doesn't help the majority of error conditions in IndexedDB.
  It also ignores the cost of handling errors in 2 different ways.

 I'm arguing that people generally won't want to check for all types of
 errors, such as writing in a READ_ONLY transaction. Compare to the
 Node.appendChild function which throws for a number of conditions, yet
 I've never seen anyone put a try around it and check for
 HIERARCHY_REQUEST_ERR.

 Additionally, anything you do can likely in theory throw exceptions.
 For example if you specify the wrong number of arguments or call a
 function that doesn't exist, this will result in an exception. However
 you rarely see people wrap try around calls to try to handle this
 type of bugs.

 Another example is the line:

 db.objectStore(myObjectStore).get(53)

 can result in errors both being thrown and being reported as an error
 event. If the objectStore myObjectStore doesn't exist, then the
 .objectStore() call will return null and the js engine will throw due
 to trying to call a function on a null value.

 Error checking is mostly useful when there are ways you can actually
 handle the error. I think generally if someone calls .put during a
 READ_ONLY transaction, they simply have a bug in their program and
 there is little they can do in terms of handling that, short of
 simply logging it.

  The advantage of 2 is that this is consistent with other error
  conditions, such as writing duplicate keys, disk errors during writing
  the database to disk, internal errors in the database, etc.

 The other problem is that users then need 2 sets of error handling routines
 for each call.  Given how difficult it is to get web developers to do any
 error checking, requiring 2 types of checks seems like a big downside.

 Actually, this is somewhat faulty math. With exceptions you can put a
 try/catch high up in a call stack to catch errors from multiple
 different locations. This will catch all errors thrown from all
 function calls inside the try. Compare that to error events, which has
 to be manually installed at every single request site. From this point
 of view, the more errors we move to being reported as exceptions, the
 more easily we're making it for developers to catch more errors.

 Javascript has a error reporting mechanism, exceptions. We should try
 to make good use of it.

  While consistency, and only needing to check for errors one way, is
  certainly good arguments, I would argue that people won't need to
  check for calling-add-on-read-only-transactions. For properly written
  code it's not an error that will occur, and thus there is no need to
  check for it. In fact, you probably are generally better off letting
  the exception bubble all the way up and get logged or caught by
  generic error handlers.

 These are awfully bold assumptions.  Simply not catching 

Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-02 Thread Jonas Sicking
Filed http://www.w3.org/Bugs/Public/show_bug.cgi?id=10064 if you want
help with the editing, or this is unclear, let me know.

/ Jonas

On Fri, Jul 2, 2010 at 12:56 AM, Jeremy Orlow jor...@chromium.org wrote:
 Ok.
 Want to write up a proposal for how to implement this (presumably in a bug)?
 J
 On Fri, Jul 2, 2010 at 6:38 AM, ben turner b...@mozilla.com wrote:

 I would also point out that throwing exceptions at the call site makes
 debugging much easier in my opinion. Our error events currently don't
 include information like filename and line number where the failing
 request was generated (though I think we should add that eventually).
 Exceptions are much easier to track down, diagnose, and fix.

 -Ben

 On Thu, Jul 1, 2010 at 2:03 AM, Jonas Sicking jo...@sicking.cc wrote:
  On Wed, Jun 30, 2010 at 8:16 PM, Jeremy Orlow jor...@chromium.org
  wrote:
  I've thought about this more and have some additional doubts inline.
  On Thu, Jul 1, 2010 at 11:55 AM, Jonas Sicking jo...@sicking.cc
  wrote:
 
  On Wed, Jun 30, 2010 at 6:42 PM, Jeremy Orlow jor...@chromium.org
  wrote:
   On Thu, Jul 1, 2010 at 11:17 AM, Jonas Sicking jo...@sicking.cc
   wrote:
  
   Hi All,
  
   Currently the IndexedDB specification is silent on what should
   happen
   if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
   IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
   transaction. There are two possible ways we can handle this:
  
   1. We can throw an exception.
   2. We can return a IDBRequest object and asynchronously fire a
   'error'
   event on this object.
  
   The advantage of 1 is that we pretty much know that this was an
   error
   due to a bug in the web page,
 
  I don't see why this is compelling.  Many of the other errors that you
  still
  propose we fire via the callback are due to bugs in the page.
 
 
   and we can always know this
   synchronously without having to consult the database.
 
  So?  Just because we can doesn't mean we should.
 
 
   Throwing an
   error means that all the existing infrastructure for error handling
   with automatically kick in. For example any higher-level try/catch
   constructs will have an opportunity to catch the error.
   Implementations generally report uncaught exceptions to an error
   log.
   The browser will fire an 'error' event on the window which the page
   can use for further logging. Firing an error event on the other
   hand
   does not allow the browser to automatically log the error in a
   console
   as the page hasn't yet gotten a chance to handle it.
 
  Sure, but this doesn't help the majority of error conditions in
  IndexedDB.
   It also ignores the cost of handling errors in 2 different ways.
 
  I'm arguing that people generally won't want to check for all types of
  errors, such as writing in a READ_ONLY transaction. Compare to the
  Node.appendChild function which throws for a number of conditions, yet
  I've never seen anyone put a try around it and check for
  HIERARCHY_REQUEST_ERR.
 
  Additionally, anything you do can likely in theory throw exceptions.
  For example if you specify the wrong number of arguments or call a
  function that doesn't exist, this will result in an exception. However
  you rarely see people wrap try around calls to try to handle this
  type of bugs.
 
  Another example is the line:
 
  db.objectStore(myObjectStore).get(53)
 
  can result in errors both being thrown and being reported as an error
  event. If the objectStore myObjectStore doesn't exist, then the
  .objectStore() call will return null and the js engine will throw due
  to trying to call a function on a null value.
 
  Error checking is mostly useful when there are ways you can actually
  handle the error. I think generally if someone calls .put during a
  READ_ONLY transaction, they simply have a bug in their program and
  there is little they can do in terms of handling that, short of
  simply logging it.
 
   The advantage of 2 is that this is consistent with other error
   conditions, such as writing duplicate keys, disk errors during
   writing
   the database to disk, internal errors in the database, etc.
 
  The other problem is that users then need 2 sets of error handling
  routines
  for each call.  Given how difficult it is to get web developers to do
  any
  error checking, requiring 2 types of checks seems like a big downside.
 
  Actually, this is somewhat faulty math. With exceptions you can put a
  try/catch high up in a call stack to catch errors from multiple
  different locations. This will catch all errors thrown from all
  function calls inside the try. Compare that to error events, which has
  to be manually installed at every single request site. From this point
  of view, the more errors we move to being reported as exceptions, the
  more easily we're making it for developers to catch more errors.
 
  Javascript has a error reporting mechanism, exceptions. We should try
  to make good use of it.
 
   

Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-02 Thread Andrei Popescu
On Thu, Jul 1, 2010 at 2:17 AM, Jonas Sicking jo...@sicking.cc wrote:

 Additionally, the structured clone algorithm, which defines that an
 exception should synchronously be thrown if the object is malformed,
 for example if it consists of a cyclic graph. So .add/.put/.update can
 already throw under certain circumstances.


This isn't actually true for the async version of our API. The current
wording is:

If the value being stored could not be serialized by the internal
structured cloning algorithm, then an error event is fired on this
method's returned object with its code set to SERIAL_ERR and a
suitable message.

In the sync version, if the structure cloning algorithm threw, we do
throw an IDBDatabaseException with code SERIAL_ERR.

When fixing 10064,  I'll also change the spec to throw for
serialization errors in the async case.

Andrei



Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-02 Thread Andrei Popescu
On Fri, Jul 2, 2010 at 10:43 AM, Jonas Sicking jo...@sicking.cc wrote:
 Filed http://www.w3.org/Bugs/Public/show_bug.cgi?id=10064

Fixed. Please have a look, in case I missed or got anything wrong. Thanks!

Andrei



Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-02 Thread Jonas Sicking
On Fri, Jul 2, 2010 at 1:02 PM, Andrei Popescu andr...@google.com wrote:
 On Fri, Jul 2, 2010 at 10:43 AM, Jonas Sicking jo...@sicking.cc wrote:
 Filed http://www.w3.org/Bugs/Public/show_bug.cgi?id=10064

 Fixed. Please have a look, in case I missed or got anything wrong. Thanks!

For add and put you should not throw DATA_ERR if the objectStore has a
key generator.

It would be ideal if we don't have to define our own error for when
cloning fails, but instead be consistent with other APIs that use
structured clones. However right now the error thrown by the HTML5
spec is pretty useless, I filed [1] on that against the HTML5 spec.

[1] http://www.w3.org/Bugs/Public/show_bug.cgi?id=10069

Looks great otherwise, thanks!

/ Jonas



Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-02 Thread Andrei Popescu
On Fri, Jul 2, 2010 at 9:45 PM, Jonas Sicking jo...@sicking.cc wrote:
 On Fri, Jul 2, 2010 at 1:02 PM, Andrei Popescu andr...@google.com wrote:
 On Fri, Jul 2, 2010 at 10:43 AM, Jonas Sicking jo...@sicking.cc wrote:
 Filed http://www.w3.org/Bugs/Public/show_bug.cgi?id=10064

 Fixed. Please have a look, in case I missed or got anything wrong. Thanks!

 For add and put you should not throw DATA_ERR if the objectStore has a
 key generator.


Oh, I thought I added a sentence about that. Will fix on Monday.

Thanks,
Andrei



Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-01 Thread Jonas Sicking
On Wed, Jun 30, 2010 at 8:16 PM, Jeremy Orlow jor...@chromium.org wrote:
 I've thought about this more and have some additional doubts inline.
 On Thu, Jul 1, 2010 at 11:55 AM, Jonas Sicking jo...@sicking.cc wrote:

 On Wed, Jun 30, 2010 at 6:42 PM, Jeremy Orlow jor...@chromium.org wrote:
  On Thu, Jul 1, 2010 at 11:17 AM, Jonas Sicking jo...@sicking.cc wrote:
 
  Hi All,
 
  Currently the IndexedDB specification is silent on what should happen
  if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
  IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
  transaction. There are two possible ways we can handle this:
 
  1. We can throw an exception.
  2. We can return a IDBRequest object and asynchronously fire a 'error'
  event on this object.
 
  The advantage of 1 is that we pretty much know that this was an error
  due to a bug in the web page,

 I don't see why this is compelling.  Many of the other errors that you still
 propose we fire via the callback are due to bugs in the page.


  and we can always know this
  synchronously without having to consult the database.

 So?  Just because we can doesn't mean we should.


  Throwing an
  error means that all the existing infrastructure for error handling
  with automatically kick in. For example any higher-level try/catch
  constructs will have an opportunity to catch the error.
  Implementations generally report uncaught exceptions to an error log.
  The browser will fire an 'error' event on the window which the page
  can use for further logging. Firing an error event on the other hand
  does not allow the browser to automatically log the error in a console
  as the page hasn't yet gotten a chance to handle it.

 Sure, but this doesn't help the majority of error conditions in IndexedDB.
  It also ignores the cost of handling errors in 2 different ways.

I'm arguing that people generally won't want to check for all types of
errors, such as writing in a READ_ONLY transaction. Compare to the
Node.appendChild function which throws for a number of conditions, yet
I've never seen anyone put a try around it and check for
HIERARCHY_REQUEST_ERR.

Additionally, anything you do can likely in theory throw exceptions.
For example if you specify the wrong number of arguments or call a
function that doesn't exist, this will result in an exception. However
you rarely see people wrap try around calls to try to handle this
type of bugs.

Another example is the line:

db.objectStore(myObjectStore).get(53)

can result in errors both being thrown and being reported as an error
event. If the objectStore myObjectStore doesn't exist, then the
.objectStore() call will return null and the js engine will throw due
to trying to call a function on a null value.

Error checking is mostly useful when there are ways you can actually
handle the error. I think generally if someone calls .put during a
READ_ONLY transaction, they simply have a bug in their program and
there is little they can do in terms of handling that, short of
simply logging it.

  The advantage of 2 is that this is consistent with other error
  conditions, such as writing duplicate keys, disk errors during writing
  the database to disk, internal errors in the database, etc.

 The other problem is that users then need 2 sets of error handling routines
 for each call.  Given how difficult it is to get web developers to do any
 error checking, requiring 2 types of checks seems like a big downside.

Actually, this is somewhat faulty math. With exceptions you can put a
try/catch high up in a call stack to catch errors from multiple
different locations. This will catch all errors thrown from all
function calls inside the try. Compare that to error events, which has
to be manually installed at every single request site. From this point
of view, the more errors we move to being reported as exceptions, the
more easily we're making it for developers to catch more errors.

Javascript has a error reporting mechanism, exceptions. We should try
to make good use of it.

  While consistency, and only needing to check for errors one way, is
  certainly good arguments, I would argue that people won't need to
  check for calling-add-on-read-only-transactions. For properly written
  code it's not an error that will occur, and thus there is no need to
  check for it. In fact, you probably are generally better off letting
  the exception bubble all the way up and get logged or caught by
  generic error handlers.

 These are awfully bold assumptions.  Simply not catching the error is not an
 option for many web applications or libraries.  At very least, they'll need
 to add finally statements to handle such cases.

How would one handle the case of .put being called on a READ_ONLY transaction?

  Additionally, the structured clone algorithm, which defines that an
  exception should synchronously be thrown if the object is malformed,
  for example if it consists of a cyclic graph. So .add/.put/.update can
  

Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-07-01 Thread Jonas Sicking
Replying to get Ben's email to the list as it seems to be stuck in moderation...

On Thu, Jul 1, 2010 at 1:38 PM, ben turner b...@mozilla.com wrote:
 I would also point out that throwing exceptions at the call site makes
 debugging much easier in my opinion. Our error events currently don't
 include information like filename and line number where the failing
 request was generated (though I think we should add that eventually).
 Exceptions are much easier to track down, diagnose, and fix.

 -Ben

 On Thu, Jul 1, 2010 at 2:03 AM, Jonas Sicking jo...@sicking.cc wrote:
 On Wed, Jun 30, 2010 at 8:16 PM, Jeremy Orlow jor...@chromium.org wrote:
 I've thought about this more and have some additional doubts inline.
 On Thu, Jul 1, 2010 at 11:55 AM, Jonas Sicking jo...@sicking.cc wrote:

 On Wed, Jun 30, 2010 at 6:42 PM, Jeremy Orlow jor...@chromium.org wrote:
  On Thu, Jul 1, 2010 at 11:17 AM, Jonas Sicking jo...@sicking.cc wrote:
 
  Hi All,
 
  Currently the IndexedDB specification is silent on what should happen
  if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
  IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
  transaction. There are two possible ways we can handle this:
 
  1. We can throw an exception.
  2. We can return a IDBRequest object and asynchronously fire a 'error'
  event on this object.
 
  The advantage of 1 is that we pretty much know that this was an error
  due to a bug in the web page,

 I don't see why this is compelling.  Many of the other errors that you still
 propose we fire via the callback are due to bugs in the page.


  and we can always know this
  synchronously without having to consult the database.

 So?  Just because we can doesn't mean we should.


  Throwing an
  error means that all the existing infrastructure for error handling
  with automatically kick in. For example any higher-level try/catch
  constructs will have an opportunity to catch the error.
  Implementations generally report uncaught exceptions to an error log.
  The browser will fire an 'error' event on the window which the page
  can use for further logging. Firing an error event on the other hand
  does not allow the browser to automatically log the error in a console
  as the page hasn't yet gotten a chance to handle it.

 Sure, but this doesn't help the majority of error conditions in IndexedDB.
  It also ignores the cost of handling errors in 2 different ways.

 I'm arguing that people generally won't want to check for all types of
 errors, such as writing in a READ_ONLY transaction. Compare to the
 Node.appendChild function which throws for a number of conditions, yet
 I've never seen anyone put a try around it and check for
 HIERARCHY_REQUEST_ERR.

 Additionally, anything you do can likely in theory throw exceptions.
 For example if you specify the wrong number of arguments or call a
 function that doesn't exist, this will result in an exception. However
 you rarely see people wrap try around calls to try to handle this
 type of bugs.

 Another example is the line:

 db.objectStore(myObjectStore).get(53)

 can result in errors both being thrown and being reported as an error
 event. If the objectStore myObjectStore doesn't exist, then the
 .objectStore() call will return null and the js engine will throw due
 to trying to call a function on a null value.

 Error checking is mostly useful when there are ways you can actually
 handle the error. I think generally if someone calls .put during a
 READ_ONLY transaction, they simply have a bug in their program and
 there is little they can do in terms of handling that, short of
 simply logging it.

  The advantage of 2 is that this is consistent with other error
  conditions, such as writing duplicate keys, disk errors during writing
  the database to disk, internal errors in the database, etc.

 The other problem is that users then need 2 sets of error handling routines
 for each call.  Given how difficult it is to get web developers to do any
 error checking, requiring 2 types of checks seems like a big downside.

 Actually, this is somewhat faulty math. With exceptions you can put a
 try/catch high up in a call stack to catch errors from multiple
 different locations. This will catch all errors thrown from all
 function calls inside the try. Compare that to error events, which has
 to be manually installed at every single request site. From this point
 of view, the more errors we move to being reported as exceptions, the
 more easily we're making it for developers to catch more errors.

 Javascript has a error reporting mechanism, exceptions. We should try
 to make good use of it.

  While consistency, and only needing to check for errors one way, is
  certainly good arguments, I would argue that people won't need to
  check for calling-add-on-read-only-transactions. For properly written
  code it's not an error that will occur, and thus there is no need to
  check for it. In fact, you probably are generally better off 

[IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-06-30 Thread Jonas Sicking
Hi All,

Currently the IndexedDB specification is silent on what should happen
if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
transaction. There are two possible ways we can handle this:

1. We can throw an exception.
2. We can return a IDBRequest object and asynchronously fire a 'error'
event on this object.

The advantage of 1 is that we pretty much know that this was an error
due to a bug in the web page, and we can always know this
synchronously without having to consult the database. Throwing an
error means that all the existing infrastructure for error handling
with automatically kick in. For example any higher-level try/catch
constructs will have an opportunity to catch the error.
Implementations generally report uncaught exceptions to an error log.
The browser will fire an 'error' event on the window which the page
can use for further logging. Firing an error event on the other hand
does not allow the browser to automatically log the error in a console
as the page hasn't yet gotten a chance to handle it.

The advantage of 2 is that this is consistent with other error
conditions, such as writing duplicate keys, disk errors during writing
the database to disk, internal errors in the database, etc.

While consistency, and only needing to check for errors one way, is
certainly good arguments, I would argue that people won't need to
check for calling-add-on-read-only-transactions. For properly written
code it's not an error that will occur, and thus there is no need to
check for it. In fact, you probably are generally better off letting
the exception bubble all the way up and get logged or caught by
generic error handlers.

Additionally, the structured clone algorithm, which defines that an
exception should synchronously be thrown if the object is malformed,
for example if it consists of a cyclic graph. So .add/.put/.update can
already throw under certain circumstances.

Also compare to if we were using a different API strategy of making
objectStores and cursors returned from READ_ONLY transactions not have
mutating functions. In this case if someone tried to call .put(), that
also would result in a exception from the JS interpreter stating that
you're calling a function that doesn't exist.

So I would argue that we should throw for at least all transaction
violations. I.e. whenever you try to perform an action not allowed by
the current transaction. This would also cover the case of calling
createObjectStore/removeObjectStore/createIndex/removeIndex during a
non-setVersion-transaction.


There is also another case where synchronously know that an error will
be reported. We could throw when IDBCursor.update() is called when the
underlying object store uses in-line keys and the property at the key
path does not match the key in this cursor's position. In this case we
similarly immediately know that there is an error without having to
consult the database. We also generally can be sure that there is a
bug in the web page which would benefit from being reported like other
bugs are.

And like stated above, IDBCursor.update() can already throw if the
passed in object can't be structurally cloned.


Jeremy previously asked if there was a test we could use to
clearly/intuitively break error conditions into two groups. Ones that
cause exceptions to be thrown, and ones that cause error events to be
fired. I would say that errors that do not depend on what data is in
the database, but rather are clearly due to errors at the call site
should throw an exception.

Let me know what you think.

/ Jonas



Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-06-30 Thread Jeremy Orlow
On Thu, Jul 1, 2010 at 11:17 AM, Jonas Sicking jo...@sicking.cc wrote:

 Hi All,

 Currently the IndexedDB specification is silent on what should happen
 if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
 IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
 transaction. There are two possible ways we can handle this:

 1. We can throw an exception.
 2. We can return a IDBRequest object and asynchronously fire a 'error'
 event on this object.

 The advantage of 1 is that we pretty much know that this was an error
 due to a bug in the web page, and we can always know this
 synchronously without having to consult the database. Throwing an
 error means that all the existing infrastructure for error handling
 with automatically kick in. For example any higher-level try/catch
 constructs will have an opportunity to catch the error.
 Implementations generally report uncaught exceptions to an error log.
 The browser will fire an 'error' event on the window which the page
 can use for further logging. Firing an error event on the other hand
 does not allow the browser to automatically log the error in a console
 as the page hasn't yet gotten a chance to handle it.

 The advantage of 2 is that this is consistent with other error
 conditions, such as writing duplicate keys, disk errors during writing
 the database to disk, internal errors in the database, etc.

 While consistency, and only needing to check for errors one way, is
 certainly good arguments, I would argue that people won't need to
 check for calling-add-on-read-only-transactions. For properly written
 code it's not an error that will occur, and thus there is no need to
 check for it. In fact, you probably are generally better off letting
 the exception bubble all the way up and get logged or caught by
 generic error handlers.

 Additionally, the structured clone algorithm, which defines that an
 exception should synchronously be thrown if the object is malformed,
 for example if it consists of a cyclic graph. So .add/.put/.update can
 already throw under certain circumstances.

 Also compare to if we were using a different API strategy of making
 objectStores and cursors returned from READ_ONLY transactions not have
 mutating functions. In this case if someone tried to call .put(), that
 also would result in a exception from the JS interpreter stating that
 you're calling a function that doesn't exist.

 So I would argue that we should throw for at least all transaction
 violations. I.e. whenever you try to perform an action not allowed by
 the current transaction. This would also cover the case of calling
 createObjectStore/removeObjectStore/createIndex/removeIndex during a
 non-setVersion-transaction.


 There is also another case where synchronously know that an error will
 be reported. We could throw when IDBCursor.update() is called when the
 underlying object store uses in-line keys and the property at the key
 path does not match the key in this cursor's position. In this case we
 similarly immediately know that there is an error without having to
 consult the database. We also generally can be sure that there is a
 bug in the web page which would benefit from being reported like other
 bugs are.

 And like stated above, IDBCursor.update() can already throw if the
 passed in object can't be structurally cloned.


 Jeremy previously asked if there was a test we could use to
 clearly/intuitively break error conditions into two groups. Ones that
 cause exceptions to be thrown, and ones that cause error events to be
 fired. I would say that errors that do not depend on what data is in
 the database, but rather are clearly due to errors at the call site
 should throw an exception.


This would limit us in the future in terms of schema changes.  The current
async interface differs starting the transaction until the first call that
accesses/modifies data (which are all async).  If we ever allow a schema
change to happen without disconnecting all clients, it'd be possible that
the objectStore could be deleted between when the call is made and when the
transaction is actually allowed to start.

This also will limit what can be done on a background thread.  For example,
an implementation couldn't do serialization of the object on a background
thread (yes, if you did this, you'd need to make sure the main thread didn't
modify it until it finished serializing).

Because of these reasons, I'm not too excited about this particular
heuristic for when to throw vs fire an error callback.  I've thought about
it a bit and can't think of anything better though, unfortunately.

I think I'm still slightly in favor of routing all errors through onerror
callbacks and never throwing from a function that returns an IDBResult, but
I think there were some good points brought up by Jonas for why throwing on
some errors would make sense.


 Let me know what you think.

 / Jonas




Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-06-30 Thread Jonas Sicking
On Wed, Jun 30, 2010 at 6:42 PM, Jeremy Orlow jor...@chromium.org wrote:
 On Thu, Jul 1, 2010 at 11:17 AM, Jonas Sicking jo...@sicking.cc wrote:

 Hi All,

 Currently the IndexedDB specification is silent on what should happen
 if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
 IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
 transaction. There are two possible ways we can handle this:

 1. We can throw an exception.
 2. We can return a IDBRequest object and asynchronously fire a 'error'
 event on this object.

 The advantage of 1 is that we pretty much know that this was an error
 due to a bug in the web page, and we can always know this
 synchronously without having to consult the database. Throwing an
 error means that all the existing infrastructure for error handling
 with automatically kick in. For example any higher-level try/catch
 constructs will have an opportunity to catch the error.
 Implementations generally report uncaught exceptions to an error log.
 The browser will fire an 'error' event on the window which the page
 can use for further logging. Firing an error event on the other hand
 does not allow the browser to automatically log the error in a console
 as the page hasn't yet gotten a chance to handle it.

 The advantage of 2 is that this is consistent with other error
 conditions, such as writing duplicate keys, disk errors during writing
 the database to disk, internal errors in the database, etc.

 While consistency, and only needing to check for errors one way, is
 certainly good arguments, I would argue that people won't need to
 check for calling-add-on-read-only-transactions. For properly written
 code it's not an error that will occur, and thus there is no need to
 check for it. In fact, you probably are generally better off letting
 the exception bubble all the way up and get logged or caught by
 generic error handlers.

 Additionally, the structured clone algorithm, which defines that an
 exception should synchronously be thrown if the object is malformed,
 for example if it consists of a cyclic graph. So .add/.put/.update can
 already throw under certain circumstances.

 Also compare to if we were using a different API strategy of making
 objectStores and cursors returned from READ_ONLY transactions not have
 mutating functions. In this case if someone tried to call .put(), that
 also would result in a exception from the JS interpreter stating that
 you're calling a function that doesn't exist.

 So I would argue that we should throw for at least all transaction
 violations. I.e. whenever you try to perform an action not allowed by
 the current transaction. This would also cover the case of calling
 createObjectStore/removeObjectStore/createIndex/removeIndex during a
 non-setVersion-transaction.


 There is also another case where synchronously know that an error will
 be reported. We could throw when IDBCursor.update() is called when the
 underlying object store uses in-line keys and the property at the key
 path does not match the key in this cursor's position. In this case we
 similarly immediately know that there is an error without having to
 consult the database. We also generally can be sure that there is a
 bug in the web page which would benefit from being reported like other
 bugs are.

 And like stated above, IDBCursor.update() can already throw if the
 passed in object can't be structurally cloned.


 Jeremy previously asked if there was a test we could use to
 clearly/intuitively break error conditions into two groups. Ones that
 cause exceptions to be thrown, and ones that cause error events to be
 fired. I would say that errors that do not depend on what data is in
 the database, but rather are clearly due to errors at the call site
 should throw an exception.

 This would limit us in the future in terms of schema changes.  The current
 async interface differs starting the transaction until the first call that
 accesses/modifies data (which are all async).  If we ever allow a schema
 change to happen without disconnecting all clients, it'd be possible that
 the objectStore could be deleted between when the call is made and when the
 transaction is actually allowed to start.

I'm not quite following here. Even if we in the future allow
objectStores to be deleted while there are transactions open against
it, then .add/.put would still know if we're inside a READ_ONLY or
READ_WRITE transaction, no? And so could still throw an error if we're
in a READ_ONLY transaction.

By the test defined above, .put would in that situation have to fire
an error event, rather than throw, if properly called on an READ_WRITE
transaction, but where the objectStore had been deleted. This because
we would have to check with the database if the objectStore was
deleted and thus would fail the do not depend on what data is in the
database check.

 This also will limit what can be done on a background thread.  For example,
 an implementation couldn't do 

Re: [IndexedDB] Should .add/.put/.update throw when called in read-only transaction?

2010-06-30 Thread Jeremy Orlow
I've thought about this more and have some additional doubts inline.

On Thu, Jul 1, 2010 at 11:55 AM, Jonas Sicking jo...@sicking.cc wrote:

 On Wed, Jun 30, 2010 at 6:42 PM, Jeremy Orlow jor...@chromium.org wrote:
  On Thu, Jul 1, 2010 at 11:17 AM, Jonas Sicking jo...@sicking.cc wrote:
 
  Hi All,
 
  Currently the IndexedDB specification is silent on what should happen
  if IDBObjectStore.add, IDBObjectStore.put, IDBObjectStore.remove,
  IDBCursor.update or IDBCursor.remove() is called from a READ_ONLY
  transaction. There are two possible ways we can handle this:
 
  1. We can throw an exception.
  2. We can return a IDBRequest object and asynchronously fire a 'error'
  event on this object.
 
  The advantage of 1 is that we pretty much know that this was an error
  due to a bug in the web page,


I don't see why this is compelling.  Many of the other errors that you still
propose we fire via the callback are due to bugs in the page.


  and we can always know this
  synchronously without having to consult the database.


So?  Just because we can doesn't mean we should.


  Throwing an
  error means that all the existing infrastructure for error handling
  with automatically kick in. For example any higher-level try/catch
  constructs will have an opportunity to catch the error.
  Implementations generally report uncaught exceptions to an error log.
  The browser will fire an 'error' event on the window which the page
  can use for further logging. Firing an error event on the other hand
  does not allow the browser to automatically log the error in a console
  as the page hasn't yet gotten a chance to handle it.


Sure, but this doesn't help the majority of error conditions in IndexedDB.
 It also ignores the cost of handling errors in 2 different ways.


  The advantage of 2 is that this is consistent with other error
  conditions, such as writing duplicate keys, disk errors during writing
  the database to disk, internal errors in the database, etc.


The other problem is that users then need 2 sets of error handling routines
for each call.  Given how difficult it is to get web developers to do any
error checking, requiring 2 types of checks seems like a big downside.


  While consistency, and only needing to check for errors one way, is
  certainly good arguments, I would argue that people won't need to
  check for calling-add-on-read-only-transactions. For properly written
  code it's not an error that will occur, and thus there is no need to
  check for it. In fact, you probably are generally better off letting
  the exception bubble all the way up and get logged or caught by
  generic error handlers.


These are awfully bold assumptions.  Simply not catching the error is not an
option for many web applications or libraries.  At very least, they'll need
to add finally statements to handle such cases.

 Additionally, the structured clone algorithm, which defines that an
  exception should synchronously be thrown if the object is malformed,
  for example if it consists of a cyclic graph. So .add/.put/.update can
  already throw under certain circumstances.
 
  Also compare to if we were using a different API strategy of making
  objectStores and cursors returned from READ_ONLY transactions not have
  mutating functions. In this case if someone tried to call .put(), that
  also would result in a exception from the JS interpreter stating that
  you're calling a function that doesn't exist.
 
  So I would argue that we should throw for at least all transaction
  violations. I.e. whenever you try to perform an action not allowed by
  the current transaction. This would also cover the case of calling
  createObjectStore/removeObjectStore/createIndex/removeIndex during a
  non-setVersion-transaction.
 
 
  There is also another case where synchronously know that an error will
  be reported. We could throw when IDBCursor.update() is called when the
  underlying object store uses in-line keys and the property at the key
  path does not match the key in this cursor's position. In this case we
  similarly immediately know that there is an error without having to
  consult the database. We also generally can be sure that there is a
  bug in the web page which would benefit from being reported like other
  bugs are.
 
  And like stated above, IDBCursor.update() can already throw if the
  passed in object can't be structurally cloned.
 
 
  Jeremy previously asked if there was a test we could use to
  clearly/intuitively break error conditions into two groups. Ones that
  cause exceptions to be thrown, and ones that cause error events to be
  fired. I would say that errors that do not depend on what data is in
  the database, but rather are clearly due to errors at the call site
  should throw an exception.
 
  This would limit us in the future in terms of schema changes.  The
 current
  async interface differs starting the transaction until the first call
 that
  accesses/modifies data (which are all async).