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

Cannot read property 'user' of undefined #4

Closed
semateos opened this issue Dec 13, 2018 · 10 comments
Closed

Cannot read property 'user' of undefined #4

semateos opened this issue Dec 13, 2018 · 10 comments

Comments

@semateos
Copy link
Owner

    at Object.run (methods.js:35)
    at MethodInvocation.newMethodOptions.run (wireMethods.js:139)
    at ValidatedActionMethod._execute (validated-method.js:94)
    at MethodInvocation.setTaskDone (validated-method.js:57)
    at DDP._CurrentMethodInvocation.withValue (livedata_connection.js:664)
    at Meteor.EnvironmentVariable.EVp.withValue (meteor.js?hash=33066830ab46d87e2b249d2780805545e40ce9ba:1196)
    at Connection.apply (livedata_connection.js:653)
    at ValidatedActionMethod.call (validated-method.js:70)
    at Promise (callpromise-mixin.js:4)
    at new Promise (<anonymous>)
    at ValidatedActionMethod.callPromise (callpromise-mixin.js:3)
    at ValidatedActionMethod.js:24```
@semateos
Copy link
Owner Author

Looks like it's missing the user from auth object here:

const userId = auth.user._id;

@wswoodruff
Copy link
Contributor

Is the title of this the error you're getting? If so, then we're missing the auth prop. It gets set on this line, and only gets set if Meteor.isServer.

Could you poke around in here and see if the line is even getting run? Did they remove Meteor.isServer or something I wonder?

argsClone.auth = { user: foundUser };

@joerou
Copy link
Contributor

joerou commented Dec 17, 2018

This is because the user is only added to the auth object if meteor.isServer. The problem with that is that validated methods run on both the client and the server (for optimistic ui). So it fails on the client because user doesn't exist, then works fine on the server and updates the ui to the correct state. Which is why the function appears to work, but still throws that error to the client console.

My guess is that you checked the login token instead of checking this.userId in the method because apollo instead of using the meteor way. So I would assume the best way to fix this is to add the currently logged in user above this and then only override it with the server variable if its being run on the server.

@wswoodruff
Copy link
Contributor

Hey that sounds about right — thanks for those thoughts!!

I wonder if any extra optimistic UI code was added in the update path we just upgraded to. (I think we went from Meteor 1.6 — 1.8)

@semateos
Copy link
Owner Author

@joerou can you take a shot at fixing that?

It seems to me (although memory is untrustworthy) that the optimistic UI is not working as it was in the previous version. My recollection was that it had better offline behavior. Could be related to this failure on the client side.

@joerou
Copy link
Contributor

joerou commented Dec 17, 2018

Yep, I'll take care of it.

@joerou
Copy link
Contributor

joerou commented Dec 17, 2018

I made a fix for this, see the latest PR.

I just used the Meteor default for loggedin user instead of checking the hash token. Then I split out the ownership logic so a different error could be pushed to the client if they were loggedin but not the owner.

@semateos
Copy link
Owner Author

@wswoodruff how does that sound? Can you give it a quick review? Thanks!

@joerou
Copy link
Contributor

joerou commented Dec 31, 2018

I was doing some experimenting this week and the solution works, except when calling from the graphiql interface, it seems this.userId is defined, but always returns null. It seems this should be set via the apollo ddp package but apparently something is getting lost. When looking into that I found there is a new way of creating the apollo client and the apollo server in the docs, maybe this is a better route? I started looking into and will keep going this week.

In the meantime I have set the old method to run on the server and set the fix to run on the client, so the auth.user object is always set and that will work for now.

@semateos
Copy link
Owner Author

OK

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

3 participants