openwhisk-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dgr...@apache.org
Subject [incubator-openwhisk-runtime-nodejs] branch master updated: Tightens logic and reports more meanigful error message when attempting to initialize more than once. (#71)
Date Mon, 09 Jul 2018 15:48:11 GMT
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 <rodric@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,


Mime
View raw message