Cannot read property 'then' of undefined when returning nested promise

Question

I keep getting the following error: TypeError: Cannot read property 'then' of undefined, in checkInventoryData. How can I resolve it?

function checkInventoryData(url, size, code) {
  var inventoryData = { };

  return $http.get(url).then(function(response) {
    var html = response.data;

    // getting error here: TypeError: Cannot read property 'then' of undefined
    getInventoryProductQty(html, size).then(function(result) {
        result = inventoryData.productQty;
    });

    return inventoryData;
  });
}

function getInventoryProductQty(html, size) {
  var inventoryAvailable;
  var deferred = $q.defer();
  try {
    var inventoryAvailablity = getInventoryAvailability(html, size);
    inventoryAvailablity.then(function(result) {
      if(result) {
        inventoryAvailable = result.getAttribute('data-available');
        deferred.resolve(inventoryAvailable);
        return deferred.promise;
      }
    });
  }
  catch(err) {
    inventoryAvailable = null;
    return inventoryAvailable;
  }
}

as an aside, unrelated to my answer, there is no need to set inventoryAvailable to null before returning it, you can just return null, and instead of assigning `var inventoryAvailability = getInventoryAvailability(html, size)`, you can just chain off of the function call on the next line — Jim Deville, Feb 26 '17 at 20:12
You have a lot more issues with this code... once the `then` issue is fixed, `checkInventoryData` will always return an empty object. — trincot, Feb 26 '17 at 20:19
@trincot how do I fix both issues? Would really appreciate your help — methuselah, Feb 26 '17 at 20:22
Avoid the [deferred antipattern](http://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, Feb 26 '17 at 20:41

Jim Deville · Accepted Answer · 2017-02-26T20:34:00.587

1

You need to return a promise from getInventoryProductQty, right now you return null in the error case, but nothing in the normal case (and neither of those are promises.

EDIT - showing this answer in OPs code, and comment on the structure.

function checkInventoryData(url, size, code) {
  var inventoryData = { };

  return $http.get(url).then(function(response) {
    var html = response.data;

    // getting error here: TypeError: Cannot read property 'then' of undefined
    getInventoryProductQty(html, size).then(function(result) {
        result = inventoryData.productQty; // this has not been set at this point, you need to set it somehow (@trincot's comment)
    });

    return inventoryData;
  });
}

function getInventoryProductQty(html, size) {
  var inventoryAvailable;
  var deferred = $q.defer();
  try {
    var inventoryAvailablity = getInventoryAvailability(html, size);
    return inventoryAvailablity.then(function(result) { // need to add this return to return the result of the then func
      if(result) {
        inventoryAvailable = result.getAttribute('data-available');
        deferred.resolve(inventoryAvailable);
        return deferred.promise;
      }
    });
  }
  catch(err) {
    //inventoryAvailable = null;
    //return inventoryAvailable;
    return $q.resolve(null); // need something like this to return a promise from the failure case
  }
}

You also should make getInventoryAvailability return a promise so that it can be called with a .catch instead of a try...catch, but that's beyond the scope of this

edited Feb 26 '17 at 20:34

answered Feb 26 '17 at 20:10

Jim Deville

10,632
1
37
47

I thought that `return deferred.promise;` was the promise being returned? – methuselah Feb 26 '17 at 20:11
It would be, but you aren't. You are returning `deferred.promise` from within the then in the try branch, but nothing from the top level function – Jim Deville Feb 26 '17 at 20:12
1

this statement is within a `then` so it only resolves this promise. But nothing is returned outside. – ValLeNain Feb 26 '17 at 20:12
2

what you probably want to do is `return inventoryAvailablity.then` instead. But take care of your `catch` which does not return a promise, as @methuselah pointed out. – ValLeNain Feb 26 '17 at 20:14
@ValLeNain can you please show me an example of how this would look? – methuselah Feb 26 '17 at 20:29
1

Avoid the [deferred antipattern](http://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Feb 26 '17 at 20:39
@Bergi didn't know it had a name, but that's where I was heading with the last line – Jim Deville Feb 26 '17 at 20:40
@JimDeville struggling to get `inventoryData`: http://pastebin.com/Lus2wnes at the moment, how would I refactor this? – methuselah Feb 26 '17 at 21:15
Not sure what the problem with that is – Jim Deville Feb 26 '17 at 21:28

ValLeNain · Answer 2 · 2017-02-26T20:40:22.070

What you're doing for now, inside getInventoryProductQty

inventoryAvailablity.then(function(result) {
      if(result) {
        inventoryAvailable = result.getAttribute('data-available');
        deferred.resolve(inventoryAvailable);
        return deferred.promise;
      }
    });

This is not a return statement and so your function does not return anything.
If what you would like to return is inventoryAvailable (once it has been retrieved), you would do something like

return inventoryAvailablity.then(function(result) {
  if(result) {
    inventoryAvailable = result.getAttribute('data-available');
    return inventoryAvailable;
  } else {
    /* reject ? */
  }
});

[edit]
I would actually rewrite the function this way

function getInventoryProductQty(html, size) {
  var inventoryAvailable;
  try {
    var inventoryAvailablity = getInventoryAvailability(html, size);
    return inventoryAvailablity.then(function(result) {
      if(result) {
        inventoryAvailable = result.getAttribute('data-available');
        return inventoryAvailable;
      } else {
        /* reject ? */
      }
    });
  }
  catch(err) {
    return $q.reject(err);
  }
}

score 1 · Answer 3 · answered Feb 26 '17 at 20:35

Once you rely on a promise, you need to make sure to keep returning promises in the call chain.

Here is the suggested code:

function checkInventoryData(url, size, code) {
  // Warning: code is nowhere used!
  // Return(!) the promise:
  return $http.get(url).then(function(response) {
    var html = response.data;
    return getInventoryProductQty(html, size);
  });
}

function getInventoryProductQty(html, size) {
  try {
    // return the result of `then`, which is a promise:
    return getInventoryAvailability(html, size).then(function(result) {
      if(result) {
        // Just return the value. No need to return a promise.
        // I assume you have a productQty property here:
        return result.getAttribute('data-available').productQty;
      } else { // also deal with the failure:
        throw "no result from getInventoryAvailability";
      }
    });
  }
  catch(err) {
    // return a failed promise here:
    return $q.reject(err);
  }
}

Don't expect checkInventoryData to synchronously return the value. It is also a promise, and you need to apply then to it to get the promised value in a callback function. Also make sure to use a catch method whenever you call checkInventoryData, so you can deal with errors.

Given that `getInventoryAvailability` never throws, that `try`/`catch` is pretty useless — Bergi, Feb 26 '17 at 20:40
That is indeed true if `getInventoryAvailability` can be trusted to return a promise. But OP did not provide the code for it. — trincot, Feb 26 '17 at 20:45

Cannot read property 'then' of undefined when returning nested promise

3 Answers3