feat(agama): add means to selectively prevent flow crash when a subfl…

…ow crashes (#4436)

* feat: update language grammar and transpiler #4404

* feat: add support to catch flow crashes #4404

* docs: update flows' lifecycle details #4404

agama/transpiler/src/main/antlr4/io/jans/agama/antlr/AuthnFlow.g4

@@ -55,7 +55,7 @@ preassign_catch: variable? WS? '|' WS? short_var WS? EQ WS? ;
 
 variable: short_var | QNAME | DOTEXPR | DOTIDXEXPR ;
 
-flow_call: preassign? FLOWCALL WS ('$' variable | qname) argument* WS? (overrides | NL) ;
+flow_call: (preassign | preassign_catch)? FLOWCALL WS ('$' variable | qname) argument* WS? (overrides | NL) ;
 
 overrides: INDENT OVERRIDE (WS STRING WS STRING)* (WS? NL STRING WS STRING)* WS? NL DEDENT ;
 //I don't get why the NL is needed above

agama/transpiler/src/main/resources/JSGenerator.ftl

@@ -11,7 +11,7 @@ function ${flow.@id}<#recurse flow>
 
 <#macro header>
 (
-<#if .node.configs?size = 0>_p<#else>${.node.configs.short_var}</#if>
+<#if .node.configs?size == 0>_p<#else>${.node.configs.short_var}</#if>
 <#if .node.inputs?size gt 0>
     , ${.node.inputs.short_var?join(", ")}
 </#if>
@@ -32,7 +32,7 @@ let idx = [], _items = []
 <#macro rrf_call>
     <#local hasbool = .node.BOOL?size gt 0>
 
-    <#if .node.variable?size = 0>
+    <#if .node.variable?size == 0>
         _it = {}
     <#else>
         _it = ${.node.variable}
@@ -76,19 +76,34 @@ try {
 </#macro>
 
 <#macro flow_call>
+    <#local catch=.node.preassign_catch?size gt 0>
+
+    <#if catch>
+try {
+    var ${.node.preassign_catch.short_var} = null
+    </#if>
+
     <#if .node.variable?size gt 0>
         _it = ${.node.variable}
     <#else>
         _it = "${.node.qname}"
     </#if>
 _it = _flowCall(_it, _basePath, <@util_url_overrides node=.node.overrides/>, <@util_argslist node=.node />)
-if (_it === undefined) return
-if (_it.bubbleUp) return _it.value 
+
+if (_it === undefined) throw new Error("No Finish instruction was reached")
+
+if (_it.bubbleUp) return _it.value
     <@util_preassign node=.node /> _it.value
+
+    <#if catch>
+} catch (_e) {
+    ${.node.preassign_catch.short_var} = _makeRhinoException(_e)
+}
+    </#if>
 </#macro>
 
 <#macro rfac>
-<#if .node.variable?size = 0>
+<#if .node.variable?size == 0>
     _it = ${.node.STRING}
 <#else>
     _it = ${.node.variable}
@@ -108,7 +123,7 @@ return _finish(_it)
 </#macro>
 
 <#macro loopy>
-    <#if .node.variable?size = 0>
+    <#if .node.variable?size == 0>
 _it = ${.node.UINT}
     <#else>
 _it = ${.node.variable}
@@ -182,7 +197,7 @@ _equals(${.node.simple_expr[0]}, ${.node.simple_expr[1]})
 </#macro>
 
 <#macro boolean_op_expr>
-    <#if .node.AND?size = 0>
+    <#if .node.AND?size == 0>
 ||
     <#else>
 &&
@@ -212,7 +227,7 @@ else {
 
 <#macro util_preassign node>
     <#local var = "" >
-    <#if node.preassign?size = 0>
+    <#if node.preassign?size == 0>
         <#if node.preassign_catch?size gt 0 && node.preassign_catch.variable?size gt 0>
             <#local var = node.preassign_catch.variable >
         </#if>

agama/transpiler/src/main/resources/util.js

@@ -55,9 +55,14 @@ function _flowCall(flowName, basePath, urlOverrides, args) {
     //wrapping with ArrayList is a workaround for a kryo deserialization exception
     p = new Packages.java.util.ArrayList(mapping.values())
     mapping = null
-    let result = f.apply(null, params)
 
-    _scriptUtils.closeSubflow()
+    let result
+    try {
+        result = f.apply(null, params)
+    } finally {
+        _scriptUtils.closeSubflow()
+    }
+
     if (_isNil(result)) return     //return undefined
 
     return { value: result,
@@ -66,6 +71,10 @@ function _flowCall(flowName, basePath, urlOverrides, args) {
 
 }
 
+function _makeRhinoException(e) {
+    return new Packages.org.mozilla.javascript.SubflowRhinoException(e)
+}
+
 function _finish(val) {
 
     let javaish = _javaish(val)

docs/admin/developer/agama/dsl-full.md

@@ -444,7 +444,7 @@ Finish it.nonsense
 **Notes:**
 
 - Any statements found after `Finish` is not reached and thus, not executed
-- If no `Finish` statement is found in a flow's execution, this will degenerate in flow crash 
+- If no `Finish` statement is found in a flow's execution, this will degenerate in flow [crash](./lifecycle.md#about-crashes)
 - When a flow is finished and was used as [subflow](#subflows) (part of the execution of a bigger parent flow), the parent does not terminate. Execution continues at the following instruction that triggered the subflow. More on `Trigger` later
 - Using `data` in the `Finish` directive is an effective way to communicate information to callers (parent flows). If a  flow has no parents, `data` is stored in the authentication server's session of the given user under the key `agamaData`. Contents are serialized to a JSON string previously 
 - A flow cannot be aborted by itself. This can only be achieved through a parent flow. Learn more about aborted flows [here](./flows-lifecycle.md#cancellation)
@@ -712,6 +712,16 @@ Log "subflow returned with success?" outcome.success
 	<tr>
 <td>
 
+```
+outcome | E = Trigger jo.jo.PersonalInfoGathering null false 
+```
+
+</td>
+		<td>Similar to the previous example. If for some reason <code>PersonalInfoGathering</code> crashes, variable <code>E</code> will hold a reference to the error in the form of a <code>java.lang.Exception</code> for further processing. Otherwise <code>E</code> evaluates to <code>null</code>. The variable on the left of the pipe (<code>|</code>) can be omitted if the outcome of the flow will not be inspected</td>
+	</tr>
+	<tr>
+<td>
+
 ```
 userPrefs = { otp: "...", ... }
 Match userPrefs.otp to
@@ -724,7 +734,7 @@ Trigger $flow
 
 </td>
 		<td>Starts a flow whose qualified name is determined at runtime</td>
-	</tr>
+	</tr>	
 </table> 
 
 ### Input parameters

docs/admin/developer/agama/flows-lifecycle.md

@@ -85,20 +85,18 @@ Ideas for a good experience:
 
 In order to override a page, the path to the corresponding template can be easily derived from the URL seen at the browser's address bar when the subflow is `Trigger`ed. Note the page may not necessarily belong directly to the subflow  triggered but probably to another flow lying deep in a chain of `Trigger` invocations. 
 
-As an example suppose you are interested in building a flow A that reuses flow B. You identify a page shown that needs to be overriden. It might happen this page is actually rendered by C - a flow that B in turn reuses. In scenarios like this cancellation still works transparently and developers need not be aware of flows dependencies. In practice, when cancellation occurs at A, it bubbles up to B and then to C, which is the target of this process. 
+As an example suppose you are interested in building a flow A that reuses flow B. You identify a page shown that needs to be overriden. It might happen this page is actually rendered by C - a flow that B in turn reuses. In scenarios like this cancellation still works transparently and developers need not be aware of flows dependencies. In practice, when cancellation occurs at C, it bubbles up to B and then to A, which is the target of this process. 
 
-Note that even flow B (as is) may also be overriding A's templates. Resolution of a template path takes place from the inner to the outer flow, so it occurs this way in the example:
+Note that even flow B (as is) may also be overriding C's templates. Resolution of a template path takes place from the inner to the outer flow, so it occurs this way in the example:
 
-1. `path` is as found in A's `RRF` instruction
+1. `path` is as found in C's `RRF` instruction
 
 1. `path` is looked up on the list provided in B's `Override templates`. If a match is found, `path` is updated accordingly 
 
-1. `path` is looked up on the list provided in C's `Override templates`. If a match is found, `path` is updated accordingly
+1. `path` is looked up on the list provided in A's `Override templates`. If a match is found, `path` is updated accordingly
 
 1. The page referenced by `path` is rendered
 
-When a page POSTs a cancellation as described earlier, the flow to return control to is determined by the path of the template that issued the given POST. Similarly a lookup on B's `Override templates` takes place followed by other on A's. 
-
 ## Timeouts
 
 Authentication flows are normally short-lived. They usually span no more than a few minutes. In Agama, the maximum amount of time an end-user can take to fully complete a flow is driven by the [configuration of the authentication server](../../config-guide/jans-cli/cli-jans-authorization-server.md), specifically the `sessionIdUnauthenticatedUnusedLifetime` property which is measured in seconds. As an example, if this value is 120, any attempt to authenticate taking more than two minutes will throw an error page.

docs/admin/developer/agama/lifecycle.md

@@ -32,9 +32,9 @@ Currently there are no IDE/editor plugins for coding in Agama available. We hope
 
 ### About crashes
 
-As a flow executes things can go wrong for reasons that developers cannot foresee. A database may have crashed, a connection to an external system may have failed, the flow engine may have some bug, etc. When an abnormal situation is presented, a flow simply crashes.
+As a flow executes things can go wrong for reasons that developers cannot foresee. A database may have crashed, a connection to an external system may have failed, the flow itself may have some bug, etc. When an abnormal situation is presented, a flow simply crashes.
 
-If a flow crashes, its parent flows (or flow) if they exist, crash as well. Trying to handle crashes involves a lot of planning and work which is too costly and will unlikely account for the so many things that might fail in a real setting. Thus, coding defensively is not recommended. While in Agama is possible to deal with Java exceptions, that feature should be used sparingly.
+If a flow crashes, its parent flows (or flow) if they exist, crash as well. Trying to handle crashes involves a lot of planning and work which is too costly and will unlikely account for the so many things that might fail in a real setting. Thus, coding defensively is not recommended. While in Agama is possible to deal with Java exceptions (product of `Call`s) and other abnormalities when `Trigger`ing flows, these features should be used sparingly.
 
 ## Creating a flow
 

jans-auth-server/agama/engine/src/main/java/io/jans/agama/engine/service/FlowService.java

@@ -355,6 +355,8 @@ private NativeObject checkJSReturnedValue(Object obj) throws Exception {
             return NativeObject.class.cast(obj);
         } catch (ClassCastException e) {
             if (Undefined.isUndefined(obj)) {
+                //This may only happen for a top-level flow. A subflow that returns undefined
+                //is handled in the generated javascript for a Trigger directive
                 throw new Exception("No Finish instruction was reached");
             } else throw e;
         }

jans-auth-server/agama/engine/src/main/java/org/mozilla/javascript/SubflowRhinoException.java

@@ -0,0 +1,39 @@
+package org.mozilla.javascript;
+
+import java.io.PrintStream;
+import java.io.PrintWriter;
+
+public class SubflowRhinoException extends Exception {
+
+    private String rhinoStackTrace;
+
+    public SubflowRhinoException(Object err) {
+
+        super(err.toString());
+        try {
+            NativeError e = (NativeError) err;
+            rhinoStackTrace = e.getStackDelegated().toString();
+        } catch(ClassCastException ex) {
+            rhinoStackTrace = "Stacktrace not available";
+        }
+
+    }
+
+    @Override
+    public void printStackTrace() {
+        printStackTrace(System.err);
+    }
+
+    @Override
+    public void printStackTrace(PrintStream p) {
+        p.println(getMessage());
+        p.print(rhinoStackTrace);
+    }
+
+    @Override
+    public void printStackTrace(PrintWriter w) {
+        w.println(getMessage());
+        w.print(rhinoStackTrace);
+    }
+
+}

jans-auth-server/agama/inboundID/src/main/java/io/jans/util/NetworkUtils.java

@@ -18,9 +18,14 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 public class NetworkUtils {
+
+    private static Logger LOG = LoggerFactory.getLogger(NetworkUtils.class);
 
     public static HTTPResponse sendGet(String url, MultivaluedMap<String, String> headers,
             MultivaluedMap<String, String> parameters) throws IOException {
@@ -54,6 +59,10 @@ public static Map<String, Object> mapFromGetRequest(String url, MultivaluedMap<S
             throws IOException, ParseException {
 
         HTTPResponse response = sendGet(url, headers, parameters);
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("Request to url {}\nHeaders: {}; Parameters: {}\nResponse (status code {}): {}",
+                    url, headers, parameters, response.getStatusCode(), response.getContent());
+        }
         if (ensureOKStatus) {
             response.ensureStatusCode(Response.Status.OK.getStatusCode());
         }