Try to reopen database in case of unexpected closure #175
Conversation
|
@jakearchibald can you merge this PR ? Thx |
|
Thanks for the ping. I'll take a look. |
|
Implemented this yesterday for our app and seems to have mitigated the issue running on Chrome Android Devices. |
|
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:
|
|
We're also seeing this issue on our production app and I just realized about the existence on this PR. Do we know if this going to be merged anytime soon? |
| // Meant to be used only in specific tests | ||
| export async function closeDatabase(dbName: string) { | ||
| if (!databases[dbName]) { | ||
| console.assert(true, `Could not find database "${dbName}" to close.`); |
|
@rgomezr391 I'm setting aside time in the next couple of weeks to pick up tickets on this project |
|
We encountered the same problem |
|
seeing the same issue as well. any progress on this? |
|
we are facing similar issue on safari, any progress on this, are the changes fine and we are planning to merge this? |
|
Does anyone have a good way to reproduce the issue? |
|
I do not currently. But I just wrapped up my usage in some try/catches with consoles about keys and payloads. I am going to push it out into the wild now and hopefully I can have some feedback in a few days. 👍 |
|
I've attempted a smaller fix in 32443d0 and it's published as 6.2.2. Please test this folks. Let me know if it solves the issue. It relies on Safari firing the close event when it closes the database. If it doesn't, I can add more code to try to detect the error and trigger a reopen-and-retry. |
|
@jakearchibald Your fix has definitely helped. Thanks! We saw
about 140 times in the 7 days before updating to v6.2.2, but only 1 time in the 7 days after. I'm not sure if it's related, but a new error has appeared about 30 times in the 7 days after:
Perhaps that was just being masked/trumped by the previous error? |
|
I can report the same as above. I am receiving substantially fewer errors since the update. And I am also receiving the |
|
Are those errors also Safari only? I assume so, since I see that string in WebKit's source. |
|
Yes, all are from WebKit (Mobile Safari or Chrome on iOS). |
|
Sigh. It's so funny that they put "refresh the page" in an error that's developer facing, not user facing. |
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:
Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.occurs #165I'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.