Azure-functions-host: Allow returning promises from functions as opposed to context.done()

[danderson00]

A more modern, industry accepted pattern for handling async operations is to use promises. Functions should allow developers to return a promise, rather than having to call context.done().

[davidebbo]

Can you illustrate with a short sample of what a Hello World http request might look like, so we can easily see the side by side? Thanks!

[danderson00]

A quick sample of connecting to a SQL database might look like:

var sql = require('mssql')

module.exports = function (context, myTimer) {
    return sql.connect(process.env.SQLAZURECONNSTR_Default)
        .then(() => new sql.Request().query('select * from mytable'))
        .then((recordset) => context.log(JSON.stringify(recordset)))
}

Still getting the SQL connection to work, but this gives an example of returning a promise. I've added the mssql module using kudu and set the Default connection string in the portal.

It should only be about 5 lines in the module that executes the function - test to see if the return value is a promise and call context.done as appropriate.

[danderson00]

It's worth noting for now that the addition of the last line below is an alternative:

var sql = require('mssql')

module.exports = function (context, myTimer) {
    sql.connect(process.env.SQLAZURECONNSTR_Default) 
        .then(() => new sql.Request().query('select * from todoitem'))
        .then((recordset) => context.log(JSON.stringify(recordset)))
        .then(context.done).catch(context.done)
}

[danderson00]

It's also worth exploring not requiring context.done() for synchronous functions. mocha looks at the function parameters and if one is provided, or a promise is returned, expect the function to be asynchronous.

For example,

test("synchronous test", function () {
    // as done is not a parameter to the test function, it is considered synchronous
})

test("asynchronous test", function (done) {
    done() // must be called
})

test("asynchronous test with promises", function () {
    return new Promise(function (resolve, reject) {
        resolve()
    })
})

[brianleroux]

Promises are better off in userland until they become a part of the core Node API. Yes, they are standardized but that wasn't agreed as a great idea by all and the entire Node API currently uses errbacks.

[danderson00]

We don't need to rely on any specific promise library, just check that it is a "thenable" that conforms to A+ Promise spec and call the .then/.catch methods on it.

[danderson00]

I also disagree with the statement that we should ignore promises because the Node API uses callbacks.

[brianleroux]

I didn't say ignore them. You can use them all you want. I'm saying a Node runtime should have good symmetry with Node APIs. Promise support _didn't_ land in Node for a host of very good reasons which should be considered by a platform depending on it. We had a nightmarish time dealing with Promises in Cordova and removed them too. Some of those issues are being corrected. Some aren't. At any rate, they should remain opt-in at the user land level not a platform concern anytime soon.

[danderson00]

They would be 100% opt in. Using callbacks, i.e. calling context.done will still work fine.

Promises are now the generally accepted approach for dealing with asynchronous operations, it would seem to me to be remiss to not cater for them. The Node API doesn't make use of them for a bunch of reasons (backwards compatibility being a major concern) which don't necessarily apply in our scenario.

[brianleroux]

Opt in: cool.

Promises are a good tool for beginners but I would not characterize them as more. I also would not use them in a production system I care about. Most of the Node core would agree. That should give you pause.

Async (the library) is a significantly more robust library for dealing w/ async operations and better it is natural to the platform idioms of errbacks.

http://npm-stat.com/charts.html?package=async&author=&from=&to=

(At any rate: opt-in is the way to go.)

[danderson00]

Fair enough. Would you care to provide more detail on why you wouldn't use promises in a production system?

[brianleroux]

• performance: they are slower…and this adds up in a larger async system
• swallowing errors: unacceptable
• non cancellable
• various userland libraries _can_ and do conflict (was a huge issue in Cordova)

Promises are easy (familiar to sync style programmers) however they are not simple. They conflate flow control with state wheres errbacks are _only_ flow control.

This discussion is useful:

https://github.com/nodejs/node/pull/5020

[danderson00]

You certainly make some compelling arguments. There are always pros and cons! I certainly have no intention of making this a lengthy debate!

Swallowing errors is probably the most problematic issue, but this is a user code issue. I've always had a firm belief that we should always have automated tests to catch these sorts of errors.

I think the biggest advantage of promises is that they are inherently more composable. Having a consistent interface that can be passed around allows the composition of asynchronous processes at the right level of abstraction, e.g. you might want to serialize a series of file writes, return a promise to represent that series of operations, but the consumer might parallelize a bunch of those operations - promises make this trivial and intuitive.

haha dang jQuery for not conforming to spec.

Anyway, let's not drag this out - opt in all the way!

[fabiocav]

@mamaso :)