node10-express-template icon indicating copy to clipboard operation
node10-express-template copied to clipboard

[WIP] Refactor to support Promises and Async/Await

Open padiazg opened this issue 5 years ago • 3 comments

Signed-off-by: Patricio Diaz [email protected]

This refactor is intended to give the node10-express template the ability to respond to promise or async/await based handler functions.

The handler in this PR has 3 examples for each scenario: callback, promise, async/await.

This proposal moves the control of sending the response to the middleware function in index.js.

When using the Promise and Async/Await flavors of handler, you must return the context variable with the resulting value you want to return.

If you are using the Callback flavor of handler, you must use the next function which is passed as the third parameter to handler to pass the control to middleware

How it was tested

For a Callback version of handler.js

$ faas new node10-express-cb --lang node10-express --prefix padiazg

handler.js

"use strict"

// callback version
const delayCallback = cb => {
    const min = 1; // 1 sec
    const max = 5; // 5 sec
    const delay = Math.round((Math.random() * (max - min) + min));
    setTimeout(() => cb(delay),  delay * 1000);
}

module.exports = (event, context, next) => {
    delayCallback(delay => {
        const result = {
            status: "You said: " + JSON.stringify(event.body),
            delay
        };
        
        context
          .status(200)
          .succeed(result);

        next();        
    })
}
$ faas up -f node10-express-cb.yml 
$ echo -n "async" | faas invoke node10-express-cb -
{"status":"You said: \"async\"","delay":2}

For a Promise version of handler.js

$ faas new node10-express-promise --lang node10-express --prefix padiazg

handler.js

"use strict"

// callback version
const delayCallback = cb => {
    const min = 1; // 1 sec
    const max = 5; // 5 sec
    const delay = Math.round((Math.random() * (max - min) + min));
    setTimeout(() => cb(delay),  delay * 1000);
}

// Uncomment the following line to use it in the promise or async/await versions
const delayPromise = () => new Promise((resolve, reject) => delayCallback(delay => resolve(delay)) )

// Promise version
module.exports = (event, context) => new Promise((resolve, reject) => {
    delayPromise()
    .then(delay => {
        const result =             {
            status: "You said: " + JSON.stringify(event.body),
            delay
        };
    
        context
            .status(200)
            .succeed(result);
            
        return resolve(context);
    })
});
$ faas up -f node10-express-promise.yml
$ echo -n "promise" | faas node10-express-promise -
{"status":"You said: \"promise\"","delay":2}

For a Async/Await version of handler.js

$ faas new node10-express-async --lang node10-express --prefix padiazg

handler.js

"use strict"

// callback version
const delayCallback = cb => {
    const min = 1; // 1 sec
    const max = 5; // 5 sec
    const delay = Math.round((Math.random() * (max - min) + min));
    setTimeout(() => cb(delay),  delay * 1000);
}

// Uncomment the following line to use it in the promise or async/await versions
const delayPromise = () => new Promise((resolve, reject) => delayCallback(delay => resolve(delay)) )

// async/await version
module.exports = async (event, context) => {  
    const delay = await delayPromise();

    const result =             {
        status: "You said: " + JSON.stringify(event.body),
        delay
    };

    context
        .status(200)
        .succeed(result);

    return context;
}
$ faas up -f node10-express-async.yml
$ echo -n "async" | faas invoke node10-express-async -
{"status":"You said: \"async\"","delay":2}

padiazg avatar Sep 17 '19 21:09 padiazg

Thank you for this PR :+1:

alexellis avatar Oct 08 '19 06:10 alexellis

As a personal comment, I think we can start thinking on dropping support for Callbacks and just support Promises, I think nobody would complain nowadays.

padiazg avatar Oct 14 '19 13:10 padiazg

generally speaking I don't like the way openfaas handles the context. context and results should be two different structures.

orefalo avatar Oct 29 '19 09:10 orefalo