Re: [w3c/IndexedDB] Specify the error code for failures when reading a value from disk (PR #430)

@inexorabletash commented on this pull request.

Overall looks good, a few notes/suggestions.

> @@ -5642,7 +5649,8 @@ To <dfn>retrieve a value from an object store</dfn> with
 
 1. If |record| was not found, return undefined.
 
-1. Let |serialized| be of |record|'s [=/value=].
+1. Let |serialized| be |record|'s [=/value=]. If an error occurs while reading the value from the

Can you update the prose introducing the algorithm to indicate that it may return an error (DOMException) ?

Reviewing the rest of the spec, https://w3c.github.io/IndexedDB/#asynchronously-execute-a-request does have prose like _"if result is an error ..."_ which should process this failure correctly. I note that if we ever allowed storing a DOMException then the behavior would be ambiguous and we'd need to be more explicit here (and elsewhere) about success vs. failure, but that's not currently a problem.

> @@ -5664,7 +5672,8 @@ store</dfn> with |targetRealm|, |store|, |range| and optional |count|, run these
 
 1. [=list/For each=] |record| of |records|:
 
-    1. Let |serialized| be |record|'s [=/value=].
+    1. Let |serialized| be |record|'s [=/value=]. If an error occurs while reading the value from the

Same as above - can you update the prose introducing the algorithm?

> @@ -1866,6 +1866,13 @@ usage.
       not be found.
     </td>
   </tr>
+  <tr>
+    <td>{{NotReadableError}}</td>
+    <td>
+      The operation failed because the file(s) containing the requested

Maybe "underlying storage" instead of "file(s)" since we don't require that it actually bottoms out in files?

> @@ -1892,8 +1899,8 @@ usage.
   <tr>
     <td>{{UnknownError}}</td>
     <td>
-      The operation failed for reasons unrelated to the database
-      itself and not covered by any other errors.
+      The operation failed for transient reasons unrelated to the
+      database itself and not covered by any other error.

In my opinion, this text scopes `UnknownError` a bit too much. If it's any other type of error, a developer should be able to look at the spec and deduce exactly what the cause was; `UnknownError` is basically for anything unspecified.

Maybe "or not covered" instead of "and not covered" ?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/w3c/IndexedDB/pull/430#pullrequestreview-2355080365
You are receiving this because you are subscribed to this thread.

Message ID: <w3c/IndexedDB/pull/430/review/2355080365@github.com>