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

requireUser sometimes hangs #22

Open
mxab opened this issue Apr 20, 2016 · 48 comments
Open

requireUser sometimes hangs #22

mxab opened this issue Apr 20, 2016 · 48 comments

Comments

@mxab
Copy link

mxab commented Apr 20, 2016

Hi,

we experience some resolving/rejecting issue in our application. we call requireUser but the then handlers are never called.

I started todo some logging in the metoer-angular-auth but it looks ok

it autoruns, finds a valid user, enqueues the deferred.resolve int the $$afterFlush, the Tracker.afterFlush gets also called, but then it disappears.

any ideas what the problem might be?

@DAB0mB
Copy link
Collaborator

DAB0mB commented Apr 20, 2016

@mxab I tried to test this issue and it all seems fine for me. I tested it using angular-meteor-whatsapp project, I used requireUser instead of awaitUser before resolving the routes (see referene), and I even tried to fulfill the promise in ChatsCtrl initialization (see reference) and it worked properly as expected. I used [email protected], [email protected] and [email protected]. Are you sure you didn't miss anything? Please provide me an example so I can analyze your issue.

@mxab
Copy link
Author

mxab commented Apr 20, 2016

it is really hard to reproduce it as we experience it randomly. I will try to replace it with awaitUser. Out of curiosity, the afterFlush seems very intentional in the requireUser part. I assume I should also do a Tracker.afterFlush then?

DAB0mB added a commit to DAB0mB/angular-meteor-auth that referenced this issue Apr 20, 2016
@DAB0mB
Copy link
Collaborator

DAB0mB commented Apr 20, 2016

The afterFlush() is there so the promise would be fulfilled once the current computation is finished, otherwise subscriptions made in the promise would be stopped. There is no need to call afterFlush(), just use the API as is.

@mxab
Copy link
Author

mxab commented Apr 20, 2016

Hmm I swtiched everything to await, but sill I have this random not finishing issue.

This is the peace of code that currently is the problem
In my $stateProvider.state({..}) definition:

 resolve: {
  'currentUser': function ($auth, $log, $timeout) {
    const awaitUser = $auth.awaitUser();
    $log.debug("awaitUser in AUTH");
    awaitUser.finally(()=> {
      $log.debug('DONE AWAITING USER');
    });
    return awaitUser;
   }
}

Quite often but so far random I see
awaitUser in AUTH
but not
DONE AWAITING USER

which does not make any sense to me

@DAB0mB
Copy link
Collaborator

DAB0mB commented Apr 21, 2016

Hmm I can't seem to reproduce your problem, everything works fine for me. Make sure you actually login before invoking awaitUser.

@dohomi
Copy link

dohomi commented Apr 22, 2016

@DAB0mB I'm working on the same project with @mxab

The problem occurs explicitly inside of the resolve of a $stateProvider.state() setup of ui-router

Following code does not work and accidentally fails for no reason - the .finally()of awaitUser will never getting called:

$stateProvider.state('auth', {
            url: '',
            abstract: true,
            resolve: {
                'currentUser': function($auth) {
                    return $auth.awaitUser();
                }
            }
        })

I replaced now the $auth.awaitUser() with standard Meteor code and it works. I am pretty sure that the reason belongs in the context of the resolve handler. Inside of a controller the code works without any issues.

        $stateProvider.state('auth', {
            url: '',
            abstract: true,
            resolve: {
                'currentUser': function($q) {
                    var defer = $q.defer();
                    Meteor.autorun(c => {
                        let loggingIn = Meteor.loggingIn();
                        if (!loggingIn) {
                            if (Meteor.userId()) {
                                defer.resolve();
                            } else {
                                defer.reject();
                            }
                            c.stop();
                        }
                    });
                    return defer.promise;
                }
            }
        })

I suspect the this.autorunwhich seems to work fine inside of any controller context - but fails if there is no controller context. Hope this helps to get closer to solve this issue

@dohomi
Copy link

dohomi commented Apr 22, 2016

