This is an automated email from the ASF dual-hosted git repository.
jamesthomas pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/incubator-openwhisk-runtime-nodejs.git
The following commit(s) were added to refs/heads/master by this push:
new b952414 Refactoring of the service and runner, no semantic changes
intended. (#133)
b952414 is described below
commit b952414ffa321522298e033c749fcf260ca21abb
Author: rodric rabbah <[email protected]>
AuthorDate: Fri Jun 14 10:06:17 2019 -0400
Refactoring of the service and runner, no semantic changes intended. (#133)
* Refactor runner. No semantic change intended.
* Replace vars with lets/consts.
* Factor run method. No semantic changes intended.
* Add invariant checks in doRun.
* Javascript-isms. No semantic change intended.
* Fix typo.
---
core/nodejs10Action/Dockerfile | 2 +-
core/nodejs10Action/knative/Dockerfile | 2 +-
core/nodejsActionBase/runner.js | 251 ++++++++++++++++-----------------
core/nodejsActionBase/src/service.js | 119 ++++++++--------
4 files changed, 181 insertions(+), 193 deletions(-)
diff --git a/core/nodejs10Action/Dockerfile b/core/nodejs10Action/Dockerfile
index a2dbaf6..9ec24ff 100644
--- a/core/nodejs10Action/Dockerfile
+++ b/core/nodejs10Action/Dockerfile
@@ -28,5 +28,5 @@ COPY ./package.json /
RUN cd / && npm install --no-package-lock \
&& npm cache clean --force
EXPOSE 8080
-# The flag --experimental-worker is to be use with care as it's an
expirimental feature more info here
https://nodejs.org/docs/latest-v10.x/api/worker_threads.html
+# The flag --experimental-worker enables worker threads, see
https://nodejs.org/docs/latest-v10.x/api/worker_threads.html
CMD node --experimental-worker --expose-gc app.js
diff --git a/core/nodejs10Action/knative/Dockerfile
b/core/nodejs10Action/knative/Dockerfile
index a519498..b1299cc 100644
--- a/core/nodejs10Action/knative/Dockerfile
+++ b/core/nodejs10Action/knative/Dockerfile
@@ -34,5 +34,5 @@ COPY ./core/nodejs10Action/package.json /
RUN cd / && npm install --no-package-lock \
&& npm cache clean --force
EXPOSE 8080
-# The flag --experimental-worker is to be use with care as it's an
expirimental feature more info here
https://nodejs.org/docs/latest-v10.x/api/worker_threads.html
+# The flag --experimental-worker enables worker threads, see
https://nodejs.org/docs/latest-v10.x/api/worker_threads.html
CMD node --experimental-worker --expose-gc app.js
diff --git a/core/nodejsActionBase/runner.js b/core/nodejsActionBase/runner.js
index 42cce76..c65bcde 100644
--- a/core/nodejsActionBase/runner.js
+++ b/core/nodejsActionBase/runner.js
@@ -16,172 +16,157 @@
*/
/**
- * Object which encapsulates a first-class function, the user code for
- * an action.
+ * Object which encapsulates a first-class function, the user code for an
action.
*
* This file (runner.js) must currently live in root directory for
nodeJsAction.
*/
-var util = require('util');
-var child_process = require('child_process');
-var fs = require('fs');
-var path = require('path');
+const fs = require('fs');
+const path = require('path');
-const serializeError = require('serialize-error');
+class NodeActionRunner {
-function NodeActionRunner() {
- // Use this ref inside closures etc.
- var thisRunner = this;
-
- this.userScriptMain = undefined;
-
- this.init = function (message) {
- function assertMainIsFunction() {
- if (typeof thisRunner.userScriptMain !== 'function') {
- throw "Action entrypoint '" + message.main + "' is not a
function.";
- }
- }
+ constructor() {
+ this.userScriptMain = undefined;
+ }
- // Loading the user code.
+ /** Initializes the runner with the user function. */
+ init(message) {
if (message.binary) {
// The code is a base64-encoded zip file.
- return unzipInTmpDir(message.code).then(function (moduleDir) {
- let parts = splitMainHandler(message.main);
- if (parts === undefined) {
- // message.main is guaranteed to not be empty but be
defensive anyway
- return Promise.reject('Name of main function is not
valid.');
- }
-
- // if there is only one property in the "main" handler, it is
the function name
- // and the module name is specified either from package.json
or assumed to be index.js
- let [index, main] = parts;
-
- try {
- // Set the executable directory to the project dir
+ return unzipInTmpDir(message.code)
+ .then(moduleDir => {
+ let parts = splitMainHandler(message.main);
+ if (parts === undefined) {
+ // message.main is guaranteed to not be empty but be
defensive anyway
+ return Promise.reject('Name of main function is not
valid.');
+ }
+
+ // If there is only one property in the "main" handler, it
is the function name
+ // and the module name is specified either from
package.json or assumed to be index.js.
+ let [index, main] = parts;
+
+ // Set the executable directory to the project dir.
process.chdir(moduleDir);
if (index === undefined && !fs.existsSync('package.json')
&& !fs.existsSync('index.js')) {
return Promise.reject('Zipped actions must contain
either package.json or index.js at the root.');
}
- // The module to require
+ // The module to require.
let whatToRequire = index !== undefined ?
path.join(moduleDir, index) : moduleDir;
- thisRunner.userScriptMain = eval('require("' +
whatToRequire + '").' + main);
- assertMainIsFunction();
+ this.userScriptMain = eval('require("' + whatToRequire +
'").' + main);
+ assertMainIsFunction(this.userScriptMain, message.main);
// The value 'true' has no special meaning here; the
successful state is
// fully reflected in the successful resolution of the
promise.
return true;
- } catch (e) {
- return Promise.reject(e);
- }
- }).catch(function (error) {
- return Promise.reject(error);
- });
- } else {
+ })
+ .catch(error => Promise.reject(error));
+ } else try {
// The code is a plain old JS file.
+ this.userScriptMain = eval('(function(){' + message.code +
'\nreturn ' + message.main + '})()');
+ assertMainIsFunction(this.userScriptMain, message.main);
+
+ return Promise.resolve(true); // See comment above about 'true';
it has no specific meaning.
+ } catch (e) {
+ return Promise.reject(e);
+ }
+ };
+
+ run(args) {
+ return new Promise((resolve, reject) => {
try {
- thisRunner.userScriptMain = eval('(function(){' + message.code
+ '\nreturn ' + message.main + '})()');
- assertMainIsFunction();
- // See comment above about 'true'; it has no specific meaning.
- return Promise.resolve(true);
+ var result = this.userScriptMain(args);
} catch (e) {
- return Promise.reject(e);
+ reject(e);
}
- }
+
+ this.finalizeResult(result, resolve);
+ });
};
- // Returns a Promise with the result of the user code invocation.
- // The Promise is rejected iff the user code throws.
- this.run = function (args) {
- return new Promise(
- function (resolve, reject) {
- try {
- var result = thisRunner.userScriptMain(args);
- } catch (e) {
- reject(e);
- }
-
- // Non-promises/undefined instantly resolve.
- Promise.resolve(result).then(function (resolvedResult) {
- // This happens, e.g. if you just have "return;"
- if (typeof resolvedResult === "undefined") {
- resolvedResult = {};
- }
- resolve(resolvedResult);
- }).catch(function (error) {
- // A rejected Promise from the user code maps into a
- // successful promise wrapping a whisk-encoded error.
-
- // Special case if the user just called `reject()`.
- if (!error) {
- resolve({error: {}});
- } else {
- resolve({error: serializeError(error)});
- }
- });
+ finalizeResult(result, resolve) {
+ // Non-promises/undefined instantly resolve.
+ Promise.resolve(result).then(resolvedResult => {
+ // This happens, e.g. if you just have "return;"
+ if (typeof resolvedResult === "undefined") {
+ resolvedResult = {};
}
- );
- };
+ resolve(resolvedResult);
+ }).catch(error => {
+ // A rejected Promise from the user code maps into a
+ // successful promise wrapping a whisk-encoded error.
+
+ // Special case if the user just called "reject()".
+ if (!error) {
+ resolve({error: {}});
+ } else {
+ const serializeError = require('serialize-error');
+ resolve({error: serializeError(error)});
+ }
+ });
+ }
+}
- // Helper function to copy a base64-encoded zip file to a temporary
location,
- // decompress it into temporary directory, and return the name of that
directory.
- // Note that this makes heavy use of shell commands because:
- // 1) Node 0.12 doesn't have many of the useful fs functions.
- // 2) We know in which environment we're running.
- function unzipInTmpDir(base64) {
- var mkTempCmd = "mktemp -d XXXXXXXX";
- return exec(mkTempCmd).then(function (tmpDir1) {
- return new Promise(
- function (resolve, reject) {
- var zipFile = path.join(tmpDir1, "action.zip");
- fs.writeFile(zipFile, base64, "base64", function (err) {
- if (err) {
- reject("There was an error reading the action
archive.");
- }
- resolve(zipFile);
- });
- }
- );
- }).then(function (zipFile) {
- return exec(mkTempCmd).then(function (tmpDir2) {
- return exec("unzip -qq " + zipFile + " -d " +
tmpDir2).then(function (res) {
- return path.resolve(tmpDir2);
- }).catch(function (error) {
- return Promise.reject("There was an error uncompressing
the action archive.");
- });
+/**
+ * Copies the base64 encoded zip file contents to a temporary location,
+ * decompresses it and returns the name of that directory.
+ *
+ * Note that this makes heavy use of shell commands because the environment is
expected
+ * to provide the required executables.
+ */
+function unzipInTmpDir(zipFileContents) {
+ const mkTempCmd = "mktemp -d XXXXXXXX";
+ return exec(mkTempCmd).then(tmpDir => {
+ return new Promise((resolve, reject) => {
+ const zipFile = path.join(tmpDir, "action.zip");
+ fs.writeFile(zipFile, zipFileContents, "base64", err => {
+ if (!err) resolve(zipFile);
+ else reject("There was an error reading the action archive.");
});
});
- }
+ }).then(zipFile => {
+ return exec(mkTempCmd).then(tmpDir => {
+ return exec("unzip -qq " + zipFile + " -d " + tmpDir)
+ .then(res => path.resolve(tmpDir))
+ .catch(error => Promise.reject("There was an error
uncompressing the action archive."));
+ });
+ });
+}
- // Helper function to run shell commands.
- function exec(cmd) {
- return new Promise(
- function (resolve, reject) {
- child_process.exec(cmd, function (error, stdout, stderr) {
- if (error) {
- reject(stderr.trim());
- } else {
- resolve(stdout.trim());
- }
- });
+/** Helper function to run shell commands. */
+function exec(cmd) {
+ const child_process = require('child_process');
+
+ return new Promise((resolve, reject) => {
+ child_process.exec(cmd, (error, stdout, stderr) => {
+ if (!error) {
+ resolve(stdout.trim());
+ } else {
+ reject(stderr.trim());
}
- );
- }
+ });
+ });
+}
- /**
- * Splits handler into module name and path to main.
- * If the string contains no '.', return [ undefined, the string ].
- * If the string contains one or more '.', return [ string up to first
period, rest of the string after ].
- */
- function splitMainHandler(handler) {
- let matches = handler.match(/^([^.]+)$|^([^.]+)\.(.+)$/);
- if (matches && matches.length == 4) {
- let index = matches[2];
- let main = matches[3] || matches[1];
- return [index, main]
- } else return undefined
- }
+/**
+ * Splits handler into module name and path to main.
+ * If the string contains no '.', return [ undefined, the string ].
+ * If the string contains one or more '.', return [ string up to first period,
rest of the string after ].
+ */
+function splitMainHandler(handler) {
+ let matches = handler.match(/^([^.]+)$|^([^.]+)\.(.+)$/);
+ if (matches && matches.length == 4) {
+ let index = matches[2];
+ let main = matches[3] || matches[1];
+ return [index, main]
+ } else return undefined
+}
+function assertMainIsFunction(userScriptMain, main) {
+ if (typeof userScriptMain !== 'function') {
+ throw "Action entrypoint '" + main + "' is not a function.";
+ }
}
module.exports = NodeActionRunner;
diff --git a/core/nodejsActionBase/src/service.js
b/core/nodejsActionBase/src/service.js
index e6beae6..11ffeb7 100644
--- a/core/nodejsActionBase/src/service.js
+++ b/core/nodejsActionBase/src/service.js
@@ -15,23 +15,22 @@
* limitations under the License.
*/
-
-var NodeActionRunner = require('../runner');
+const NodeActionRunner = require('../runner');
function NodeActionService(config) {
- var Status = {
+ const Status = {
ready: 'ready',
starting: 'starting',
running: 'running',
stopped: 'stopped',
};
- // TODO: save the entire configuration for use by any of the route handlers
- var status = Status.ready;
- var ignoreRunStatus = config.allowConcurrent === undefined ? false :
config.allowConcurrent.toLowerCase() === 'true';
- var server = undefined;
- var userCodeRunner = undefined;
+ const ignoreRunStatus = config.allowConcurrent === undefined ? false :
config.allowConcurrent.toLowerCase() === 'true';
+
+ let status = Status.ready;
+ let server = undefined;
+ let userCodeRunner = undefined;
function setStatus(newStatus) {
if (status !== Status.stopped) {
@@ -84,34 +83,32 @@ function NodeActionService(config) {
* req.body = { main: String, code: String, binary: Boolean }
*/
this.initCode = function initCode(req) {
-
if (status === Status.ready && userCodeRunner === undefined) {
-
setStatus(Status.starting);
- var body = req.body || {};
- var message = body.value || {};
+ let body = req.body || {};
+ let message = body.value || {};
if (message.main && message.code && typeof message.main ===
'string' && typeof message.code === 'string') {
- return doInit(message).then(function(result) {
+ return doInit(message).then(_ => {
setStatus(Status.ready);
return responseMessage(200, { OK: true });
- }).catch(function(error) {
+ }).catch(error => {
setStatus(Status.stopped);
- var errStr = 'Initialization has failed due to: ' +
error.stack ? String(error.stack) : error;
+ let errStr = `Initialization has failed due to:
${error.stack ? String(error.stack) : error}`;
return Promise.reject(errorMessage(502, errStr));
});
} else {
setStatus(Status.ready);
- var msg = 'Missing main/no code to execute.';
+ let msg = 'Missing main/no code to execute.';
return Promise.reject(errorMessage(403, msg));
}
} else if (userCodeRunner !== undefined) {
- var msg = 'Cannot initialize the action more than once.';
+ let msg = 'Cannot initialize the action more than once.';
console.error('Internal system error:', msg);
return Promise.reject(errorMessage(403, msg));
} else {
- var msg = 'System not ready, status is ' + status + '.';
+ let msg = `System not ready, status is ${status}.`;
console.error('Internal system error:', msg);
return Promise.reject(errorMessage(403, msg));
}
@@ -126,12 +123,22 @@ function NodeActionService(config) {
* req.body = { value: Object, meta { activationId : int } }
*/
this.runCode = function runCode(req) {
- if (status === Status.ready) {
+ if (status === Status.ready && userCodeRunner !== undefined) {
if (!ignoreRunStatus) {
setStatus(Status.running);
}
- return doRun(req).then(function(result) {
+ // these are defensive checks against the expected interface
invariants
+ let msg = req && req.body || {};
+ if (msg.value === null || msg.value === undefined) {
+ msg.value = {};
+ } else if (typeof msg.value !== 'object') {
+ let errStr = `Internal system error: the argument must be a
dictionary but has type '${typeof msg.value}'.`;
+ console.error('Internal system error:', errStr);
+ return Promise.reject(errorMessage(403, errStr));
+ }
+
+ return doRun(msg).then(result => {
if (!ignoreRunStatus) {
setStatus(Status.ready);
}
@@ -140,13 +147,13 @@ function NodeActionService(config) {
} else {
return responseMessage(200, result);
}
- }).catch(function(error) {
- var msg = 'An error has occurred: ' + error;
+ }).catch(error => {
+ let msg = `An error has occurred: ${error}`;
setStatus(Status.stopped);
return Promise.reject(errorMessage(502, msg));
});
} else {
- var msg = 'System not ready, status is ' + status + '.';
+ let msg = userCodeRunner ? `System not ready, status is
${status}.` : 'System not initialized.';
console.error('Internal system error:', msg);
return Promise.reject(errorMessage(403, msg));
}
@@ -155,43 +162,41 @@ function NodeActionService(config) {
function doInit(message) {
userCodeRunner = new NodeActionRunner();
- return userCodeRunner.init(message).then(function(result) {
- // 'true' has no particular meaning here. The fact that the promise
- // is resolved successfully in itself carries the intended message
- // that initialization succeeded.
- return true;
- }).catch(function(error) {
- // emit error to activation log then flush the logs as this
- // is the end of the activation
- console.error('Error during initialization:', error);
- writeMarkers();
- return Promise.reject(error);
- });
+ return userCodeRunner
+ .init(message)
+ // returning 'true' has no particular meaning here. The fact that
the promise resolved
+ // successfully in itself carries the intended message that
initialization succeeded.
+ .then(_ => true)
+ // emit error to activation log then flush the logs as this is the
end of the activation
+ .catch(error => {
+ console.error('Error during initialization:', error);
+ writeMarkers();
+ return Promise.reject(error);
+ });
}
- function doRun(req) {
- var msg = req && req.body || {};
+ function doRun(msg) {
// Move per-activation keys to process env. vars with __OW_ (reserved)
prefix
- Object.keys(msg).forEach(
- function(k) {
- if (typeof msg[k] === 'string' && k !== 'value'){
- var envVariable = '__OW_' + k.toUpperCase();
- process.env['__OW_' + k.toUpperCase()] = msg[k];
- }
- }
- );
-
- return userCodeRunner.run(msg.value).then(function(result) {
- if (typeof result !== 'object') {
- console.error('Result must be of type object but has type "' +
typeof result + '":', result);
+ Object.keys(msg).forEach(k => {
+ if (typeof msg[k] === 'string' && k !== 'value') {
+ let envVariable = '__OW_' + k.toUpperCase();
+ process.env[envVariable] = msg[k];
}
- writeMarkers();
- return result;
- }).catch(function(error) {
- console.error(error);
- writeMarkers();
- return Promise.reject(error);
});
+
+ return userCodeRunner
+ .run(msg.value)
+ .then(result => {
+ if (typeof result !== 'object') {
+ console.error(`Result must be of type object but has type
"${typeof result}":`, result);
+ }
+ writeMarkers();
+ return result;
+ }).catch(error => {
+ console.error(error);
+ writeMarkers();
+ return Promise.reject(error);
+ });
}
function writeMarkers() {
@@ -200,8 +205,6 @@ function NodeActionService(config) {
}
}
-NodeActionService.getService = function(config) {
- return new NodeActionService(config);
-};
+NodeActionService.getService = config => new NodeActionService(config);
module.exports = NodeActionService;
