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

[1.8.3] onLogin handler fired twice #10853

Closed
AndyCC25 opened this issue Jan 9, 2020 · 22 comments · Fixed by #11785
Closed

[1.8.3] onLogin handler fired twice #10853

AndyCC25 opened this issue Jan 9, 2020 · 22 comments · Fixed by #11785
Assignees
Labels
Milestone

Comments

@AndyCC25
Copy link

AndyCC25 commented Jan 9, 2020

Updating the version from 1.8.1 to 1.8.3, notice that a part of the code that relies in a callback to Accounts.onLogin() method from it’s being called twice. reference to https://forums.meteor.com/t/onlogin-handler-fired-twice-meteor-1-8-3/51224.

  • Version: Meteor: 1.8.3
  • Last version without the issue: Meteor 1.8.1
  • OS: Ubuntu 14.04
  • Expected Behavior: onLogin callback should run once
  • Actual Behavior: onLogin callback runs twice
  • Reproduction: run the reproduction project and notice that the onLogin callback fire twice, can check by a simple console.log(). Project: https://github.com/AndyCC25/meteor-login-bug
@benjamn benjamn self-assigned this Jan 9, 2020
@benjamn benjamn added this to the Package Patches milestone Jan 9, 2020
@stale
Copy link

stale bot commented Feb 10, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Feb 10, 2020
@afrokick
Copy link
Contributor

not stale

@stale stale bot removed the stale-bot label Feb 10, 2020
@stale
Copy link

stale bot commented Mar 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Mar 13, 2020
@stale
Copy link

stale bot commented Apr 15, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Apr 15, 2020
@brendonlamb
Copy link

Still not state in Meteor 1.10.2 - just upgraded, same issue.

@stale stale bot removed the stale-bot label Apr 22, 2020
@stale
Copy link

stale bot commented May 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label May 24, 2020
@AndyCC25
Copy link
Author

Also I notice that still happens the two fire

@stale stale bot removed the stale-bot label May 27, 2020
@AndyCC25
Copy link
Author

a WA could use a once() function from underscore

@santiagopuentep
Copy link

Same issue in Meteor 1.11.

@namenotrequired
Copy link
Contributor

It seems like there's a few separate bugs going on here.

On initial login

A seemingly old bug in accounts-ui-unstyled/login_buttons_dropdown.js: if I click on the login button, I login once. But if I submit the form by hitting enter, I trigger two separate event listeners that both try to log me in, and we end up calling onLogin twice.

I made a PR to fix it (and a related bug): #11173

On load when already logged in

On refresh, it is called once by loggedInAndDataReadyCallback, as normal, and once by _startupCallback which was introduced in this PR that was included in Meteor 1.8.2

I assume this is the bug that didn't happen on 1.8.1. I haven't looked into it further yet.

@stale
Copy link

stale bot commented Oct 31, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Oct 31, 2020
@santiagopuentep
Copy link

Still happening on Meteor 1.11.

@stale stale bot removed the stale-bot label Nov 1, 2020
@filipenevola filipenevola added the confirmed We want to fix or implement it label Nov 11, 2020
@filipenevola
Copy link
Collaborator

This should be fixed on Meteor 1.12

@jasongrishkoff
Copy link

jasongrishkoff commented Dec 8, 2020

Was optimistic v1.12 would solve my double login issue, but I guess not.

Accounts.onLogin still fires twice for me when I refresh the page (if previous logged in). If I load the page logged out, then log in, onLogin only fires once. Specifically, this happens when onLogin passes {type: "resume"}.

I wonder if it has something to do with this part of accounts-base:

  // https://github.com/meteor/meteor/blob/devel/packages/accounts-base/accounts_client.js
  // _startupCallback executes on onLogin callbacks
  // at registration time if already logged in
  // this can happen when new AccountsClient is created
  // before callbacks are registered see #10157
  _startupCallback(callback) {
    // Are we already logged in?
    if (this.connection._userId) {
      // If already logged in before handler is registered, it's safe to
      // assume type is a 'resume', so we execute the callback at the end
      // of the queue so that Meteor.startup can complete before any
      // embedded onLogin callbacks would execute.
      Meteor.setTimeout(() => callback({ type: 'resume' }), 0);
    }
  }

I can tell that the startup finishes before Accounts.onLogin fires, so not entirely sure how that works.

@filipenevola
Copy link
Collaborator

@namenotrequired maybe @jasongrishkoff's case is a different one?

@jamesloper
Copy link

jamesloper commented Feb 23, 2021

Same behavior here as @jasongriskoff - When I log in with my React form, it fires once. When using a resume token it's firing twice in rapid succession. It seems like a race condition to me. The time between the two logins is median 70 to 80 milliseconds but as high as 400 millis. I also tried removing all onLogin callbacks and just putting in a single one as a test.

Would it be possible to reopen this issue?

@jamesloper
Copy link

jamesloper commented Feb 25, 2021

This bug might be simpler to find than we thought. It looks like the first time it fires, Meteor.user() is undefined and the second time, it is the actual user document. Meteor.userId() is set both times. So it seems the meteor internals are emitting before and after the default null sub is ready.

@santiagopuentep
Copy link

@jamesloper this is exactly the problem I'm having, it fires twice, but the first time Meteor.user() is not available; the second time it its.

@StorytellerCZ
Copy link
Collaborator

This issue persists as per #11446, which was closed as a duplicate of this one.

@brucejo75
Copy link
Contributor

@jasongrishkoff & @namenotrequired you were right on target.
PR #11785 should address it.

Sorry I just noticed this issue. To speed things up next time:

  • Referencing a PR or other issue does not notify the author
  • In a case like this it may have been addressed quicker if you had mentioned the author of the offending PR (e.g. @brucejo75)

Sorry I goofed it & sorry I did not see this earlier! Simple fix.

@paulincai
Copy link
Contributor

@filipenevola this is still going on in 2.5.3

@filipenevola filipenevola added this to the Release 2.5.5 milestone Jan 18, 2022
@filipenevola
Copy link
Collaborator

Should be fixed in Meteor 2.5.5 coming out soon.

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

Successfully merging a pull request may close this issue.