Skip to content
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

Try to reopen database in case of unexpected closure #175

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

zarko-tg
Copy link

It does not seems to be much activity on this project but here's comes a rather important PR for which hopefully we get feedback.

There (still) seems to be a problem with iOS Safari, where the database connection is unexpectedly closed. This is difficult to reproduce but it appears that it happens when an app has been active in the background for some time and the internet connection has been lost (and then maybe reestablished). It's also unclear whether this is caused by a bug in iOS Safari or the occasional unexpected closure is actually valid - behaving by design.

From what I could see idp-keyval does not seem to have any recovery logic to handle this case. I guess in practice this could mean that the functionality is broken until one kills/restarts the app.

Our error logs are now clogged with "InvalidStateError: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing." reports and we're not really sure what's the effect of this on around 10% of our users/user sessions.

Same/similar reported issues are:

I've noticed that projects like Firebase (storage related) and Dexie have already taken measures for database connection recovery.

This PR is sort of a blind attempt to address the issue given the difficulty to reproduce, but we're testing it atm. It doesn't seem to cause any new issues/problems.

@bastienlemaitre
Copy link

@jakearchibald can you merge this PR ? Thx

@jakearchibald
Copy link
Owner

Thanks for the ping. I'll take a look.

@jkukatzkihuebinet
Copy link

Implemented this yesterday for our app and seems to have mitigated the issue running on Chrome Android Devices.
+1 on getting this merged

@zarko-tg
Copy link
Author

Hi, it's been some time now since I filed this PR, so regardless whether it gets accepted or not, I feel like providing a little update for those affected.

We've been using a patched idb-keyval with the PR's code in a production Ionic/Capacitor app. It doesn't seems to cause any new problem but I cannot conclude that it actually helps much. That may sound strange but the reasons are the following:

  • I still haven't been able to properly reproduce the original issue, even after searching for similar cases online.
  • The original issue might be a side effect or a consequence of a bigger issue with iOS/Safari, namely that the database cannot be reopened, which is a major bummer. Some say a (technical) workaround is to kill/restart the app or restart the device. There are a number of bugs filed for WebKit and/or Capacitor on the matter, but sadly close to zero action from people who should have the knowledge to fix or remedy the issue.
  • Because of the latter, our production error logs are now clogged / over quota, and we're not really sure what's going on out there. We don't know what it means in practice for the user experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants