then is not a function error - Node promise library

Question

I'm new to Node and this is the first time I've tried to use a promise library. With that said ,this is the simple example I'm following: http://zpalexander.com/blog/javascript-promises-node-js/

This is the code that executes without error however as expected it logs "done" prior to finishing the file read and looping through the array.

    var Promise = require('bluebird');
var Converter = Promise.promisifyAll(require("csvtojson")).Converter;
var converter = new Converter({});
var Client = Promise.promisifyAll(require('node-rest-client')).Client;
var client = new Client();
var responseArray = [];

converter.fromFile("./ToterFeed.csv", function(err,result){
    for (var i = 0, len = result.length; i < len; i++) {
        var args = {
            data: JSON.stringify(result[i]),
            headers: { "Content-Type": "application/json" }
        };
        client.post("http://192.168.1.102:8080/api/ToterFeed/v1", args, function (data, response) {
            responseArray.push(data.responses);
            console.log(data.responses[0]);

        });


    }

})

console.log("done");

This is the updated code based on my interpretation. I receive the "then is not a function" error.

  var Promise = require('bluebird');
var Converter = Promise.promisifyAll(require("csvtojson")).Converter;
var converter = new Converter({});
var Client = Promise.promisifyAll(require('node-rest-client')).Client;
var client = new Client();
var responseArray = [];

converter.fromFile("./ToterFeed.csv").then(function(err,result){
    for (var i = 0, len = result.length; i < len; i++) {
        var args = {
            data: JSON.stringify(result[i]),
            headers: { "Content-Type": "application/json" }
        };
        client.post("http://192.168.1.102:8080/api/ToterFeed/v1", args, function (data, response) {
            responseArray.push(data.responses);
            console.log(data.responses[0]);
        });
    }

}).then(console.log("done"));

My goal is to run the fill read and array iteration to completion then print "done". Any insights is much appreciated! Thanks.

Solution

You're almost there, a few small changes needed - the HTTP post will also be asynchronous, so as you've wrapped it using promisifyAll, you can return that from your first callback (which will mean subsequent calls to .then will await its completion). Note also that promisifyAll creates extra functions with a suffix of Async, so you need to call those versions to get promises back.

As you're iterating over an array and creating multiple posts, you need to use Promise.all to await the completion of all of the promises you're creating.

Lastly, your final console.log needs to be wrapped in a function (.then() always takes a function as a parameter). Something like this should work:

var Promise = require('bluebird');
var Converter = Promise.promisifyAll(require("csvtojson")).Converter;
var converter = new Converter({});
var Client = Promise.promisifyAll(require('node-rest-client').Client);
var responseArray = [];

converter.fromFileAsync("./ToterFeed.csv").then(function(result){
    var promises = [];
    for (var i = 0, len = result.length; i < len; i++) {
        var args = {
            data: JSON.stringify(result[i]),
            headers: { "Content-Type": "application/json" }
        };
        // Add a promise to the array of promises - this
        // will call the post then process the result.
        promises.push(
           Client.postAsync(
               "http://192.168.1.102:8080/api/ToterFeed/v1", 
               args
           ).then(function (data) {
               responseArray.push(data.responses);
           }));
    }
    // Return Promise.all on all of our posts and 
    // their response handlers
    return Promise.all(promises);
}).then(function () {
    console.log("done");
});

Comments

[Ken]

Thanks for the example, the other comments make more sense to me after seeing the example. I'm getting a different error now and I'm not sure if I'm doing something wrong or if it's a bug with bluebird. On this line: converter.fromFileAsync("./ToterFeed.csv").then(function(err‌​,result){ the array is now being returned to the err variable instead of the result variable. Any ideas on why?

[Bergi]

Where does that for loop end? Looks like a syntax error to me.

[Ken]

Just before this line: }}).then(function (data) { I had to add another curly bracket.

[Mark Hughes]

No, @Bergi is right - you need to collect the promises up and do a .all then a .then for the final console.log to operate after ALL of the promises have resolved, else it'll only do the first post. I'll update...

[Mark Hughes]

@Bergi it should be correct now - added a use of Promise.all to await all iterations of the for loop.

[Mark Hughes]

@Ken Take a look at the updated version now - on your question about the data being placed in the err variable, unlike Node's idiomatic callback signature, promises resolve with the single parameter to the then function being the result of the promise resolution. Error handling is done with a second function to the then (i.e. `.then(function (data) { // success }, function (err) { // failure }), or with a .catch at the end of the chain.

[Bergi]

Thanks. You should however just return data.responses;, then the Promise.all will resolve with the responseArray by itself - no need to manually push the results there.

[Ken]

This is starting to make more sense, I see what you did with the promises array and promise.all. The catch at the end of the chain is certainly cleaner and easier to work with. I'm getting the error "client.postAsync is not a function" it looks correct to me. Also, could I have taken my entire script (except the "done") and wrapped it in a function, then used promisifyAll on it?

[Mark Hughes]

That would indeed be neater @Bergi - for clarity, that would be because Promise.all resolves with an array of the results of all promises that are within it.

[Mark Hughes]

@Ken not really, no, as you have a number of unhandled asynchronous operations which wouldn't be handled no matter what you wrapped around it. It should also be noted that promisifyAll is intended to bridge old non-promise-aware library code for use in new promise-aware code - if you're writing your own code and intend to use promises, then use them directly.

[Ken]

So I reread zpalexander.com/blog/javascript-promises-node-js which now makes a whole lot more sense. :) However I keep studying the code and still can't figure out why I get the error "client.postAsync is not a function"

[Mark Hughes]

@Ken It sounds like a problem with the bluebird promisify call to me, I'd start by checking that is correct. My first guess would be that as you're new-ing up an instance of Client, you need to call promisify on that instance, rather than on the Client class.

[Ken]

I've really simplified it but still get the same error. Could this be a bug with promisifyAll? var Promise = require('bluebird'); var Client = Promise.promisifyAll(require('node-rest-client')).Client; var args = { data: {"MPN":"abc123","ITEM_DESC":"cool description"}, headers: { "Content-Type": "application/json" } }; Client.postAsync( "192.168.1.102:8080/api/ToterFeed/v1", args ).then(function (data) { console.log(data.responses); });

[Mark Hughes]

@Ken Have you tried var Client = require('node-rest-client').Client; var client = Promise.promisifyAll(new Client()); ? i.e. use promisifyAll on the instance, rather than the class?

[Mark Hughes]

@Ken ... Or var Client = Promise.promisifyAll(require('node-rest-client').Client); ? I think the problem is you're not promisifying Client.

[Ken]

Yeah, tried those first and I get the same error. To simplify I didn't new it up I just: var Client = Promise.promisifyAll(require('node-rest-client')).Client;

[Mark Hughes]

You need to call promisifyAll on the object you want to add the Async methods to - so that final example there should be var Client = Promise.promisifyAll(require('node-rest-client').Client); - note the position of the final bracket.

[Ken]

That gives me the same error. since I'm not having a problem wrapping csvtojson I am going to try replacing node-rest-client with a different rest API.

[Ken]

Just to finish this out, I was able to get everything working using needle. Bluebird's PromisifyAll worked great with it. Thanks for all the help and suggestions!

[Mark Hughes]

Glad you got it working @Ken - they can be tricky but once you've got your head around them, Promises make the code much clearer.