JaneDawson commented 7 years ago

Hi @adriancarriger,

as suggested here, I'm using the undocumented reset() function before logging out my users from my Ionic 3 app to prevent Firebase permission-denied-errors.

This works pretty well - no permission-denied errors at all after adding the reset() call. However, unfortunately it results in another error which is now shown twice on any logout function call:

vendor.js:1 ERROR Error: Uncaught (in promise): TypeError: Cannot read property 'loaded' of undefined(…)

Is this somehow related to your code?

My code for the logout call: homePage:

performLogout(){
   this.authService.logoutUser().then(() => {
      this.nav.setRoot(Page2);
   });
}

authService:

constructor(public afAuth: AngularFireAuth,
            public db: AngularFireOfflineDatabase) { }

logoutUser() {
    this.db.reset();
    return this.afAuth.auth.signOut();
  }

Ionic Info:

cli packages: (D:\path)

    @ionic/cli-plugin-cordova       : 1.6.2
    @ionic/cli-plugin-ionic-angular : 1.4.1
    @ionic/cli-utils                : 1.7.0
    ionic (Ionic CLI)               : 3.7.0

global packages:

    Cordova CLI : 7.0.1

local packages:

    @ionic/app-scripts : 2.0.2
    Cordova Platforms  : android 6.2.3
    Ionic Framework    : ionic-angular 3.5.3

System:

    Android SDK Tools : 25.2.5
    Node              : v8.2.1
    OS                : Windows 10
    npm               : 5.3.0

JaneDawson commented 7 years ago

No idea?

adriancarriger commented 7 years ago

Hi @JaneDawson I think it would help to gather a bit more info to help debug your issue:

Questions

What version of AngularFire2 Offline are you using?
Are you using a list, object or both?
Is there any more info inside of the undefined(…)?
Would you be able to post a Plunker that reproduces the issue?

Does the issue still persist if you use the following test code for logoutUser?

// This code is just for testing
logoutUser() {
return new Promise(resolve => {
this.db.reset();
setTimeout(() => {
  this.afAuth.auth.signOut()
    .then(() => resolve());
}, 1000);
}) 
}

Thoughts

I noticed that your error says "(in promise)", and that it wants the property loaded. There are only two places (1, 2) in the Afo code that have .loaded inside of a promise. Does that somehow seem related to your issue/code?

Hopefully this helps us get closer to resolving your issue. Thanks! 👍

JaneDawson commented 7 years ago

Hi @adriancarriger.

Thank you very much for your answer and the time you take dealing with my issues. I appreciate that and I really admire the work you are doing.

Regarding your questions: 1) I wasn't using the most current version, actually. I now updated to version 4.2.4. The issue still occurs, however now I just receive one Error: ERROR TypeError: Cannot set property 'loaded' of undefined(…) Meaning we are dealing with setting loadednow, instead of just reading it.

2) I use both lists and objects.

3) Yes, there is some more info inside the (...). Screenshot: http://i.imgur.com/ViuPxtH.png

4) Unfortunately, I never created a Plunker before and wouldn't now how to do this properly. If we manage to solve it without a Plunker, I would prefer it...

5) Yes, the error still persists with the timeout. When timing out for 5 seconds, it becomes pretty clear, that the error is thrown immediately when calling the logout function (and hence the reset()) and doesn't wait for the timeout to finish.

I hope this somehow helps you?

adriancarriger commented 7 years ago

@JaneDawson thanks for the update! I'll work on creating a demo that reproduces your setup to try and reproduce the issue. If you'd like to do more testing you might try using only object and another test using only lists to see if the issue is present in both situations.

JaneDawson commented 7 years ago

Hi @adriancarriger.

I did some further testing. Running on an iOS device I receive the error TypeError: undefined is not an object (evaluating 'e.listCache[t].loaded=!0') — vendor.js:0. Hence, I suggest that the issue is connected to lists, not objects. As you linked before, it will most probably be this part of the code.

Screenshot of Error on iOS: http://i.imgur.com/GX5acY8.png

Hope this helps...

adriancarriger commented 7 years ago

Hi @JaneDawson, I've created a demo to try and reproduce the issue. So far the demo doesn't result in the error you have so it sounds like I might be missing some details.

Demo flow

Here's how the demo is laid out:

Starts with the Home component
User signs in and is redirected to the About component
The about component displays data that can only be read while signed in
User signs out, offline data is reset and is redirected back to the home component

