Meteor.userId() and Meteor.user() are unusable in deferred callbacks.

[mizzao]

@avital: Upon revisiting, this issue is actually a little bit worse than what we discussed today. It doesn't just null out the userId; attempting to access the user in a callback on the server actually throws an error.

The repro is here: https://github.com/mizzao/user-deferred-callback. Note that you'll need to mrt install . to get the accounts-testing package that logs the user in on the client.

The code that demonstrates the error is this, on the server:

Meteor.methods({
    "print-userId": function() {
        console.log(Meteor.userId());
        console.log(Meteor.user());
        Meteor.defer(function() {
            console.log(Meteor.userId());
            console.log(Meteor.user());
        });
    }
});

You'd expect the same thing to print twice. Instead, what you get is

I20140610-00:21:18.954(-4)? r4FcfxE62x2Qb2sMj
I20140610-00:21:18.956(-4)? { _id: 'r4FcfxE62x2Qb2sMj',
I20140610-00:21:18.956(-4)?   createdAt: Tue Jun 10 2014 00:20:50 GMT-0400 (EDT),
I20140610-00:21:18.956(-4)?   services: { resume: { loginTokens: [Object] } },
I20140610-00:21:18.957(-4)?   username: 'luser' }
I20140610-00:21:18.957(-4)? Exception in defer callback: Error: Meteor.userId can only be invoked in method calls. Use this.userId in publish functions.
I20140610-00:21:18.958(-4)?     at Object.Meteor.userId (packages/accounts-base/accounts_server.js:19)
I20140610-00:21:18.958(-4)?     at app/user-deferred-callback.js:30:22
I20140610-00:21:18.958(-4)?     at _.extend.withValue (packages/meteor/dynamics_nodejs.js:37)
I20140610-00:21:18.959(-4)?     at packages/meteor/timers.js:6
I20140610-00:21:18.959(-4)?     at runWithEnvironment (packages/meteor/dynamics_nodejs.js:89)

The deferred callback actually thinks it's in a publish function, and of course this is not the case. It's because, as we discussed earlier today, in 1a503d1 (2 years ago) where @debergalis added some code to re-bind the Meteor environment without the current method invocation. Because accounts was added after this and Meteor.userId() is read from the method invocation, this also kills the ability to use Meteor.userId(). (The current version of this code is at https://github.com/meteor/meteor/blob/devel/packages/meteor/timers.js.)

@glasser discussed two ways to fix this: (1) either removing the code that unbinds the current method invocation, assuming it doesn't cause any memory leaks; or (2) just storing the userId in a separate environment variable.

At the same time, I'd actually suggest coming up with some way to bind the userId to an environment variable as well, so that it is accessible in publish functions; we use a nasty workaround for this in collection-hooks and it would be great to have some way for this.userId to be accessible to packages that do stuff in publish functions.

I'd be happy to revisit this issue on Friday with anyone at MDG.

[glasser]

It doesn't really think it's in a publish function. It thinks it's not in a method, and we figured when writing the error message that the most likely place you'd call Meteor.userId outside of methods is a publish function.

This is working as intended. The callback really is not running as part of the method! It's running some time later! Now, yes, with fibers and futures, it could be running concurrently with the method, but that requires rather fancy footwork, and anyone who can do that should be able to just call Meteor.userId from the actual method body and save it in a variable.

It doesn't seem right if you're using a method to kick off some administrative task some time later that it should run in the context of the user who might not even be connected any more. If you want to remember the userId in the callback, you should close over it.

[mizzao]

@glasser I'm writing a package that requires the userId to be available in callbacks because it's used to determine other variables (https://github.com/mizzao/meteor-partitioner), and I can't expect to ask the user to wrap the userId and pass it in for every callback. I'm only reporting in this issue what you mentioned yesterday.

I think it's just unexpected behavior to have the userId be mysteriously unbound, but not any other environment variables.

Also, this still doesn't answer the question of why DDP._CurrentInvocation needs to be unbound, especially since its userId field came along after this code was written.