[GitHub] rabbah closed pull request #3897: Do not abort conductor action execution when component action is invalid, not found, not accessible or when composition is too long

[GitBox <gi...@apache.org>]

rabbah closed pull request #3897: Do not abort conductor action execution when component action is invalid, not found, not accessible or when composition is too long
URL: https://github.com/apache/incubator-openwhisk/pull/3897
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/core/controller/src/main/scala/whisk/core/controller/actions/PrimitiveActions.scala b/core/controller/src/main/scala/whisk/core/controller/actions/PrimitiveActions.scala
index e6dc3cb3c2..c2ab52dff5 100644
--- a/core/controller/src/main/scala/whisk/core/controller/actions/PrimitiveActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/actions/PrimitiveActions.scala
@@ -28,6 +28,7 @@ import whisk.core.connector.ActivationMessage
 import whisk.core.controller.WhiskServices
 import whisk.core.database.{ActivationStore, NoDocumentException}
 import whisk.core.entitlement.{Resource, _}
+import whisk.core.entity.ActivationResponse.ERROR_FIELD
 import whisk.core.entity._
 import whisk.core.entity.size.SizeInt
 import whisk.core.entity.types.EntityStore
@@ -354,10 +355,17 @@ protected[actions] trait PrimitiveActions {
                   tryInvokeNext(user, fqn, params, session)
 
                 case Some(_) => // composition is too long
-                  Future.successful(ActivationResponse.applicationError(compositionIsTooLong))
+                  invokeConductor(
+                    user,
+                    payload = Some(JsObject(ERROR_FIELD -> JsString(compositionIsTooLong))),
+                    session = session)
 
                 case None => // parsing failure
-                  Future.successful(ActivationResponse.applicationError(compositionComponentInvalid(next)))
+                  invokeConductor(
+                    user,
+                    payload = Some(JsObject(ERROR_FIELD -> JsString(compositionComponentInvalid(next)))),
+                    session = session)
+
               }
           }
       }
@@ -388,16 +396,22 @@ protected[actions] trait PrimitiveActions {
               // successful resolution
               invokeComponent(user, action = next, payload = params, session)
           }
-          .recover {
+          .recoverWith {
             case _ =>
               // resolution failure
-              ActivationResponse.applicationError(compositionComponentNotFound(fqn.asString))
+              invokeConductor(
+                user,
+                payload = Some(JsObject(ERROR_FIELD -> JsString(compositionComponentNotFound(fqn.asString)))),
+                session = session)
           }
       }
-      .recover {
+      .recoverWith {
         case _ =>
           // failed entitlement check
-          ActivationResponse.applicationError(compositionComponentNotAccessible(fqn.asString))
+          invokeConductor(
+            user,
+            payload = Some(JsObject(ERROR_FIELD -> JsString(compositionComponentNotAccessible(fqn.asString)))),
+            session = session)
       }
   }
 
diff --git a/docs/conductors.md b/docs/conductors.md
index c36f25e6dc..490d18894a 100644
--- a/docs/conductors.md
+++ b/docs/conductors.md
@@ -245,19 +245,18 @@ If a secondary activation returns an error dictionary, the conductor action invo
 
 In a continuation dictionary, the _params_ field is optional and its value if present should be a dictionary. The _action_ field is optional and its value if present should be a string. The _state_ field is optional and its value if present should be a dictionary. If the value _v_ of the _params_ field is not a dictionary it is automatically boxed into dictionary `{ value: v }`. If the value _v_ of the _state_ field is not a dictionary it is automatically boxed into dictionary `{ state: v }`.
 
-If the _action_ field is defined in the output of the conductor action, the runtime attempts to convert its value (irrespective of its type) into the fully qualified name of an action and invoke this action (using the default namespace if necessary). There are four failure modes:
+If the _action_ field is defined in the output of the conductor action, the runtime attempts to convert its value (irrespective of its type) into the fully qualified name of an action and invoke this action (using the default namespace if necessary). The action name should be a fully qualified name, which is of the form `/namespace/package-name/action-name` or `/namespace/action-name`. Failure to specify a fully qualified name may result in ambiguity or even a parsing error. There are four failure modes:
 
 - parsing failure,
 - resolution failure,
 - entitlement check failure,
 - internal error (invocation failure or timeout).
 
-In any of the first three failure scenarios, the conductor action invocation ends with an _application error_ status code and an error message describing the reason for the failure. In the latter, the status code is _internal error_.
-The action name should be a fully qualified name, which is of the form `/namespace/package-name/action-name` or `/namespace/action-name`. Failure to specify a fully qualified name may result in ambiguity or even a parsing error that terminates the activation.
+In the last failure scenario, the conductor action invocation ends with an _internal error_ status code and an error message describing the reason for the failure.
 
