You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by dg...@apache.org on 2018/07/09 15:48:11 UTC
[incubator-openwhisk-runtime-nodejs] branch master updated:
Tightens logic and reports more meanigful error message when attempting to
initialize more than once. (#71)
This is an automated email from the ASF dual-hosted git repository.
dgrove 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 184f130 Tightens logic and reports more meanigful error message when attempting to initialize more than once. (#71)
184f130 is described below
commit 184f130c3e1d7c7ddda9fd3b0b6e677e262e9bfd
Author: rodric rabbah <ro...@gmail.com>
AuthorDate: Mon Jul 9 11:48:08 2018 -0400
Tightens logic and reports more meanigful error message when attempting to initialize more than once. (#71)
* Adds a check in the init-handler to report better error if initialization is attempted more than once.
Also:
- Moved state setting out of doInit so that all state setting is localized to the route handlers.
- Moved console.error messages around so that they appear before the sentinel on init and run errors.
- Adjust tests for upstream changes.
---
core/nodejs6Action/CHANGELOG.md | 5 +
core/nodejs8Action/CHANGELOG.md | 5 +
core/nodejsActionBase/src/service.js | 39 ++++---
.../NodeJsActionContainerTests.scala | 126 ++++++++++++---------
4 files changed, 104 insertions(+), 71 deletions(-)
diff --git a/core/nodejs6Action/CHANGELOG.md b/core/nodejs6Action/CHANGELOG.md
index 279d777..1bfb451 100644
--- a/core/nodejs6Action/CHANGELOG.md
+++ b/core/nodejs6Action/CHANGELOG.md
@@ -19,6 +19,11 @@
# NodeJS 6 OpenWhisk Runtime Container
+## 1.9.3
+Changes:
+ - Disallow re-initialization.
+ - Fix bug where some log messages appear after the log maker.
+
## 1.9.2
Change: Update Node.js
diff --git a/core/nodejs8Action/CHANGELOG.md b/core/nodejs8Action/CHANGELOG.md
index 8533ef1..5dac5ef 100644
--- a/core/nodejs8Action/CHANGELOG.md
+++ b/core/nodejs8Action/CHANGELOG.md
@@ -19,6 +19,11 @@
# NodeJS 8 OpenWhisk Runtime Container
+## 1.6.3
+Changes:
+ - Disallow re-initialization.
+ - Fix bug where some log messages appear after the log maker.
+
## 1.6.2
Change: Update Node.js
diff --git a/core/nodejsActionBase/src/service.js b/core/nodejsActionBase/src/service.js
index 536aad4..e1318ee 100644
--- a/core/nodejsActionBase/src/service.js
+++ b/core/nodejsActionBase/src/service.js
@@ -72,7 +72,7 @@ function NodeActionService(config) {
* req.body = { main: String, code: String, binary: Boolean }
*/
this.initCode = function initCode(req) {
- if (status === Status.ready) {
+ if (status === Status.ready && userCodeRunner === undefined) {
setStatus(Status.starting);
var body = req.body || {};
@@ -80,19 +80,25 @@ function NodeActionService(config) {
if (message.main && message.code && typeof message.main === 'string' && typeof message.code === 'string') {
return doInit(message).then(function (result) {
+ setStatus(Status.ready);
return responseMessage(200, { OK: true });
}).catch(function (error) {
- // this writes to the activation logs visible to the user
- console.error('Error during initialization:', error);
var errStr = error.stack ? String(error.stack) : error;
+ setStatus(Status.stopped);
return Promise.reject(errorMessage(502, "Initialization has failed due to: " + errStr));
});
} else {
setStatus(Status.ready);
- return Promise.reject(errorMessage(500, "Missing main/no code to execute."));
+ return Promise.reject(errorMessage(403, "Missing main/no code to execute."));
}
+ } else if (userCodeRunner !== undefined) {
+ var msg = "Cannot initialize the action more than once.";
+ console.error("Internal system error:", msg);
+ return Promise.reject(errorMessage(403, msg));
} else {
- return Promise.reject(errorMessage(502, "Internal system error: system not ready, status: " + status));
+ var msg = "System not ready, status is " + status + ".";
+ console.error("Internal system error:", msg);
+ return Promise.reject(errorMessage(403, msg));
}
};
@@ -112,19 +118,18 @@ function NodeActionService(config) {
setStatus(Status.ready);
if (typeof result !== "object") {
- console.error('Result must be of type object but has type "' + typeof result + '":', result);
return errorMessage(502, "The action did not return a dictionary.");
} else {
return responseMessage(200, result);
}
}).catch(function (error) {
- setStatus(Status.ready);
-
- return Promise.reject(errorMessage(500, "An error has occurred: " + error));
+ setStatus(Status.stopped);
+ return Promise.reject(errorMessage(502, "An error has occurred: " + error));
});
} else {
- console.log('[runCode]', 'cannot schedule runCode due to status', status);
- return Promise.reject(errorMessage(500, "Internal system error: container not ready, status: " + status));
+ var msg = "System not ready, status is " + status + ".";
+ console.error("Internal system error:", msg);
+ return Promise.reject(errorMessage(403, msg));
}
};
@@ -132,14 +137,15 @@ function NodeActionService(config) {
userCodeRunner = new NodeActionRunner();
return userCodeRunner.init(message).then(function (result) {
- setStatus(Status.ready);
// '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();
- setStatus(Status.stopped);
return Promise.reject(error);
});
}
@@ -152,9 +158,12 @@ function NodeActionService(config) {
process.env['__OW_' + p.toUpperCase()] = msg[p];
});
- return userCodeRunner.run(msg.value).then(function(response) {
+ 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);
+ }
writeMarkers();
- return response;
+ return result;
}).catch(function (error) {
console.error(error);
writeMarkers();
diff --git a/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala b/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
index 8898e22..2cb4906 100644
--- a/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
+++ b/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
@@ -25,7 +25,7 @@ import spray.json._
abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with WskActorSystem {
- lazy val nodejsContainerImageName = "nodejs6action"
+ val nodejsContainerImageName: String
override def withActionContainer(env: Map[String, String] = Map.empty)(code: ActionContainer => Unit) = {
withContainer(nodejsContainerImageName, env)(code)
@@ -35,52 +35,80 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws
behavior of nodejsContainerImageName
- testNotReturningJson("""
+ override val testNoSourceOrExec = {
+ TestConfig("")
+ }
+
+ override val testNotReturningJson = {
+ TestConfig(
+ """
|function main(args) {
| return "not a json object"
|}
- """.stripMargin)
+ """.stripMargin,
+ enforceEmptyErrorStream = false)
+ }
- testEcho(Seq {
- (
- "node",
- """
- |function main(args) {
- | console.log('hello stdout')
- | console.error('hello stderr')
- | return args
- |}
- """.stripMargin)
- })
-
- testUnicode(Seq {
- (
- "node",
- """
- |function main(args) {
- | var str = args.delimiter + " ☃ " + args.delimiter;
- | console.log(str);
- | return { "winter": str };
- |}
- """.stripMargin.trim)
- })
-
- testEnv(Seq {
- (
- "node",
+ override val testEcho = {
+ TestConfig("""
+ |function main(args) {
+ | console.log('hello stdout')
+ | console.error('hello stderr')
+ | return args
+ |}
+ """.stripMargin)
+ }
+
+ override val testUnicode = {
+ TestConfig("""
+ |function main(args) {
+ | var str = args.delimiter + " ☃ " + args.delimiter;
+ | console.log(str);
+ | return { "winter": str };
+ |}
+ """.stripMargin.trim)
+ }
+
+ override val testEnv = {
+ TestConfig("""
+ |function main(args) {
+ | return {
+ | "api_host": process.env['__OW_API_HOST'],
+ | "api_key": process.env['__OW_API_KEY'],
+ | "namespace": process.env['__OW_NAMESPACE'],
+ | "action_name": process.env['__OW_ACTION_NAME'],
+ | "activation_id": process.env['__OW_ACTIVATION_ID'],
+ | "deadline": process.env['__OW_DEADLINE']
+ | }
+ |}
+ """.stripMargin.trim)
+ }
+
+ override val testInitCannotBeCalledMoreThanOnce = {
+ TestConfig("""
+ |function main(args) {
+ | return args
+ |}
+ """.stripMargin)
+ }
+
+ override val testEntryPointOtherThanMain = {
+ TestConfig(
"""
- |function main(args) {
- | return {
- | "api_host": process.env['__OW_API_HOST'],
- | "api_key": process.env['__OW_API_KEY'],
- | "namespace": process.env['__OW_NAMESPACE'],
- | "action_name": process.env['__OW_ACTION_NAME'],
- | "activation_id": process.env['__OW_ACTIVATION_ID'],
- | "deadline": process.env['__OW_DEADLINE']
- | }
- |}
- """.stripMargin.trim)
- })
+ | function niam(args) {
+ | return args;
+ | }
+ """.stripMargin,
+ main = "niam")
+ }
+
+ override val testLargeInput = {
+ TestConfig("""
+ |function main(args) {
+ | return args
+ |}
+ """.stripMargin)
+ }
it should "fail to initialize with bad code" in {
val (out, err) = withNodeJsContainer { c =>
@@ -490,20 +518,6 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws
})
}
- it should "support actions using non-default entry point" in {
- val (out, err) = withNodeJsContainer { c =>
- val code = """
- | function niam(args) {
- | return { result: "it works" };
- | }
- """.stripMargin
-
- c.init(initPayload(code, main = "niam"))._1 should be(200)
- val (runCode, runRes) = c.run(runPayload(JsObject()))
- runRes.get.fields.get("result") shouldBe Some(JsString("it works"))
- }
- }
-
it should "support zipped actions using non-default entry point" in {
val srcs = Seq(
Seq("package.json") -> examplePackageDotJson,