Skip to content
This repository was archived by the owner on Jun 18, 2020. It is now read-only.

[WIP] Refactor to support Promises and Async/Await #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
node_modules/**
build

#template
58 changes: 55 additions & 3 deletions template/node10-express/function/handler.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,64 @@
"use strict"

module.exports = (event, context) => {
let err;
// callback version
const delayCallback = cb => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now part of the main template, was that intended or was it a mistake?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see handler.js as an example on how to build the code for the serverless function. As the main motivation for this refactoring is to handle asynchronicity, I use that function to simulate some load, and show how to take advantage of this new feature. But it's just a proposal, you can keep it or use the original example.

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) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next parameter is new I think? Why is it needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we use express we are using a framework, which is in control of the execution of our code, not us.
Having said that, the middleware function at index.js is in the "middle" of the execution flow, so we need to "fit" in that schema, node.js coders are used to and aware of that.
I think it's better if we just "build" the response in handler.js and let middleware in index.js take care of sending the response and handling errors.
Then once you build the response you call next to return control to the framework.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next parameter is supposed to be used when you return a callback to the middleware, it's not used when returning a Promise (Promise or Async/Await)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't line the next() parameter, it's confusing. mixes express and faas programming.
With that said, I believe callbacks should removed altogether, so it doesn't become an issue anymore

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the use of the next() function it is confusing, but is something a express user is used to. I'm open to suggestions on how to solve the callback call in a different way. I already suggested that we need to drop support for it, but until that happens there must be a way to use it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other way is to pass the res object to the handler function, with passes the control of the flow to it. I think the function main code has to be in control of that.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or not use callbacks ;-)

I rebuilt a brand new template - thanks for your inspiration. will publish soon - I am testing right now. It also works differently, more inlined with lambda.

  • New better intuitive handler.js
  • Super tiny docker image using multistage build, down to 65MB (from 118MB!)
  • Garbage collector manually triggered every 30s
  • Image properly handles signals using a graceful shutdown
  • Catch all uncaught exceptions and unhandled promise rejections

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's something I already asked about, what happens if we don't use callbacks at all

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

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

next();
})
}

// 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);
})
});
*/

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

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

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

return context;
}
*/
80 changes: 42 additions & 38 deletions template/node10-express/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ app.use(bodyParser.raw());
app.use(bodyParser.text({ type : "text/*" }));
app.disable('x-powered-by');

let isObject = a => !!a && (a.constructor === Object);

class FunctionEvent {
constructor(req) {
this.body = req.body;
Expand All @@ -25,59 +27,69 @@ class FunctionEvent {
}

class FunctionContext {
constructor(cb) {
this.value = 200;
this.cb = cb;
this.headerValues = {};
constructor() {
this.statusCode = 200;
this.headerValues = {};
}

status(value) {
if(!value) {
return this.value;
if (value) {
this.statusCode = value;
}

this.value = value;
return this;
}

headers(value) {
if(!value) {
return this.headerValues;
if(value && value.constructor === Object) {
this.headerValues = value;
}

this.headerValues = value;
return this;
return this;
}

succeed(value) {
let err;
this.cb(err, value);
this.succeedValue = (Array.isArray(value) || isObject(value)) ? JSON.stringify(value) : value;
}

fail(value) {
let message;
this.cb(value, message);
fail(message) {
this.statusCode = 500;
this.failMessage = message;
}
}

var middleware = (req, res) => {
let cb = (err, functionResult) => {
if (err) {
console.error(err);
return res.status(500).send(err);
}
const middleware = (req, res) => {
let fnEvent = new FunctionEvent(req);
let fnContext = new FunctionContext();

if(isArray(functionResult) || isObject(functionResult)) {
res.set(fnContext.headers()).status(fnContext.status()).send(JSON.stringify(functionResult));
} else {
res.set(fnContext.headers()).status(fnContext.status()).send(functionResult);
const sendFail = () => {
res.status(500).send(fnContext.failMessage);
}

const handleResult = result => {
if (fnContext.failMessage) {
sendFail()
}
};
else {
res.set(fnContext.headers).status(fnContext.statusCode).send(fnContext.succeedValue);
}
}

let fnEvent = new FunctionEvent(req);
let fnContext = new FunctionContext(cb);
let next = () => handleResult(fnContext);

const result = handler(fnEvent, fnContext, next);

handler(fnEvent, fnContext, cb);
if (result instanceof FunctionContext) {
handleResult(result);
}
else if (result instanceof Promise) {
result
.then(asyncResult => handleResult(asyncResult))
.catch(err => {
fnContext.fail(err);
handleResult(fnContext);
})
}
};

app.post('/*', middleware);
Expand All @@ -91,11 +103,3 @@ const port = process.env.http_port || 3000;
app.listen(port, () => {
console.log(`OpenFaaS Node.js listening on port: ${port}`)
});

let isArray = (a) => {
return (!!a) && (a.constructor === Array);
};

let isObject = (a) => {
return (!!a) && (a.constructor === Object);
};