Next steps

Do you notice any important differences between your implementation and the demo?
Would you be able to post more of your code?

Thanks!

JaneDawson commented 7 years ago

Hi @adriancarriger,

encouraged by your demo, I did some further research and can locate the error more narrowly, now.

My performLogout() function is a bit more complex than I have shown in my initial post. Basically, I delete the users device token from the database on logout:

HomePage

performLogout(){

      // delete users device token
      this.pushNotificationService.deleteToken(this.userId).then( () => {

        this.authService.logoutUser().then(() => {
          this.nav.setRoot(ExplainSlidesPage);
        })
        .catch(error => {
          this.errorService.handleError(error);
        });

      })
      .catch(error => {
        this.errorService.handleError(error);
      });
    }

  }

PushNotificationService

constructor(public db: AngularFireOfflineDatabase,
                     public db_online: AngularFireDatabase
             ) { }

deleteToken(userId){
    // Prepares the token deletion
    return new Promise ( (resolve, reject) => {

      this.getCurrentToken().then(tokenToDelete => {

        this.getUsersDeviceTokensOnce(userId).subscribe(deviceTokens => {

          for (let token of deviceTokens){
            if (token.$value === tokenToDelete){

              this.executeTokenDeletion(userId, token.$key)
                .then( res => resolve(res))
                .catch(err => reject(err));

            }
          }

          console.log("tokenToDelete doesn't exist.");
          resolve();

        }, error => reject(error));

      })
      .catch( err => reject(err));
    });    
  }

getCurrentToken() {
    // returns the current Device Token.
    return new Promise((resolve, reject) => {

      FCMPlugin.getToken(token => {
        resolve(token);
      }, (err) => {
        reject(err);
      });

    });
  }

getUsersDeviceTokensOnce(userId){
    return (<any>this.db.list('/users/'+userId+'/device_token')).take(1);
  }

executeTokenDeletion(userId, fbTokenId){
    return (<any>this.db.list('/users/'+userId+'/device_token/'+fbTokenId)).remove();
  }

authService

constructor(public afAuth: AngularFireAuth,
            public db: AngularFireOfflineDatabase) { }

logoutUser() {
    this.db.reset();
    return this.afAuth.auth.signOut();
  }

As you can see, there are essentially two AngularFire2-Offline-operations called on logout: getUsersDeviceTokensOnce () and executeTokenDeletion(). If I change these to call AngularFire2 (db_online) instead, the error goes away. However, then I receive several Permission Denied errors regarding the transactions node of my firebase DB. (This is something that doesn't make sense to me at all, because I always read and write from/to this node using AngularFire2-Offline and never AngularFire2, hence it should be covered by the reset() call??)

Does all this somehow makes sense to you? Would be very grateful if you shed some light on all that...

JaneDawson commented 7 years ago

In addition: If I reduce my logout function to the following, I don't receive any errors at all (not even the transaction-node related errors):

performLogout(){

        this.authService.logoutUser().then(() => {
          this.nav.setRoot(ExplainSlidesPage);
        })
        .catch(error => {
          this.errorService.handleError(error);
        });

  }

I don't see why...

JaneDawson commented 7 years ago

Any idea, @adriancarriger?

adriancarriger commented 7 years ago

Hi @JaneDawson, I'm not sure just from looking at it, but I'll try making my demo mirror the recent code you posted and see if I can reproduce.

JaneDawson commented 7 years ago

Hi @adriancarriger. And thank you (again) for all your effort. I'm currently trying to rebuild the full push notification thing, using the phonegap-push-plugin instead of the cordova-plugin-fcm. I hope to be able to prevent the error by this. So please, don't put to much effort in this for now. I'll let you know if I was able to solve this error. Thank you!

adriancarriger commented 7 years ago

@JaneDawson thanks for the update! I threw together some code that is close to what you showed above, but I didn't have time to finish it. If you still have issues with the new plugin just let me know what code should be different so I can make changes to see if the issue is present. Thanks and good luck! 👍

JaneDawson commented 7 years ago

Hi @adriancarriger. It looks very promising. I wasn't able to reproduce this issue with the new plugin. Hence, I will close this for now... Thank you!

adriancarriger / angularfire2-offline

reset() results in “Cannot read property ‘loaded’ of undefined” Error #58

Questions

Thoughts

Demo flow

Next steps