Coming back from nested functions

Question

I'm new to node/javascript so my problem is likely trivial, but its causing me trouble no less.

I have the following code. It successfully checks a database for both the Unit and Owner values. The problem is that the code following the if statement else if (Owner){ executes as expected, however, AFTER that the program never gets to the return reply(output); line, which I expect it should.

I think its the way that I am coming back from the Owner.findOne(... code.

Can anyone see what I'm doing wrong?

exports.sale = {
    tags: ['api'],
    validate : {
        //blah blah blah
    },
    handler : function(request, reply) {
        var output = {
            success: true,
            operations: [],
            epoch: Date.now()
        };

        Unit.findById(request.payload.deviceNumber, function(err, device) {
            if (err) {
                //blah blah blah
            }
            if (device) {
                Owner.findOne({OwnerId: device.Owner}, function(err, Owner) {
                    if (err) {
                        //blah blah blah
                    }
                    else if (Owner){
                        //make changes to output.operations

                    }
                });
            } else {
                output.success = false;

            }
            return reply(output);

        });

    }
};

Possible duplicate of [Why is my variable unaltered after I modify it inside of a function? - Asynchronous code reference](http://stackoverflow.com/questions/23667086/why-is-my-variable-unaltered-after-i-modify-it-inside-of-a-function-asynchron) — t.niese, Jul 17 '16 at 12:58

Samuel Toh · Accepted Answer · 2016-07-17T13:17:37.487

There are couple of problems in the code.

First, assuming the if (err) clause does not have the same return reply(output) statement, this means that your code will always do the return(output) no matter what as Unit.findById is asynchronous. That is, the code is not going to wait for a response from Unit.findById to finish and as soon as it calls it, the code will move on and eventually hitting the return statement.

Secondly, Owner.findOne should have its own return statement within the callback function's code block as the code will only travel within it.

Hence for happy path cases where Owner.findOne and Unit.findById executes successfully, you will still receive a response of output.success = false.

So, ignoring code readability, to fix the problem - your code should look something like this.

Unit.findById(request.payload.deviceNumber, function(err, device) {
    if (err) {
        //blah blah blah
        output.success = false;
        return reply(output);
    }
    if (device) {
       Owner.findOne({OwnerId: device.Owner}, function(err, Owner) {
           if (err) {
              //blah blah blah
              output.success = false;
              return reply(output);
           }
           else if (Owner){
               //make changes to output.operations
           }
           output.success = true;
           return reply(output);
       });
   } else {
       output.success = false;
       return reply(output); 
   }

});

t.niese · Answer 2 · 2016-07-17T13:13:52.283

Owner.findOne is another async function, so you need to move reply(output) into the else block where you have output.success = false; and add another reply(output) call within the callback of Owner.findOne.

In your code return reply(output) is called before the async callback of Owner.findOne is executed. In addition you do no need return, because you cannot return a value from those callbacks using return, the return only exit the function there.

exports.sale = {
  tags: ['api'],
  validate: {
    //blah blah blah
  },
  handler: function(request, reply) {
    var output = {
      success: true,
      operations: [],
      epoch: Date.now()
    };

    Unit.findById(request.payload.deviceNumber, function(err, device) {
      if (err) {
        //blah blah blah
      }
      if (device) {
        Owner.findOne({
          OwnerId: device.Owner
        }, function(err, Owner) {
          if (err) {
            //blah blah blah
          } else if (Owner) {
            //make changes to output.operations
          }
          reply(output);
        });
      } else {
        output.success = false;
        reply(output);
      }
    });
  }
};

Coming back from nested functions

2 Answers2