3

I'm writing a Node.js app that:

  1. accepts a file (from an HTTP "POST"), then
  2. writes the file to a Box.com repository.

This Node.js code to read the file data from the HTTP "POST" message works perfectly:

 // ORIGINAL (non-Promise; parse only)
 app.post('/upload', function(req,res) {
   console.log('/upload...');

   var form = new multiparty.Form ();
   form.parse(req, function(err, fields, files) {
     res.writeHead(200, {'content-type': 'text/plain'});
     res.write('received upload:\n\n');
     res.end(util.inspect({fields: fields, files: files}));
 });

The problem is that reading the file data is just the first of several things I need to do, all of which involve asynchronous callbacks. So I'm trying to use promises to serialize the calls (gracefully handling errors as needed):

var Promise = require('bluebird');
  ...
  // IDEAL SOLUTION (using "promises")
  var data = {};
  try {
    parsePostMsg(req, data))
    .then(sendAck(res, data))
    .then(writeTempFile())
    .then(sendToBox())
    .then(deleteTempFile());
  } catch (e) {
    console.log("app.post(/upload) ERROR", e);
    deleteTempFile();
  }

PROBLEM:

The very first function, parsePostMsg(), itself has a callback. Which never gets invoked:

function parsePostMsg(req, data) {
  console.log("parsePostMsg...");
  return new Promise(function(resolve, reject) {
    var form = new multiparty.Form ();
    form.parse(req, function(err, fields, files) {
      data.fields = fields; // <-- This never gets called, so fields & files
      data.files = files;   //     never get initialized!
    });
  });
}

Q: How do I correctly 1) Create the promise, then 2) invoke parsePostMsg() so that 3) form.parse() correctly gets invoked along the chain?

Q: Am I even creating the "Promise" correctly, in the right place????

Q: What about resolve() and reject()? Where do they fit in?

======================================================================

ADDENDUM: I've tried many things, nothing has worked so far.

GOAL: to get these functions (and their associated callbacks, where applicable) to run in this order:

  1. parsePostMsg(req, data) // Wait for "parse form data" callback to complete
  2. sendAck(res, data) // Synchronous
  3. writeTempFile(data) // Build an HTTP message, send it, and wait for response from the remote server
  4. deleteTempFile(data) // Cleanup when everything is done

Here's a failing example:

/*
 * If the "timeout" values are the same (e.g. "10"), everything works fine.
 *
 * But if the timeout values are *different*, the order gets scrambled:
 *     Invoking parsePostMsg...
 *     ... OK ...
 *     Done: data= {}
 *     writeTemp@setting data.temp { temp: 'foo' }
 *     sendAck@acknowledging {} { temp: 'foo' }
 *     deleteTempFile@callback: data.temp was  foo
 *     parsePostMsg@setting data.{fields, files} {} { temp: undefined, fields: [], files: [] }
 */
var Promise = require('bluebird');

var req = {}, res = {}, data = {};
var temp;

function parsePostMsg(req, data) {
  console.log("parsePostMsg...");
  return new Promise(function(resolve, reject) {
    setTimeout(function() {
      data.fields = [];
      data.files = [];
      console.log("parsePostMsg@setting data.{fields, files}", req, data);
      resolve();
    }, 35);
  });
}

function sendAck(req, data) {
  console.log("sendAck...");
  return new Promise(function(resolve, reject) {
    setTimeout(function() {
      console.log("sendAck@acknowledging", req, data);
      resolve();
    }, 5);
  });
}

function writeTempFile(data) {
  console.log("writeTemp...");
  return new Promise(function(resolve, reject) {
    setTimeout(function() {
      data.temp = "foo";
      console.log("writeTemp@setting data.temp", data);
      resolve();
    }, 2);
  });
}

function deleteTempFile(data) {
  console.log("deleteTemp...");
  return new Promise(function(resolve, reject) {
    setTimeout(function() {
      console.log("deleteTempFile@callback: data.temp was ", data.temp);
      data.temp = undefined;
      resolve();
    }, 15);
  });
}

console.log("Invoking parsePostMsg...");
parsePostMsg(req, data)
  .then(sendAck(res, data))
  .then(writeTempFile(data))
  .then(deleteTempFile(data));
console.log("Done: data=", data);
Mifeet
  • 12,949
  • 5
  • 60
  • 108
paulsm4
  • 114,292
  • 17
  • 138
  • 190
  • Check in documentation what `.then` is supposed to accept https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then – zerkms Apr 28 '16 at 00:25
  • 3
    To use promises, ALL your async operations needs to be ";promisified" and return a promise that resolves with the value when the async operation completes successfully or rejects with a reason if the operation fails. – jfriend00 Apr 28 '16 at 00:28
  • You'll want to start with [How do I convert an existing callback API to promises?](http://stackoverflow.com/q/22519784/1048572) – Bergi Apr 28 '16 at 00:41

4 Answers4

3

Your failing example doesn't work because you are not chaining correctly.

Lets look at this code here:

parsePostMsg(req, data)
    .then(sendAck(res, data))
    .then(writeTempFile(data))
    .then(deleteTempFile(data))
    // This console.log will execute way before
    // the promise is resolved
    console.log("Done: data=", data);

