Provide better error message for failed shutdown

[schmidt-sebastian]

This changes the error message for terminate() to state that the client is now in an undefined state.

We could probably make shutdown idempotent for IndexedDB failures, but it might be quite an engineering feet that spans more than just Firestore (since we also need to delete the FirebaseApp). I am not opposed to doing this, but would like to propose this simple "solution" for now.

Addresses #2755

[google-oss-bot]

Binary Size Report

Affected SDKs

• @firebase/firestore

  Type	Base (36be62a)	Head (192d7c2)	Diff
  browser	249 kB	249 kB	-169 B (-0.1%)
  esm2017	195 kB	195 kB	-30 B (-0.0%)
  main	475 kB	474 kB	-157 B (-0.0%)
  module	247 kB	246 kB	-147 B (-0.1%)
  react-native	195 kB	195 kB	-30 B (-0.0%)

• @firebase/firestore/exp

  Type	Base (36be62a)	Head (192d7c2)	Diff
  browser	188 kB	188 kB	-30 B (-0.0%)
  main	467 kB	467 kB	-161 B (-0.0%)
  module	188 kB	188 kB	-30 B (-0.0%)
  react-native	188 kB	188 kB	-30 B (-0.0%)

• @firebase/firestore/lite

  Type	Base (36be62a)	Head (192d7c2)	Diff
  browser	64.3 kB	64.2 kB	-68 B (-0.1%)
  main	142 kB	142 kB	-387 B (-0.3%)
  module	64.3 kB	64.2 kB	-68 B (-0.1%)
  react-native	64.4 kB	64.3 kB	-68 B (-0.1%)

• @firebase/firestore/memory

  Type	Base (36be62a)	Head (192d7c2)	Diff
  browser	187 kB	187 kB	-74 B (-0.0%)
  esm2017	146 kB	146 kB	+55 B (+0.0%)
  main	349 kB	349 kB	-68 B (-0.0%)
  module	185 kB	185 kB	-52 B (-0.0%)
  react-native	146 kB	146 kB	+55 B (+0.0%)

• firebase

  Type	Base (36be62a)	Head (192d7c2)	Diff
  firebase-firestore.js	288 kB	287 kB	-144 B (-0.1%)
  firebase-firestore.memory.js	227 kB	227 kB	-50 B (-0.0%)
  firebase.js	821 kB	821 kB	-146 B (-0.0%)

Test Logs

• Base (36be62a8): https://github.com/firebase/firebase-js-sdk/actions/runs/196873691
• Head (192d7c2c): https://github.com/firebase/firebase-js-sdk/actions/runs/198031632

[wilhuff]

If the Firestore instance fails to terminate, what's the user's recourse? Should they retry? Could they retry? Other entry points like listen are retryable and it seems like this aspect should be consistent. At the very least, the error message here or our comments should indicate what the user should do.

Note that users may be calling terminate directly and may not be deleting the Firebase App. I agree we can't solve the app deletion case without considerable effort, but it seems like solving the standalone terminate case is something we could handle.

I'm not sure it helps, but in the iOS port this has been reworked slightly so that the async queue is flagged as shut down first and then the termination action is enqueued (I haven't ported this yet because of higher priority work). In the arrangement the block that shuts everything down can run repeatedly. It seems like this change would make terminate retryable, no?

[changeset-bot]

💥 No Changeset

Latest commit: b77597f

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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