0

i have come across an oddity in what gets returned when i run similar mutations. i am just learning this stack, and i implement mongodb with mongoose as my database layer.

i have two simple mutations. one creates a user and adds it to the database aka register. the other adds a profile pic obviously after the initial user object is created.

both resolvers return the full user object, which is nice to do for updating the ui, store, etc, i am planning on implementing subscriptions so it is important that i get the new data back.

i was working thing out with graphiql and ran into a weird problem. my update resolver was returning null even though the image url was getting saved to the db. the register resolver was returning all the user fields.

upon console logging the return objects in the resolvers, both functions were returning a full user object.

when i tried to make the return object non nullable i would get the error cant return null for non nullable for the update resolver.

my resolver was initially using findOneAndUpdate with a callback that did in fact return the user object. and yes, i was still getting null. weird.

i changed my resolver to instead a more manual approach. i look up the existing user with findOne passing in the user id, then explicitly saying user.profilePic = "url of pic" and the calling save of the whole user object and returning the user object in the callback to that. and boom, that works!

so what is causing this? i get the initial feeling that this has something to do with timing somehow aka the not waiting for the callback....i really dont understand why my first approach doesn't work and my second does. i will attach the code for both maybe someone with a deeper understanding of timing or possibly async functions can chime in. maybe i need to update my style from callbacks to promises or async await.

//this one doesnt work    
addProfilePic: (root, { input }, context) => {
  let update = { profilePic: input.profilePic };
  let query = { id: input.id };
  let options = { new: true, upsert: true};
  let callback = ((err, user) => {
    if(err) console.log(err.message);
    return user;
  })
  return updatedUser = User.findOneAndUpdate(query, update, options, callback)
}                    

//this one works, but returns old user object to client...
//so really, no, it doesn't work
addProfilePic: (root, { input }, context) => {
  return User.findOne({id: input.id}, ((err,user) => {
    if(err)console.log(err);
    if(user){
      user.profilePic = input.profilePic;
      user.save((err) => {
        if(err)console.log(err);
        console.log(user);
        return user;
      })
    }
  })
})

note: passing in context, when i actually implement i will get id from context, which contains the user when they are logged in. note: these are soon cool tools, but a lot to learn, especially for someone with 3 months total coding exp...ie me...

benjaminadk
  • 468
  • 6
  • 18
  • Possible duplicate of [Why does a GraphQL query return null?](https://stackoverflow.com/questions/56319137/why-does-a-graphql-query-return-null) – Daniel Rearden Jun 01 '19 at 15:59

1 Answers1

2

Despite what the docs say, when you include a callback, findOneAndUpdate returns undefined. On the other hand, findOne returns a Query object. The problem here is when you pass a callback, the intent is for you to handle the values passed to your callback as arguments, not deal with whatever the call's return value is.

With GraphQL, a resolver can return a value or a Promise that resolves to that value. The Query object returned by findOne isn't a Promise, but is "thenable" and so in a sense you are "getting away" with doing things this way. However, I suspect if you look at what's actually being returned by GraphQL, you'll find that it's returning the original user object, not the saved one.

As you conjectured there is a better way :)

To get mongoose to return a Promise, you need to:

  1. Drop the callback altogether
  2. Append .exec() to the end of your call

Now your resolver looks like this:

addProfilePic: (root, { input }, context) => {
  let update = { profilePic: input.profilePic };
  let query = { id: input.id };
  let options = { new: true, upsert: true};
  return User.findOneAndUpdate(query, update, options).exec()
}

A couple of additional notes to get you on the right path:

You'll notice I don't do any error handling in the code above. This is because GraphQL will actually catch those errors for you and include them in the response. However, if you wanted to either provide additional error information, or obfuscate the details being returned to the client, you can append catch() to your call, modify your error inside it and then throw it back.

Now that you're returning a promise, you can use then() if you need to work with the query result, just remember to return the value inside!

return User.findOneAndUpdate(query, update, options).exec()
  .then(user => {
    console.log(user)
    // you could modify the object being handed to GraphQL here
    return user // IF you use a then, make sure you return the value!!
  })

Lastly, if you do end up using callbacks, be aware that, unlike with Promises, the return statements inside them don't do anything (at least not in this case). Because they are asynchronous, there's no way you can somehow return the values they are called with back to your origin call (not without wrapping everything inside a Promise and that's a path that's not really worth taking if you can already have a Promise returned).

Daniel Rearden
  • 80,636
  • 11
  • 185
  • 183
  • your are right, i realized i was returning my original findOne user object, even though graphiql was displaying the correct up to date data. i like this pattern for errors as well, it gets tedious the way i was doing it. i have found it tough to find standard patterns for this stack, kind of just winging it. very helpful advice. – benjaminadk Aug 21 '17 at 03:02
  • @benjaminadk [This is a curated list of GraphQL examples](https://github.com/chentsulin/awesome-graphql). There's a couple in there that use mongoose that you may find helpful. – Daniel Rearden Aug 21 '17 at 03:28