What happens is that parsePostMsg() is called correctly, however everything after will execute way before the promise is resolved. This is because you are actually executing the functions immediately and then the output of these executions is what the promise will attempt to use when it resolves. This is why, if you set the timeout in parsePostMsg() to a couple of seconds, the output from that function will be logged last.

So what this should look like is like this:

parsePostMsg(req, data)
    .then(sendAck)
    .then(writeTempFile)
    .then(deleteTempFile)
    .then(function () {
        // Now the console.log will log when everything is done.
        console.log("Done: data=", data);
    });

Here you are telling the promise that it should execute the functions when the promise resolves. But to do this we must set up the promise chain in a correct way. To chain these methods together, we must return the values we want to use in a function, in the previous function. For example we must return the arguments for the sendAck() function, in the parsePostMsg() function.

Lets write the parsePostMsg so that it will return all the arguments that are required in the chain.

function parsePostMsg(req, res, data) {
    console.log("parsePostMsg...");
    return new Promise(function (resolve, reject) {
        setTimeout(function () {
            data.fields = [];
            data.files = [];
            console.log("parsePostMsg@setting data.{fields, files}", req, data);

            // Here we pass all the arguments into the resolve method
            // This means that the following then() call will receive
            // These argument. Take note that this is an array.
            resolve([req, res, data]);
        }, 3000);
    });
}

Now we have changed the parsePostMsg so that it will pass all it's arguments along in the chain. Lets change the other methods in the same way.

function sendAck(req, res, data) {
    console.log("sendAck...");
    return new Promise(function (resolve, reject) {
        setTimeout(function () {
            console.log("sendAck@acknowledging", res, data);
            resolve([req, res, data]);
        }, 3000);
    });
}

function writeTempFile(req, res, data) {
    console.log("writeTemp...");
    return new Promise(function (resolve, reject) {
        setTimeout(function () {
            data.temp = "foo";
            console.log("writeTemp@setting data.temp", data);
            resolve([req, res, data]);
        }, 3000);
    });
}

function deleteTempFile(req, res, data) {
    console.log("deleteTemp...");
    return new Promise(function (resolve, reject) {
        setTimeout(function () {
            console.log("deleteTempFile@callback: data.temp was ", data.temp);
            data.temp = undefined;
            resolve([req, res, data]);
        }, 3000);
    });
}

Now notice that all the functions take in 3 arguments but we are calling the resolve method with an array. If we simply use .then() this won't work as it is, but bluebird provides a special helper method called .spread() which destructs the array and calls the method with all the members of the array. Note that you can use ES2015 destructing instead of using .spread() if you are using ES2015.

So with using .spread() the code should look like this:

parsePostMsg(req, res, data)
    .spread(sendAck)
    .spread(writeTempFile)
    .spread(deleteTempFile)
    .spread(function (req, res, data) {
        // Now the console.log will log when everything is done.
        console.log("Done: data=", data);
    });

And here's the failure example you provided working correctly:

var Promise = require('bluebird');

var req = {},
    res = {},
    data = {};
var temp;

function parsePostMsg(req, res, data) {
    console.log("parsePostMsg...");
    return new Promise(function (resolve, reject) {
        setTimeout(function () {
            data.fields = [];
            data.files = [];
            console.log("parsePostMsg@setting data.{fields, files}", req, data);
            resolve([req, res, data]);
        }, 3000);
    });
}

function sendAck(req, res, data) {
    console.log("sendAck...");
    return new Promise(function (resolve, reject) {
        setTimeout(function () {
            console.log("sendAck@acknowledging", res, data);
            resolve([req, res, data]);
        }, 3000);
    });
}

function writeTempFile(req, res, data) {
    console.log("writeTemp...");
    return new Promise(function (resolve, reject) {
        setTimeout(function () {
            data.temp = "foo";
            console.log("writeTemp@setting data.temp", data);
            resolve([req, res, data]);
        }, 3000);
    });
}

function deleteTempFile(req, res, data) {
    console.log("deleteTemp...");
    return new Promise(function (resolve, reject) {
        setTimeout(function () {
            console.log("deleteTempFile@callback: data.temp was ", data.temp);
            data.temp = undefined;
            resolve([req, res, data]);
        }, 3000);
    });
}

console.log("Invoking parsePostMsg...");

parsePostMsg(req, res, data)
    .spread(sendAck)
    .spread(writeTempFile)
    .spread(deleteTempFile)
    .spread(function (req, res, data) {
        console.log("Done: data=", data);
    })
    .catch(function(err) {
        // Always put a .catch() at the end of your promise chains, ALWAYS,
    // it is literally the ultimate method to handle promise errors.
        console.warn(err);
    });