I made a very small change inside of angular-meteor-auth.js in line 118:
I changed:

        var computation = this.autorun(function (computation) {

to

        var computation = Meteor.autorun(function (computation) {

And now the resolve handler works as expected. I'm getting the feeling that this.autorun, this.subscribe is only working in controller context and not inside of any other context.

@dohomi
Copy link

dohomi commented Apr 22, 2016

One important thing how to actually trigger this error, because most of the time on page reloads it works:
Open the link with the resolve handler with CMD+clickand open multiple times the link in new browser window. Then the resolve handler starting to stuck

@DAB0mB
Copy link
Collaborator

DAB0mB commented Apr 22, 2016

@dohomi you mentioned something about subscriptions. We actually had a similar issue which was already fixed, #8. Sadly I can't reproduce this problem so I wish I could help you but I can't, I used awaitUser in many of my projects and I never encountered any problems using it (unless you use older version of angular-meteor or angular-meteor-auth). If you can provide a repo which reproduces the problem I would be glad to help you. If you manage to find the source of the problem and a solution to it please share it with us so we can all enjoy it.

@DAB0mB
Copy link
Collaborator

DAB0mB commented Apr 22, 2016

Maybe there is some sort of a synchronous error thrown in the resolve handler and angular silently ignores it.

@dohomi
Copy link

dohomi commented Apr 22, 2016

I opened this issue where I had to switch back to Meteor.subscribe:
Urigo/angular-meteor#1338

@dohomi
Copy link

dohomi commented Apr 22, 2016

Is there anything against changing this.autorun to Tracker.autorun? For us it would solve the issue for now.

@DAB0mB
Copy link
Collaborator

DAB0mB commented Apr 22, 2016

@dohomi @mxab I'm having troubles reproducing the issue. If we can may a call or something it would be much easier and we can go through it together. Feel free to contact me on the email address presented in my Github account and I would be more than happy to listen and try to help you :-)

edit:
@dohomi for your recent comment there shouldn't be a problem, the main advantage of using $scope.subscribe() is that it will be stopped once the $scope has been destroyed.

@mxab
Copy link
Author

mxab commented Apr 22, 2016

Sounds good, we are currently trying to isolate the issue without luck, I want to make some more tests. I first got the feeling that the defer.resolve(user) somehow does not trigger the then but currently it looks more like that the autorun is not that reactive on the logginIn() call as it should.

@mxab
Copy link
Author

mxab commented Apr 23, 2016

Hi @DAB0mB I did some more debugging. I still don't know why this is only happening in our project but what I found out that something stops the autorun that does checks for Meteor.loggingIn()

This is my own implementation in my ui-router resolve that I think does pretty much the same thing as the awaitUser

var deferred = $q.defer();

var computation = Tracker.autorun(function (c) {

    var loggingIn = Meteor.loggingIn();
    console.log('autorunning...');
    if (!loggingIn) {
        console.log('loggingIn done!');

        Tracker.afterFlush(function () {
            var user = Meteor.user();

            if (user) {

                deferred.resolve(user);
                console.info('resolving user');
            } else {

                deferred.reject('AUTH_REQUIRED');
                console.error('REJECTING');
            }
        });
        console.log('stopping');
        c.stop();
    } else {
        console.log('loggingIn...');
    }
});
computation.onInvalidate(function () {
    console.trace('onInvalidate');
});
computation.onStop(function () {
    console.trace('onStop');
});


deferred.promise.finally(function () {
    console.log("=================DONE=================");
});

return deferred.promise;

Everytime I reload the page the onStop functions gets called but not because of the loggingIn is false but if I look at the stack trace from something outside.

onStop(anonymous function) @ globalStates.js:69 // my onStop handler
(anonymous function) @ tracker.js:307
Tracker.nonreactive @ tracker.js:589
Tracker.Computation.stop @ tracker.js:306
(anonymous function) @ tracker.js:567
(anonymous function) @ tracker.js:284
Tracker.nonreactive @ tracker.js:589
Tracker.Computation.invalidate @ tracker.js:283
Tracker.Dependency.changed @ tracker.js:417
$$Reactive.$$changed @ angular-meteor.js:2439
boundMixin.(anonymous function) @ angular-meteor.js:1796
(anonymous function) @ angular-meteor.js:2287
Scope.$digest @ modules.js?hash=a85fbee…:54246
$$Core.$$throttledDigest @ angular-meteor.js:1991
boundMixin.(anonymous function) @ angular-meteor.js:1796
(anonymous function) @ angular-meteor.js:1665
Tracker.Computation._compute @ tracker.js:323
Tracker.Computation._recompute @ tracker.js:342
Tracker._runFlush @ tracker.js:481
onGlobalMessage @ setimmediate.js:102

As the autorun is stopped the deferred is never resolve nor rejected, which seems to be our issue.

If I surround my whole implementation with another Tracker.afterFlush it seems to work

var deferred = $q.defer();
Tracker.afterFlush(function () {
 var computation = Tracker.autorun(function (c) {
...

@mxab
Copy link
Author

mxab commented May 3, 2016

Just some more notes on the issue, it seems that during the setup of the autorun there is an active computation Tracker.active seems to be true, which as as far as I get it stops the autorun on invalidation

//Tracker.autorun
...
  if (Tracker.active)                                                                                                 // 565
    Tracker.onInvalidate(function () {                                                                                // 566
      c.stop();                                                                                                       // 567
    });   

which then leads the accounts package when it does the _setLoggingIn(false) to activate the invalidate the dependencies and stops the autorun that fulfills/rejects the promise
Unfortunately it seems that the Tracker.active is always true, but the stopping only happens sometimes/randomly

@mxab
Copy link
Author

mxab commented May 3, 2016

It maybe related to the same issue that is happening here
meteor/react-packages#99 (comment)

@DAB0mB
Copy link
Collaborator

DAB0mB commented May 4, 2016

@mxab I'm not sure which package is causing this problem, but look at the test scenario I wrote:

it('should keep waiting for user once an outer computation has stopped', function(done) {
  login();

  function login() {
    Accounts.login('tempUser').onStart(function() {
      awaitUser();
    }).onEnd(function() {
      scope.$$afterFlush('$$throttledDigest');
    })
  }

  function awaitUser() {
    var c = Tracker.autorun(function(c) {
      Tracker.onInvalidate(function() {
        c.stop();
      });

      scope.$awaitUser().then(done);
    });

    c.invalidate();
  }
});

It seems like this is the case, am I right? So it means it's not an issue related only to angular-meteor-auth but to any peace of code which is using Scope#autorun().

I hope I understood you correctly, and if so I will solve it by making this test pass and then you can test your app with my fix to see if it really works.

@dohomi
Copy link

dohomi commented May 5, 2016

Hi @DAB0mB
as we found out in our research that the react package has a similar problem we think its internally a bug of Tracker.autorunin edge cases. We solve it now with wrapping $auth.awaitUser into a small service setting a timeout:

 const deferred = this.$q.defer();
 setTimeout(()=> {
            this.$auth.awaitUser().then(deferred.resolve,deferred.reject);
 });
 return deferred.promise;

This works now as expected and the promise resolves correctly and the autorunseems to be reliable.

@mxab
Copy link
Author

mxab commented May 5, 2016

@DAB0mB not sure I fully understand what you test does

So the problem as far as I see it is that the $auth.awaitUser is called but there is an active computation and the loggingIn autorun is treated as a nested autorun and is therefore stopped

if(Tracker.active){
 console.error('active computation',Tracker.currentComputation._func);
}

points to a function in the rootScope service, but not sure if this is really the issue

@DAB0mB
Copy link
Collaborator

DAB0mB commented May 6, 2016

@mxab @dohomi I'm working on a proper fix and it should be ready today I believe

DAB0mB added a commit to DAB0mB/angular-meteor-auth that referenced this issue May 6, 2016
@DAB0mB
Copy link
Collaborator

DAB0mB commented May 6, 2016

@mxab @dohomi can you test your app against my changes? Note that I changed both angular-meteor and angular-meteor-auth

@mxab
Copy link
Author

mxab commented May 7, 2016

@DAB0mB ok will do, which commit/branch should i use for the angular-meteor package?

@mxab
Copy link
Author

mxab commented May 8, 2016

ok tested it with

"angular-meteor": "https://github.com/DAB0mB/angular-meteor.git#8a4c3dc93b940fb147a00470b33ad79cf1380be1",
    "angular-meteor-auth": "https://github.com/DAB0mB/angular-meteor-auth.git#5cb1bcc2030790710063a2cb6ab6833feb73c6f5",

Unfortunately it sill hangs, i put some log outputs in the package to the auths,

if(Tracker.active){
  console.warn("TRACKER ACTIVE", Tracker.currentComputation)
  debugger;
}
var computation = this.autorun(function (computation) {
...

During my resolve call in the ui router there seems to be an active computation

@DAB0mB
Copy link
Collaborator

DAB0mB commented May 8, 2016

@mxab OK they don't seem like the correct branches, try it with https://github.com/DAB0mB/angular-meteor-auth/tree/fix/%2322 and https://github.com/DAB0mB/angular-meteor/tree/feat/auto-promise. In my new implementation I wrapped it with settimeout() as @dohomi suggested, so it should work. Sorry for the late response.

@mxab
Copy link
Author

mxab commented May 9, 2016

Tried this one as well, no difference, I looked a bit more at my stacktrace. The active computation comes from another awaitUser that is happening in a angular run handler. which is really strange. I also tried to isolate this example (having a run that awaits the user and a resolve in the ui router that awaits the user) but with no luck.

@mxab
Copy link
Author

mxab commented Jun 5, 2016

I just noticed that this problem is not only affecting the awaitUser. We experiencing several this.subscribe(...) calls are getting stopped before they finish and they do this always if there is a Tracker.active is already true. The problem is that it happens randomly and in the constructor of Controller.

class MyStateController{
 constructor(){
  if(Tracker.active){ // appears randomly
    //if active the subscription will be stopped
  }
  this.subscribe('foo', ()=>[], ()=>{
     //this will only be called if Tracker.active was false
  })
}
}

@otporsad
Copy link

Hi everyone, what is the status of this bug? I belive i have the same problem but as i see fix is waiting for some time now?

@mxab
Copy link
Author

mxab commented Jun 22, 2016

@otporsad it is kind of good to hear that someone else has this issue as well. I found out that this issue is actually not only affecting the auth package but more or less all initial subscriptions as well (randomly).

One thing how I finally mitigated this problem is to remove all direct calls for $auth.awaitUsers in angular.run(...) functions and put them inside a $timeout.

Could you check if this might help in your case as well?

@otporsad
Copy link

My problem is with resolve in router.
For example:
resolve: { currentUser: ['$auth', function($auth) { return $auth.awaitUser(); }],

Now the app hangs while loading that route. If i comment out return $auth.awaitUser(); route loads as expected.

I have also one more resolve item that calls a service that has tracker autorun and it hangs to. I dont know if that is related.

If i put $auth.awaitUser in $timeout like this:

resolve: { currentUser: ['$auth', function($auth, $timeout) { return $timeout(() => { return $auth.awaitUser(); }, 0); }],

but still problem is not solved. I have been keeping an eye on this issue for days hoping someone smarter will figure something out but no luck :(

But worst thing is that sometimes it reloads and everything works as expected. In my experience this happends when browser window is open and not touched. Then following a file edit meteor reloads the page and route sometimes works as expected, but mostly not. If there is anything i can help to test or solve this please let me know.

@mxab
Copy link
Author

mxab commented Jun 22, 2016

Could you check the stacktrace in the resolve call to see if and which computation may already be running?

resolve: { 
  currentUser: ['$auth', function($auth) {
    if(Tracker.active){ // appears randomly
      console.trace('ALREADY in computation');
    }
    return $auth.awaitUser(); 
  }
}]}

Also do you use the tmeasday:publish-counts or the meteorhacks:unblock package by any chance? I cannot really say if they had been related to the issue but it kind of got better after I removed them from my project (but it did not fix it totally)?

@otporsad
Copy link

otporsad commented Jun 22, 2016

Sorry for formatting, i dont use those packages.

Trace:
How do i check coputation?
I see these entries:
Tracker.Computation._compute @ tracker.js:323
Tracker.Computation @ tracker.js:211
Tracker.autorun @ tracker.js:588

but dont understand much...

$stateProvider.state.resolve.currentUser @ status.routes.js:15
invoke @ modules.js?hash=371aed9…:29137
proceed @ angular-ui-router.js:475
invoke @ angular-ui-router.js:471
(anonymous function) @ angular-ui-router.js:450
resolve @ angular-ui-router.js:554
resolveState @ angular-ui-router.js:3583
transitionTo @ angular-ui-router.js:3292
(anonymous function) @ angular-ui-router.js:2384
invoke @ modules.js?hash=371aed9…:29137
handleIfMatch @ angular-ui-router.js:1868
(anonymous function) @ angular-ui-router.js:1925
check @ angular-ui-router.js:2041
update @ angular-ui-router.js:2050
$broadcast @ modules.js?hash=371aed9…:42195
afterLocationChange @ modules.js?hash=371aed9…:37980
(anonymous function) @ modules.js?hash=371aed9…:37966
$eval @ modules.js?hash=371aed9…:41872
$digest @ modules.js?hash=371aed9…:41685
$$Core.$$throttledDigest @ angular-meteor.js:2010
boundMixin.(anonymous function) @ angular-meteor.js:1796
(anonymous function) @ angular-meteor.js:1665
Tracker.Computation._compute @ tracker.js:323
Tracker.Computation @ tracker.js:211
Tracker.autorun @ tracker.js:588
$$Core.autorun @ angular-meteor.js:1900
boundMixin.(anonymous function) @ angular-meteor.js:1796
$$Auth @ angular-meteor-auth.js:90
(anonymous function) @ angular-meteor.js:1756
_construct @ angular-meteor.js:1755
(anonymous function) @ angular-meteor.js:1738
mixin @ angular-meteor.js:1737
(anonymous function) @ angular-meteor-auth.js:204
invoke @ modules.js?hash=371aed9…:29137
(anonymous function) @ modules.js?hash=371aed9…:28945
forEach @ modules.js?hash=371aed9…:24749
createInjector @ modules.js?hash=371aed9…:28945
doBootstrap @ modules.js?hash=371aed9…:26179
bootstrap @ modules.js?hash=371aed9…:26200
onReady @ app.js:57
fire @ jquery.js:3187
fireWith @ jquery.js:3317
ready @ jquery.js:3536
completed @ jquery.js:3552

@mxab
Copy link
Author

mxab commented Jun 22, 2016

Ok unfortunately there is nothing I could spot directly, but it looks like there is already await computation running.

Tracker.Computation._compute @ tracker.js:323 <---------------------------
Tracker.Computation @ tracker.js:211<---------------------------
Tracker.autorun @ tracker.js:588<---------------------------
$$Core.autorun @ angular-meteor.js:1900<---------------------------
boundMixin.(anonymous function) @ angular-meteor.js:1796<---------------------------
$$Auth @ angular-meteor-auth.js:90<---------------------------
(anonymous function) @ angular-meteor.js:1756
_construct @ angular-meteor.js:1755
(anonymous function) @ angular-meteor.js:1738
mixin @ angular-meteor.js:1737
(anonymous function) @ angular-meteor-auth.js:204
invoke @ modules.js?hash=371aed9…:29137
(anonymous function) @ modules.js?hash=371aed9…:28945
forEach @ modules.js?hash=371aed9…:24749
createInjector @ modules.js?hash=371aed9…:28945
doBootstrap @ modules.js?hash=371aed9…:26179
bootstrap @ modules.js?hash=371aed9…:26200
onReady @ app.js:57
fire @ jquery.js:3187
fireWith @ jquery.js:3317
ready @ jquery.js:3536
completed @ jquery.js:3552

Just to be sure, that is really not resolving, could you extend the resolve:

resolve: { 
  currentUser: ['$auth', function($auth) {
    var isActive = Tracker.active;
    if(isActive){ // appears randomly
      console.trace('ALREADY in computation');
    }
    return $auth.awaitUser().finally(function(){ 
         console.log("WAS COMPUTATION ACTIVE?", isActive)
    }); 
  }
}]}

The problem should be there if you don't see the `WAS COMPUTATION ACTIVE?``

And one more thing could you show how you start the app (onReady @ app.js:57)

@otporsad
Copy link

I start my app with:
onReady = function () { angular.bootstrap(document, ['meteorDentalApp']); };

I do not see WAS COMPUTATION ACTIVE? only: ALREADY in computation.

@mxab
Copy link
Author

mxab commented Jun 22, 2016

can you show me what is at angular-meteor-auth.js:90 and angular-meteor.js:1756 and arround

@otporsad
Copy link

angular-meteor-auth.js:90

function $$Auth() {
        var vm = arguments.length <= 0 || arguments[0] === undefined ? this : arguments[0];

        // Reset auth properties
     /* 90 */   this.autorun(function () {
          // Note that we use Meteor and not Accounts since the following methods are
          // not available in older versions of `accounts-base` meteor package
          vm.currentUser = Meteor.user();
          vm.currentUserId = Meteor.userId();
          vm.isLoggingIn = Meteor.loggingIn();
        });
      }

angular-meteor.js:1756

      // Invoke function mixins with the provided context and arguments
      this._construct = function (context) {
        for (var _len = arguments.length, args = Array(_len > 1 ? _len - 1 : 0), _key = 1; _key < _len; _key++) {
          args[_key - 1] = arguments[_key];
        }

        _this._mixins.filter(_underscore2.default.isFunction).forEach(function (mixin) {
    /* 1756 */      mixin.call.apply(mixin, [context].concat(args));
        });

        return context;
      };

@mxab
Copy link
Author

mxab commented Jun 22, 2016

hmm @DAB0mB @Urigo I'm not sure but would it make sense to put the initial autorun in the $$Auth constructor in an timeout?

@anymos
Copy link

anymos commented Sep 28, 2016

Good morning all, joining the group with the exact same issue. (Tracker.active) which make the computation seen as nested and then failing lamentably.

I am not using meteor-auth, but a quite similar implementation than @mxab

I am going to investiguate more to find out the root cause of this ...

@mxab
Copy link
Author

mxab commented Sep 28, 2016

@anymos You can try to check if it works for you if you start angular after the initial flush

Tracker.afterFlush(function(){
 angular.bootstrap(...)
})

@anymos
Copy link

anymos commented Sep 29, 2016

@mxab, that s interesting, I was thinking that using an afterFluch as you was suggesting to startAngular would have worked.

I still have the tracekrActive even so

I will continue to try to find out, it must be somewhere in the code ;-)

@mxab
Copy link
Author

mxab commented Sep 29, 2016

ok, this was just an @otporsad I discussed, for me it kind of works to make sure that any potential autorun inside a angular run is scheduled after a $timeout

@anymos
Copy link

anymos commented Sep 29, 2016

I agree that there something linked to the $timeout, this is actaully waht I use at the moment to overcome the issue.

But I am not really sure what is the root cause, I have another application that is really similar to the one I have trouble with.

I am still investigating comparing the code line by line to try to find out the root cause of it.

it s a matter or time, but I will find it

@anymos
Copy link

anymos commented Sep 29, 2016

ok progressing it s coming, in my case from a node_modules.

Trying to find out which one now

@anymos
Copy link

anymos commented Sep 29, 2016

I found the offensive package:

I have changed : "angular-ui-router": "^0.3.1", to "angular-ui-router": "^0.2.18" and the problem disappear.

Absolutely no idea why it is like that. but ti solved it.

such a pain to nail it down.

I believe the next step would to find out what is the difference between those version to get the root cause of the problem

@mxab, could you kindly check on your side if you have the same version and if it s solving the issue ?

@mxab
Copy link
Author

mxab commented Sep 29, 2016

0.3.1, I can try to check if it works, but as far as I remember the issue was quite random, and I need to revert all the runs

@anymos
Copy link

anymos commented Sep 29, 2016

"angular-ui-router": "^0.3.0" is having the same problem

@anymos
Copy link

anymos commented Sep 29, 2016

for me its a 100% coming up at each time, it s why I am taking the time to find out where it s coming from, as from what I read a lot of time it s random and thus impossible to find out

@anymos
Copy link

anymos commented Sep 29, 2016

unfortunately not that much time left to investigate until the end to find out the real root cause.

I fail to understand why and how router ui can have a side effect on the meteor computation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants