Promise .then chain 'Cannot set property of undifined'

Question

I'm new to promises and can't solve this syntax problem.

In my .then chain I got this part. I got an array of arrays and would like to add a new property to some objects inside the arrays. Before I do this I have to make a request with googleDirection.directionRequest this resolves a new promise. But while adding the attribute to the object the object is undefined (I think because the dbResult is outside of the nested .then chain). I tried different ways but can't solve this. Hope some body can help me with this.

.then(function(dbResult){
    for(var i = 0; i < dbResult.length; i++){
      if(dbResult[i].length == 1){            
        var smallestValue = googleDirection.directionRequest(req.body.startlocation.latitude + "," + req.body.startlocation.longitude, dbResult[i][0].startLatitude + "," + dbResult[i][0].startLongitude,true,false)
        .then(function(distance){
          var values = [];
          for(var j = 0; j < distance.routes.length; j++){
            values.push(distance.routes[j].legs[0].distance.value);
          }
          return Math.min.apply(Math,values);
        })
        .then(function(value){
          return(value);
        })
        smallestValue.then(function(value){
          dbResult[i].tripStartToConstrStart = value;
        })
      }
    }
    return dbResult
  })
.then(function(dbResult){
    console.log(dbResult)       
  })

This is the error:

(node:8708) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot set property 'tripStartToConstrStart' of undefined
(node:8708) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): TypeError: Cannot set property 'tripStartToConstrStart' of undefined

If you want to wait for multiple directionRequests, you need `Promise.all`. — Bergi, Nov 09 '17 at 11:20
You could use `let i` instead of `var i` as well, but the comment from @Bergi is ofcourse correct (the `let i` would just solve the `dbResult[i].tripStartToConstrStart = value` error as `i` would be the current index when defined with `let`) — Icepickle, Nov 09 '17 at 11:22
but the problem is not the request, the problem ist, that I can't add a new property to `dbResult[i]` inside the nested .then. the `dbresult[i]` is undefined. but outside the for look it works correct — WeSt, Nov 09 '17 at 11:25
`dbResult` is an array of objects , `dbResult[i]` is a object, the `if(dbResult[i].length == 1){` it's length may not be right. — JiangangXiong, Nov 09 '17 at 11:25
@WeSt The problem is that you're not waiting for the requests in the loop to add the results before inspecting the results — Bergi, Nov 09 '17 at 11:26
@JiangangXiong it's corrects there are arrays in the dbResult array. sorry it was my mistake at the post above — WeSt, Nov 09 '17 at 11:27
@Bergi I tried it with a console.log and it works, the results are there. I added the error in my post — WeSt, Nov 09 '17 at 11:31
That's because the asynchronous task ,the i always be incremented before the request returned the result. — JiangangXiong, Nov 09 '17 at 11:33
@WeSt That error is because [`i` is having the wrong value in the callback closures](https://stackoverflow.com/q/750486/1048572), as Icepickle correctly observed — Bergi, Nov 09 '17 at 11:36

JLRishe · Accepted Answer · 2017-11-09T11:47:11.123

The reason for your error is known as "closing over the loop variable" and it means that when the asynchronous operations complete and try to execute

dbResult[i].tripStartToConstrStart = value;

i has the same value as dbResult.length (since the for loop has completed by then), and dbResult[dbResult.length] is undefined.

As the commenters have already said, you could solve this by using let, but I think it would be easier to use a map that returns all the promises you need to await (since not awaiting the promises is another big issue in your code):

.then(function(dbResult) {
    var startLocation = req.body.startLocation; 

    var promises = dbResult.map(function(item) {
        if (item.length !== 1) {
            return item;
        }

        return googleDirection.directionRequest(
            startlocation.latitude + "," + startlocation.longitude, 
            item[0].startLatitude + "," + item[0].startLongitude, 
            true, 
            false
        ).then(function(distance) {
            var distances = distance.routes.map(function (route) {
                return route.legs[0].distance.value;
            });

            item.tripStartToConstrStart = Math.min.apply(Math, distances);

            return item;
        });
    });

    return Promise.all(promises);
})
.then(function(dbResult) {
    console.log(dbResult)
})

this works like a charm. Thanks a lot. I like your code style, do you have any good sources for JS/nodeJS design patterns like yours? — WeSt, Nov 09 '17 at 11:53
@WeSt It's just stuff I've picked up along the way, but some rules of thumb are: (1) to execute an async operation for each item in an array, use `Promise.all(arr.map(....))` (2) to convert each value in an array to another value and create an array of the new values, use `arr.map(...)` (3) to just iterate over an array, use `arr.forEach(....)` (4) only use `for(; ;)` when (2) and (3) aren't well suited to the task. — JLRishe, Nov 09 '17 at 12:07

JiangangXiong · Answer 2 · 2017-11-09T11:41:59.290

That's because the asynchronous task ,the i always be incremented before the request returned the result.

You can use let i = 0 in

```

for(var i = 0; i < dbResult.length; i++){

```

Or you can use the closure to solve the problem.

Think of this three example:

for (var i =0; i< 10;i++) {
  setTimeout(function(){
    console.log(i);
  },10)
}

for (let i =0; i< 10;i++) {
  setTimeout(function(){console.log(i)},10)
}

for(var i = 0; i < 10; i++){
  (function(j) {
    setTimeout(function() {
      console.log(j)
    }, 10);
  })(i);
}

Promise .then chain 'Cannot set property of undifined'

2 Answers2