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

Re-open IndexedDB if closed #3535

Merged
merged 6 commits into from
Aug 5, 2020
Merged

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Jul 31, 2020

This PR addresses an IndexedDb error that may occur when a transaction is first opened. The error The database connection is closing can be reproduced locally by invoking SimpleDb.close() and then calling db.transaction and this PR fixes at least this case by extending the IndexedDB retry code to include the code around db.transaction and by closing and re-opening the IndexedDB instance.

Fixes #3495

@changeset-bot
Copy link

changeset-bot bot commented Jul 31, 2020

🦋 Changeset is good to go

Latest commit: 5596625

We got this.

This PR includes changesets to release 10 packages
Name Type
@firebase/firestore Patch
firebase Patch
firebase-exp Patch
firebase-firestore-integration-test Patch
@firebase/testing Patch
firebase-browserify-test Patch
firebase-package-typings-test Patch
firebase-messaging-selenium-test Patch
firebase-typescript-test Patch
firebase-webpack-test Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jul 31, 2020

Binary Size Report

Affected SDKs

  • @firebase/analytics

    Type Base (7f9b3d9) Head (b9cb0c3) Diff
    esm2017 9.66 kB 9.89 kB +229 B (+2.4%)
    main 10.9 kB 11.1 kB +225 B (+2.1%)
    module 10.6 kB 10.8 kB +234 B (+2.2%)
  • @firebase/firestore

    Type Base (7f9b3d9) Head (b9cb0c3) Diff
    browser 247 kB 248 kB +434 B (+0.2%)
    esm2017 193 kB 194 kB +229 B (+0.1%)
    main 472 kB 472 kB +615 B (+0.1%)
    module 245 kB 245 kB +412 B (+0.2%)
    react-native 194 kB 194 kB +229 B (+0.1%)
  • @firebase/firestore/exp

    Type Base (7f9b3d9) Head (b9cb0c3) Diff
    browser 187 kB 187 kB +229 B (+0.1%)
    main 465 kB 465 kB +615 B (+0.1%)
    module 187 kB 187 kB +229 B (+0.1%)
    react-native 187 kB 187 kB +229 B (+0.1%)
  • firebase

    Type Base (7f9b3d9) Head (b9cb0c3) Diff
    firebase-analytics.js 28.0 kB 28.3 kB +318 B (+1.1%)
    firebase-firestore.js 286 kB 286 kB +407 B (+0.1%)
    firebase.js 819 kB 820 kB +604 B (+0.1%)

Test Logs

@schmidt-sebastian schmidt-sebastian changed the title WIP Re-open IndexedDB if closed Re-open IndexedDB if closed Aug 4, 2020

request.onblocked = () => {
reject(
new IndexedDbTransactionError(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mostly a copy of the code above, but this line changed from FirestoreError to IndexedDbTransactionError.

Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

db.close();
await db.runTransaction('readwrite', V1_STORES, () =>
PersistencePromise.resolve()
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't make an assertion, which is usually a sign that something's missing. If this would implicitly fail somehow it's worth adding a comment about that or perhaps adding a does not throw expectation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how expect().doesNotThrow() interacts with Promises, so I added a comment instead.

@wilhuff wilhuff assigned schmidt-sebastian and unassigned wilhuff Aug 5, 2020
@schmidt-sebastian schmidt-sebastian merged commit 36be62a into master Aug 5, 2020
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/indexeddbfailures branch August 10, 2020 19:14
@google-oss-bot google-oss-bot mentioned this pull request Aug 11, 2020
@firebase firebase locked and limited conversation to collaborators Sep 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firestore breaks on mobile safari if network is lost while app is in the background.
3 participants