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

Meteor.userId() throws an error in validateLoginAttempt/onLogin/onLoginFailure callbacks #4862

Closed
brettle opened this issue Aug 2, 2015 · 4 comments

Comments

@brettle
Copy link
Contributor

brettle commented Aug 2, 2015

Meteor.userId() is throwing an error when called within Accounts.validateLoginAttempt/Accounts.onLogin/Accounts.onLoginFailure callbacks. This is preventing me from determining which (if any) already logged in user made the login attempt.

Here's a reproduction recipe using this github repo:

[brettle@localhost git]$ git clone git@github.com:brettle/Meteor.userId-throws-error-in-validateLoginAttempt-callback.git login-callback-bug
Cloning into 'login-callback-bug'...
remote: Counting objects: 13, done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 13 (delta 0), reused 13 (delta 0), pack-reused 0
Receiving objects: 100% (13/13), done.
Checking connectivity... done.
[brettle@localhost git]$ cd login-callback-bug/
[brettle@localhost login-callback-bug]$ meteor
[[[[[ ~/git/login-callback-bug ]]]]]          

=> Started proxy.                             
=> Started MongoDB.                           
=> Started your app.                          

=> App running at: http://localhost:3000/

Visit http://localhost:3000/ and click the "Login with correct password" button. You'll get this on the server console:

I20150801-21:56:11.520(-7)? In validateLoginAttempt callback:
I20150801-21:56:11.521(-7)?   Meteor.userId() throws: Error: Meteor.userId can only be invoked in method calls. Use this.userId in publish functions.
I20150801-21:56:11.521(-7)?   this.userId: undefined
I20150801-21:56:11.522(-7)? In onLogin callback:
I20150801-21:56:11.522(-7)?   Meteor.userId() throws: Error: Meteor.userId can only be invoked in method calls. Use this.userId in publish functions.
I20150801-21:56:11.522(-7)?   this.userId: undefined

Click the "Login with incorrect password" button and you'll get this on the server console:

I20150801-21:56:15.091(-7)? In validateLoginAttempt callback:
I20150801-21:56:15.091(-7)?   Meteor.userId() throws: Error: Meteor.userId can only be invoked in method calls. Use this.userId in publish functions.
I20150801-21:56:15.091(-7)?   this.userId: undefined
I20150801-21:56:15.091(-7)? In onLoginFailure callback:
I20150801-21:56:15.091(-7)?   Meteor.userId() throws: Error: Meteor.userId can only be invoked in method calls. Use this.userId in publish functions.
I20150801-21:56:15.091(-7)?   this.userId: undefined

I suspect this is related to these API functions using Hooks which binds the callbacks "with Meteor.bindEnvironment, so they will be called with the Meteor environment of the calling code that registered the callback."

brettle added a commit to brettle/meteor that referenced this issue Aug 3, 2015
Changes to packages/callback-hook:
- Add bindEnvironment option to Hook constructor (defaults to true).
- Add internal helper function dontBindEnvironment() which does all the
  error handling stuff like Meteor.bindEnvironment() but doesn't change
  the environment.
- Use dontBindEnvironment() instead of Meteor.bindEnvironment() when
  bindEnvironment option is false.
- Add tests.

Changes to packages/accounts-base:
- Create hooks with { bindEnvironment: false }
- Add test for whether Meteor.userId() is available in login callbacks.
@brettle
Copy link
Contributor Author

brettle commented Aug 3, 2015

It is, in fact, the Meteor.bindEnvironment() wrapping that is causing the problem. I've just created PR #4867 which fixes it. I don't see a good reason for using bindEnvironment here. Maybe @awwx just used because it was part of the Hook code that he extracted from livedata when he implemented login hooks 18 months ago?

stubailo pushed a commit that referenced this issue Aug 3, 2015
Changes to packages/callback-hook:
- Add bindEnvironment option to Hook constructor (defaults to true).
- Add internal helper function dontBindEnvironment() which does all the
  error handling stuff like Meteor.bindEnvironment() but doesn't change
  the environment.
- Use dontBindEnvironment() instead of Meteor.bindEnvironment() when
  bindEnvironment option is false.
- Add tests.

Changes to packages/accounts-base:
- Create hooks with { bindEnvironment: false }
- Add test for whether Meteor.userId() is available in login callbacks.
@stubailo stubailo closed this as completed Aug 3, 2015
@stubailo
Copy link
Contributor

stubailo commented Aug 4, 2015

Thanks for the PR!

@brettle
Copy link
Contributor Author

brettle commented Aug 5, 2015

Thanks for the fast review and merge!

On Tue, Aug 4, 2015 at 1:57 PM, Sashko Stubailo notifications@github.com
wrote:

Thanks for the PR!


Reply to this email directly or view it on GitHub
#4862 (comment).

brettle added a commit to brettle/meteor-workaround-issue-4862 that referenced this issue Aug 18, 2015
@brettle
Copy link
Contributor Author

brettle commented Aug 18, 2015

The brettle:workaround-issue-4862 package provides a workaround for Meteor releases where this is not fixed.

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

2 participants