-If there is no error, _action_ is invoked on the _params_ dictionary if specified (auto boxed if necessary) or if not on the empty dictionary.
+If there is no error, _action_ is invoked on the _params_ dictionary if specified (auto boxed if necessary) or if not on the empty dictionary. Upon completion of this invocation, the conductor action is activated again. The input dictionary for this activation is a combination of the output dictionary for the component action and the value of the _state_ field from the prior secondary conductor activation. Fields of the _state_ dictionary (auto boxed if necessary) are added to the output dictionary of the component activation, overriding values of existing fields if necessary.
 
-Upon completion of this invocation, the conductor action is activated again. The input dictionary for this activation is a combination of the output dictionary for the component action and the value of the _state_ field from the prior secondary conductor activation. Fields of the _state_ dictionary (auto boxed if necessary) are added to the output dictionary of the component activation, overriding values of existing fields if necessary.
+In the first three failures scenarios, the conductor action is activated again. The input dictionary for this activation is a combination of an error object with an error message describing the reason for the failure and the value of the _state_ field from the prior secondary conductor activation (as in the previous scenario).
 
 On the other hand, if the _action_ field is not defined in the output of the conductor action, the conductor action invocation ends. The output for the conductor action invocation is either the value of the _params_ field in the output dictionary of the last secondary activation if defined (auto boxed if necessary) or if absent the complete output dictionary.
 
@@ -267,4 +266,6 @@ There are limits on the number of component action activations and secondary con
 
 The maximum number _n_ of permitted component activations is equal to the maximum number of components in a sequence action. It is configured via the same configuration parameter. The maximum number of secondary conductor activations is _2n+1_.
 
-If either of these limits is exceeded, the conductor action invocation ends with an _application error_ status code and an error message describing the reason for the failure.
+If the maximum number of permitted component activations is exceeded the conductor action is activated again. The input dictionary for this activation is a combination of an error object with an error message describing the reason for the failure and the value of the _state_ field from the prior secondary conductor activation.
+
+If the maximum number of secondary conductor activations is exceeded, the conductor action invocation ends with an _application error_ status code and an error message describing the reason for the failure.
diff --git a/tests/src/test/scala/system/basic/WskConductorTests.scala b/tests/src/test/scala/system/basic/WskConductorTests.scala
index 7302e85d61..f708b4f5c7 100644
--- a/tests/src/test/scala/system/basic/WskConductorTests.scala
+++ b/tests/src/test/scala/system/basic/WskConductorTests.scala
@@ -105,7 +105,7 @@ class WskConductorTests extends TestHelpers with WskTestHelpers with JsHelpers w
         activation.response.status shouldBe "application error"
         activation.response.result.get.fields.get("error") shouldBe Some(
           JsString(compositionComponentInvalid(JsString(invalid))))
-        checkConductorLogsAndAnnotations(activation, 1) // echo
+        checkConductorLogsAndAnnotations(activation, 2) // echo
       }
 
       // an undefined action
@@ -116,7 +116,7 @@ class WskConductorTests extends TestHelpers with WskTestHelpers with JsHelpers w
         activation.response.status shouldBe "application error"
         activation.response.result.get.fields.get("error") shouldBe Some(
           JsString(compositionComponentNotFound(s"$namespace/$missing")))
-        checkConductorLogsAndAnnotations(activation, 1) // echo
+        checkConductorLogsAndAnnotations(activation, 2) // echo
       }
   }
 
diff --git a/tests/src/test/scala/whisk/core/controller/test/ConductorsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/ConductorsApiTests.scala
index a95c560eb3..a93436b4f6 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ConductorsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ConductorsApiTests.scala
@@ -117,7 +117,7 @@ class ConductorsApiTests extends ControllerTestCommon with WhiskActionsApi {
       response.fields("response").asJsObject.fields("status") shouldBe "application error".toJson
       response.fields("response").asJsObject.fields("result") shouldBe JsObject(
         "error" -> compositionComponentInvalid(invalid.toJson).toJson)
-      response.fields("logs").convertTo[JsArray].elements.size shouldBe 1
+      response.fields("logs").convertTo[JsArray].elements.size shouldBe 2
     }
 
     // an undefined action
@@ -128,7 +128,7 @@ class ConductorsApiTests extends ControllerTestCommon with WhiskActionsApi {
       response.fields("response").asJsObject.fields("status") shouldBe "application error".toJson
       response.fields("response").asJsObject.fields("result") shouldBe JsObject(
         "error" -> compositionComponentNotFound(s"$namespace/$missing").toJson)
-      response.fields("logs").convertTo[JsArray].elements.size shouldBe 1
+      response.fields("logs").convertTo[JsArray].elements.size shouldBe 2
     }
   }
 
@@ -180,7 +180,7 @@ class ConductorsApiTests extends ControllerTestCommon with WhiskActionsApi {
       response.fields("response").asJsObject.fields("status") shouldBe "application error".toJson
       response.fields("response").asJsObject.fields("result") shouldBe JsObject(
         "error" -> compositionComponentNotAccessible(forbidden.drop(1)).toJson)
-      response.fields("logs").convertTo[JsArray].elements.size shouldBe 1
+      response.fields("logs").convertTo[JsArray].elements.size shouldBe 2
     }
 
     // dynamically invoke step action twice, forwarding state


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services