grimurd
  • 2,750
  • 1
  • 24
  • 39
  • 1
    BRILLIANT - thank you! Among the (key points!) I was missing: 1) use of "spread()" vs. ".then()", 2) using "resolve" to pass results from one "thenable" function to the next (eliminating any need for globals), 3) importance of calling the function by passing function name only - *WITHOUT* "()" and *WITHOUT* a parameter list. It works like a charm. Thank you. – paulsm4 Apr 30 '16 at 18:06
  • 1
    Just so you know, using new Promise() isn't necessary that often except for special cases in my experience. I recommend using Promise.try() instead to start a promise chain or if you are working with a module that using callbacks, use bluebird Promisify as was suggested by @JosephTheDreamer. You can most of the time simply return a value normally inside a promise function and that that will be passed along the chain. – grimurd Apr 30 '16 at 18:13
  • Point well taken. Actually, "Promise.promisify()" was one of (many, many) things I tried. But it didn't work ... because of the *other* problems, which you explained so well. I'll add "promisify()" back now - thank you for the reminder. – paulsm4 Apr 30 '16 at 18:16
2

Before anything else, I'd like to mention that Bluebird has a function called promisify that converts callback-based functions into promise-returning functions.

But under the hood...

Create the promise

You're already on the right track. A promise is an object with 2 states (resolved and rejected). All you need to do is define when it resolves or rejects. That's what the resolve and reject functions are for.

function parsePostMsg(req, data) {

  // Return a promise
  return new Promise(function(resolve, reject) {
    var form = new multiparty.Form();

    // that callse your async, callback-ish function during construction
    form.parse(req, function(err, fields, files) {

      // that rejects when there's an error
      if(err) reject(err);

      // or resolves when everything goes well
      else resolve({ fields: fields, files: files });

    });
  });
}

invoke parsePostMsg()

To know when your promisified async operation resolves or rejects, promises expose a then method which accepts 2 callbacks. The first one is executed when they promise resolves, the second one is executed when the promise is rejected. They are both passed the argument used when calling resolve/reject.

In this case, we expect the error from form.parse when the promise rejects or the object containing fields and files when the promise resolves.

// parsePostMsg returns a promise where we hook a then
parsePostMsg(req, data).then(function(data){

  // The object you passed to `resolve` is `data` in here
  // data.fields
  // data.files

}, function(error){

  // The `err` you passed to `reject` is `error` in here

})

Am I even creating the "Promise" correctly, in the right place????

Refer to first block

What about resolve() and reject()? Where do they fit in?

Refer to first block

Joseph
  • 117,725
  • 30
  • 181
  • 234
2

I'm semi new to promises but you should probably read this, https://promisesaplus.com/, also your try catch is useless I think, since any exceptions thrown by the promise are separate from that code.

The resolve function is the function to run when the promise is fulfilled, reject is for when it fails, reject is where you probably want your error handling. Remember everything that is attached to the promise is independent of any surrounding code.

Blue
  • 463
  • 4
  • 18
0

You're actually pretty close! You are creating the promise correctly and invoking it with .then. Unfortunately, the next step in your chain will never begin, because your promise never resolves!

resolve and reject are a Promise's way of saying "this worked, move on to the next step" or "this didn't work, throw an error!" respectively. In your case, you probably want to do something like this:

function parsePostMsg(req, data) {
  console.log("parsePostMsg...");
  return new Promise(function(resolve, reject) {
    var form = new multiparty.Form ();
    form.parse(req, function(err, fields, files) {
      if (err) {
        reject(err)
      } else {
        data.fields = fields;
        data.files = files;
        resolve();
      }
    });
  });
}

Notice how, with the promise model, you can "return" using resolve from your outer method from within an inner method!

Note that you will need to make sure all the steps in your chain follow this pattern. I've thrown together a simple example using setTimeout to mock async function calls to show you sort of what it would look like:

var data = {};
var req, res;
var temp;

function parsePostMsg(req, data) {
  console.log("parsePostMsg...");
  return new Promise(function(resolve, reject) {
    setTimeout(function() {
      data.fields = [];
      data.files = [];
      resolve();
    }, 10);
  });
}

function sendAck(req, data) {
  console.log("sendAck...");
  return new Promise(function(resolve, reject) {
    setTimeout(function() {
      console.log("acknowleding", req, data);
      resolve();
    }, 10);
  });
}

function writeTempFile() {
  console.log("writeTemp...");
  return new Promise(function(resolve, reject) {
    setTimeout(function() {
      temp = "foo";
      resolve();
    }, 10);
  });
}

function deleteTempFile() {
  console.log("deleteTemp...");
  return new Promise(function(resolve, reject) {
    setTimeout(function() {
      console.log("temp was ", temp);
      temp = undefined;
      resolve();
    }, 10);
  });
}

parsePostMsg(req, data)
  .then(sendAck(res, data))
  .then(writeTempFile())
  .then(deleteTempFile());
Hamms
  • 5,016
  • 21
  • 28
  • Thank you for a really excellent reply ... but it's still not working. If I run your example as-is, it's perfect. But if I modify the "timeout" values to a random values ... the callback order gets scrambled. The callback with the shorter timeout gets run first; the longer timeouts last. I'm trying to force "sendAck" to wait until parsePostMsg signals "done", to make "writeTempFile" to wait until "sendAck" is done, etc. Q: How can I enforce "order"? How can I "serialize" these calls?????? – paulsm4 Apr 30 '16 at 05:51