-
Notifications
You must be signed in to change notification settings - Fork 147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
set
rejects with null
- possible bug in error handling
#163
Comments
Am I right that you don't have a reproducible case for this, but you're seeing it in logging? |
@jakearchibald Correct; I have not been able to reproduce it locally. |
No problem, thanks for all the details. I'll see if I can figure something out. |
I have what appears to be a reproducible test case: https://codesandbox.io/s/goofy-kowalevski-zncg4g?file=/src/index.js I've observed this in Safari for Mac and Safari for iPhone. Each time you click "set value", it writes a bunch of values to IndexedDB. After doing this ~19 times, Safari should prompt you to allow more storage. If you choose to not allow, then the error handler is called with (I tried other failure modes, such as saving a non-structured-cloneable object or exceeding the quota in Chrome. The error handler received the error in these cases, as intended. That did not match my understanding of the spec, so I may have misunderstood things.) |
idb-keyval reported IDBTransaction.error if a `set` operation failed. However, based on my reading of the IndexedDB spec, and based on testing, IDBTransaction.error may not be set at the time the `error` event fires. This can be seen under the following scenarios: - If IDBObjectStore.add fails because a key already exists - If a quota is exceeded in Safari - If the transaction is aborted immediately after an IndexedDB operation is requested The `error` event's EventTarget should give the specific IDBRequest that failed, and that IDBRequest's error property should always be populated, so accessing `event.target` should fix this issue. I tried using a similar approach for `abort` events, but it did not work; strangely, the abort event's target appears to be the IDBOpenRequest associated with the transaction, rather than the transaction itself, so it does not have a usable error property. This PR also adds limited unit tests to cover this scenario. Aborting a transaction immediately after issuing a database request appears to be a good way to force an error. Fixes jakearchibald#163
idb-keyval reported IDBTransaction.error if a `set` operation failed. However, based on my reading of the IndexedDB spec, and based on testing, IDBTransaction.error may not be set at the time the `error` event fires. This can be seen under the following scenarios: - If IDBObjectStore.add fails because a key already exists - If a quota is exceeded in Safari - If the transaction is aborted immediately after an IndexedDB operation is requested The `error` event's EventTarget should give the specific IDBRequest that failed, and that IDBRequest's error property should always be populated, so accessing `event.target` should fix this issue. I tried using a similar approach for `abort` events, but it did not work; strangely, the abort event's target appears to be the IDBOpenRequest associated with the transaction, rather than the transaction itself, so it does not have a usable error property. This PR also adds limited unit tests to cover this scenario. Aborting a transaction immediately after issuing a database request appears to be a good way to force an error. Fixes jakearchibald#163
See #164 for a potential fix. Some additional notes from my testing:
|
We've occasionally seen calls to
set
fail with anull
error. I'd like to know whyset
calls are failing, so I'm trying to figure out what can cause this.From reading the specs and experimenting, I believe that
null
may happen under the following circumstances:The transaction's
abort
method is called. Since idb-keyval doesn't expose its transaction object, this seems unlikely, but maybe it could happen if, e.g., the web page is reloaded in mid-write? (I don't have a deep understanding of how web APIs handle things like page reloads.)A request error occurs. If I understand correctly, a request's
error
event bubbles to the transaction (because the request's parent is the transaction). After theerror
event is done, it aborts the transaction, which setsIDBTransaction.error
and fires the transaction'sabort
event.set
causes its promise to be rejected on either anerror
event (which may bubble from the request) or anabort
event (see here and here). Since theerror
event fires first and does not setIDBTransaction.error
, a request error causes the promise to be rejected without the error being available.Are you aware of other circumstances that may cause a
null
rejection?Can you update idb-keyval to expose the error if the request fails?
See https://codesandbox.io/s/serene-danilo-lseiit?file=/src/index.js for a demonstration. It looks like a transaction's error event could get the error details via event.target.error (although that doesn't fit as well with idb-keyval's as-small-as-possible goal).
The text was updated successfully, but these errors were encountered: