localForage icon indicating copy to clipboard operation
localForage copied to clipboard

Reliability issues in v1.5.5

Open AshleyScirra opened this issue 8 years ago • 9 comments

We use localforage in our web app Construct 3. It's a great library. However we sometimes get rare and mysterious errors when writing to storage. To investigate this further we've set up analytics to log any rejections when calling setItem(). We updated to 1.5.5 because we wanted the fix in #694 (i.e. reconnect to DB after InvalidStateError), particularly since many users have long-duration sessions in our web app. However once we made the swap, there's been no clear improvement in reliability.

Initially we were on 1.5.0 and, excluding QuotaExceededError, our top errors (covering a 30-day period) with this version were:

  1. TimeoutError: Transaction timed out due to inactivity.
  2. DataError: Failed to write blobs.
  3. InvalidStateError: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
  4. [object DOMError] (guess our analytics system couldn't read this one...)
  5. AbortError: The transaction was aborted, so the request cannot be fulfilled.
  6. UnknownError: Internal error committing transaction.

I was hoping 1 and 3 would be fixed in 1.5.5.

After updating to 1.5.5 our top errors now look like this:

  1. DataError: Failed to write blobs.
  2. TimeoutError: Transaction timed out due to inactivity.
  3. NotFoundError: Failed to execute 'transaction' on 'IDBDatabase': One of the specified object stores was not found.
  4. UnknownError: Internal error committing transaction.
  5. AbortError: The transaction was aborted, so the request cannot be fulfilled.

My main concern is that "NotFoundError: Failed to execute 'transaction' on 'IDBDatabase': One of the specified object stores was not found." is an entirely new kind of error that we never saw before prior to 1.5.5. Perhaps this is a localforage bug? I'm not sure what more evidence I can provide, but it might be worth reviewing the code looking our for a race condition or some such. It's also possible, but hard to prove, that every time InvalidStateError would have happened previously, we now get a NotFoundError instead, which might suggest an issue with how the database connection is recovered.

Additionally, the "fix for long-running sessions" patch does not appear to have completely resolved the problem: we still get a lot of "TimeoutError: Transaction timed out due to inactivity.". Should this have been fixed in #694? It looks like it specifically targeted InvalidStateError, which has indeed dropped off our error reports, but we originally had more TimeoutError anyway. Perhaps the patch could be updated to fix that kind of error too?

As for "failed to write blobs", I've no idea why that happens!

Are there any meaningful fixes in 1.5.6? The changelog does not appear to suggest so. Should I try updating anyway?

AshleyScirra avatar Jan 29 '18 12:01 AshleyScirra

Here's a theory: when the DB times out and would throw InvalidStateError, if you then create two transactions simultaneously, maybe both fail and it races _tryReconnect. It doesn't look like there's any kind of provision to only do it once even if multiple transactions are made while it's reconnecting.

AshleyScirra avatar Jan 29 '18 12:01 AshleyScirra

Thanks for the detailed report @AshleyScirra After a first read it seems that waiting for _deferReadiness inside _tryReconnect would fix that race condition. I will try to tackle this. PRs are always welcome though.

thgreasi avatar Jan 29 '18 13:01 thgreasi

Did this get addressed by 1.7.3? It looks like there were updates to _tryReconnect, but it still calls _deferReadiness without waiting for it.

AshleyScirra avatar Dec 18 '18 16:12 AshleyScirra

No afaik. I hope I can invest some time on LF during Christmas though.

thgreasi avatar Dec 19 '18 06:12 thgreasi

Likewise. I’m really behind after working on the WordPress 5.0 release, but I should have some free time soon.

tofumatt avatar Dec 19 '18 11:12 tofumatt

Any updates on this?

johannesjo avatar Oct 10 '19 15:10 johannesjo

Unfortunately not, sorry šŸ˜ž

We'll definitely provide updates here when we have any, but I've still not been able to devote the time to things I want to right now.

tofumatt avatar Oct 10 '19 15:10 tofumatt

In the end we just moved to our own lightweight wrapper over IndexedDB, and all the errors relating to DataError, InvalidStateError, NotFoundError, UnknownError and AbortError went away. (Still a few lingering TimeoutErrors, but no idea why.) These days I don't think there's much reason to have any backends other than IndexedDB and in-memory store, which makes much of localforage redundant. So I'd recommend moving off localforage and using something like idb-keyval instead.

AshleyScirra avatar Oct 10 '19 15:10 AshleyScirra

@AshleyScirra I also moved to idb-keyval, but my users are still encountering stability issues and deleted data, which seem to be related to IndexedDB in general. May I ask what your experience has been with this?

johannesjo avatar Sep 29 '20 11:09 johannesjo