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

app.delete() timing out w/Firestore initialized starting in 4.12 #605

Closed
jamesdaniels opened this issue Mar 29, 2018 · 7 comments
Closed

Comments

@jamesdaniels
Copy link
Member

jamesdaniels commented Mar 29, 2018

[REQUIRED] Describe your environment

  • Operating System version: any
  • Firebase SDK version: 4.12
  • Firebase Product: firestore / angularfire2

[REQUIRED] Describe the problem

Steps to reproduce:

The angularfire2 test suite uses Firestore with persistence enabled and the tests are failing w/timeouts as of the release of 4.12.

See here: https://travis-ci.org/angular/angularfire2/builds/359661112?utm_source=github_status&utm_medium=notification

This could be A) a unpatched timer not playing nice with Zone.js B) a deeper issue; I haven't done a lot of digging yet. My first action is going to be making our pull requests use the yarn.lock so we aren't broken. Will dig in further after that and update this issue.

Relevant Code:

https://github.com/angular/angularfire2/blob/master/src/firestore/firestore.spec.ts

@mikelehen
Copy link
Contributor

This is hopefully fixed by #592 which should be going out in the next release. You can try npm install firebase@next and see if that fixes it...

@jamesdaniels
Copy link
Member Author

I'm still seeing it locally, my timeout is 5 seconds. I'll up and see if it helps but not anticipating that the latest patch fixes unforunately.

@jamesdaniels
Copy link
Member Author

jamesdaniels commented Mar 30, 2018

Found the root issue, calling delete() on the app instance is timing out; we do this in the afterEach. Further, the only occurs when firestore has been initialized and we've done nothing of significance (get, put, etc.); tearing down almost immediately.

@jamesdaniels jamesdaniels changed the title Seeing timeouts while in offline mode starting in 4.12 app.delete() timing out w/Firestore initialized starting in 4.12 Mar 30, 2018
@mikelehen
Copy link
Contributor

I've been able to reproduce this outside of angular with the following code:

  firebase.initializeApp(config);
  // create ref to start client initialization.
  firebase.firestore().doc('foo/bar')

  console.log('Calling delete...');
  await firebase.app().delete();
  console.log('Deleted.');

Digging in, it looks like calling app.delete() ends up causing our setUserChangeListener() callback in FirestoreClient.start() to never get called.

this.credentials.setUserChangeListener(user => {

This means we never resolve the initializationDone promise and so FirestoreClient.start() blocks the AsyncQueue indefinitely and so then our FirestoreClient.shutdown() queued task never runs...

And this in turn causes app.delete() to hang.

So I think auth is getting deleted before us and neutering our underlying app.INTERNAL.addTokenListener() call:

(this.app as _FirebaseApp).INTERNAL.addAuthTokenListener(

I'm not sure why this broke in 4.12.0. Auth must have changed something in their delete() implementation or the order of delete() calls must have changed.

Unfortunately I'm not sure what the best fix is and I'm going to be away for the next week, but I think this is unlikely to affect actual apps since it only happens if you call app.delete() immediately without actually doing anything. As long as you wait for a single firestore operation to complete first, then you won't hit this.

@jamesdaniels
Copy link
Member Author

@mikelehen now that we've gotten to the bottom of it, I agree, low risk. In the meantime I've implemented a work around in the AF2 test suite so my Functions work isn't blocked.

Enjoy your week off and thanks for hanging in there with me today!

@mikelehen
Copy link
Contributor

Verified this still repros with latest bits (5.8.0).

@DellaBitta DellaBitta added the bug label Jun 7, 2023
@hsubox76
Copy link
Contributor

As part of an issue cleanup effort, we are closing old issues related to versions before 9. The SDK had a large rewrite for version 9 to introduce the modular API, and issues opened under previous versions may no longer apply or may manifest in a different way. If you are still experiencing the same issue with the latest version of the Firebase JS SDK, please open a new issue with as much information as you can provide about how you are currently experiencing it. Thank you!

@firebase firebase locked and limited conversation to collaborators Aug 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants