fix(engine): return null on function invocation

* if a function invocation returns an error then log the error and return null instead * following the DMN spec, a function invocation should return null instead of propagating an error
camunda · Aug 18, 2021 · c98c118 · c98c118
1 parent c4fbcf1
commit c98c118
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 43 deletions.
diff --git a/src/main/scala/org/camunda/feel/impl/builtin/StringBuiltinFunctions.scala b/src/main/scala/org/camunda/feel/impl/builtin/StringBuiltinFunctions.scala
@@ -1,9 +1,9 @@
 package org.camunda.feel.impl.builtin
 
-import java.util.regex.Pattern
 import org.camunda.feel.impl.builtin.BuiltinFunction.builtinFunction
 import org.camunda.feel.syntaxtree._
 
+import java.util.regex.Pattern
 import scala.util.Try
 
 object StringBuiltinFunctions {
@@ -96,11 +96,15 @@ object StringBuiltinFunctions {
     params = List("input", "pattern", "replacement"),
     invoke = {
       case List(ValString(input), ValString(pattern), ValString(replacement)) =>
-        Try (Pattern.compile(pattern))
-          .map {pattern =>
+        Try(Pattern.compile(pattern))
+          .map { pattern =>
             val m = pattern.matcher(input)
             ValString(m.replaceAll(replacement))
-          }.getOrElse(ValNull)
+          }
+          .recover { _ =>
+            ValError(s"Invalid pattern '$pattern'")
+          }
+          .get
     }
   )
 
@@ -111,11 +115,15 @@ object StringBuiltinFunctions {
                 ValString(pattern),
                 ValString(replacement),
                 ValString(flags)) =>
-        Try (Pattern.compile(pattern, patternFlags(flags)))
-          .map {pattern =>
+        Try(Pattern.compile(pattern, patternFlags(flags)))
+          .map { pattern =>
             val m = pattern.matcher(input)
             ValString(m.replaceAll(replacement))
-          }.getOrElse(ValNull)
+          }
+          .recover { _ =>
+            ValError(s"Invalid pattern '$pattern'")
+          }
+          .get
     }
   )
 
@@ -164,7 +172,11 @@ object StringBuiltinFunctions {
           .map { pattern =>
             val m = pattern.matcher(input)
             ValBoolean(m.find)
-          }.getOrElse(ValNull)
+          }
+          .recover { _ =>
+            ValError(s"Invalid pattern '$pattern'")
+          }
+          .get
       }
     }
   )
@@ -177,7 +189,11 @@ object StringBuiltinFunctions {
           .map { pattern =>
             val m = pattern.matcher(input)
             ValBoolean(m.find)
-          }.getOrElse(ValNull)
+          }
+          .recover { _ =>
+            ValError(s"Invalid pattern '$pattern'")
+          }
+          .get
     }
   )
 
@@ -189,7 +205,11 @@ object StringBuiltinFunctions {
           .map { pattern =>
             val r = pattern.split(string, -1)
             ValList(r.map(ValString).toList)
-          }.getOrElse(ValNull)
+          }
+          .recover { _ =>
+            ValError(s"Invalid pattern for delimiter '$delimiter'")
+          }
+          .get
     }
   )
 

diff --git a/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala b/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala
@@ -777,8 +777,14 @@ class FeelInterpreter {
       }
     }
 
-    val result = function.invoke(paramList)
-    context.valueMapper.toVal(result)
+    function.invoke(paramList) match {
+      case ValError(failure) => {
+        // TODO (saig0): customize error handling (#260)
+        logger.warn(s"Failed to invoke function: $failure")
+        ValNull
+      }
+      case result => context.valueMapper.toVal(result)
+    }
   }
 
   private def findFunction(ctx: EvalContext,

diff --git a/src/test/scala/org/camunda/feel/impl/builtin/BuiltinNumberFunctionTest.scala b/src/test/scala/org/camunda/feel/impl/builtin/BuiltinNumberFunctionTest.scala
@@ -47,9 +47,7 @@ class BuiltinNumberFunctionsTest
     eval(""" decimal(5.5, 0, "HALF_UP") """) should be(ValNumber(6))
     eval(""" decimal(5.5, 0, "HALF_DOWN") """) should be(ValNumber(5))
     eval(""" decimal(5.5, 0, "HALF_EVEN") """) should be(ValNumber(6))
-    eval(""" decimal(5.5, 0, "UNNECESSARY") """) should be(
-      ValError(
-        "Failed to apply rounding mode 'UNNECESSARY': Rounding necessary"))
+    eval(""" decimal(5.5, 0, "UNNECESSARY") """) should be(ValNull)
 
     eval(""" decimal(2.5, 0, "UP") """) should be(ValNumber(3))
     eval(""" decimal(2.5, 0, "DOWN") """) should be(ValNumber(2))
@@ -58,9 +56,7 @@ class BuiltinNumberFunctionsTest
     eval(""" decimal(2.5, 0, "HALF_UP") """) should be(ValNumber(3))
     eval(""" decimal(2.5, 0, "HALF_DOWN") """) should be(ValNumber(2))
     eval(""" decimal(2.5, 0, "HALF_EVEN") """) should be(ValNumber(2))
-    eval(""" decimal(2.5, 0, "UNNECESSARY") """) should be(
-      ValError(
-        "Failed to apply rounding mode 'UNNECESSARY': Rounding necessary"))
+    eval(""" decimal(2.5, 0, "UNNECESSARY") """) should be(ValNull)
 
     eval(""" decimal(1.6, 0, "UP") """) should be(ValNumber(2))
     eval(""" decimal(1.6, 0, "DOWN") """) should be(ValNumber(1))
@@ -69,9 +65,7 @@ class BuiltinNumberFunctionsTest
     eval(""" decimal(1.6, 0, "HALF_UP") """) should be(ValNumber(2))
     eval(""" decimal(1.6, 0, "HALF_DOWN") """) should be(ValNumber(2))
     eval(""" decimal(1.6, 0, "HALF_EVEN") """) should be(ValNumber(2))
-    eval(""" decimal(1.6, 0, "UNNECESSARY") """) should be(
-      ValError(
-        "Failed to apply rounding mode 'UNNECESSARY': Rounding necessary"))
+    eval(""" decimal(1.6, 0, "UNNECESSARY") """) should be(ValNull)
 
     eval(""" decimal(1.1, 0, "UP") """) should be(ValNumber(2))
     eval(""" decimal(1.1, 0, "DOWN") """) should be(ValNumber(1))
@@ -80,9 +74,7 @@ class BuiltinNumberFunctionsTest
     eval(""" decimal(1.1, 0, "HALF_UP") """) should be(ValNumber(1))
     eval(""" decimal(1.1, 0, "HALF_DOWN") """) should be(ValNumber(1))
     eval(""" decimal(1.1, 0, "HALF_EVEN") """) should be(ValNumber(1))
-    eval(""" decimal(1.1, 0, "UNNECESSARY") """) should be(
-      ValError(
-        "Failed to apply rounding mode 'UNNECESSARY': Rounding necessary"))
+    eval(""" decimal(1.1, 0, "UNNECESSARY") """) should be(ValNull)
 
     eval(""" decimal(1.0, 0, "UP") """) should be(ValNumber(1))
     eval(""" decimal(1.0, 0, "DOWN") """) should be(ValNumber(1))
@@ -109,9 +101,7 @@ class BuiltinNumberFunctionsTest
     eval(""" decimal(-1.1, 0, "HALF_UP") """) should be(ValNumber(-1))
     eval(""" decimal(-1.1, 0, "HALF_DOWN") """) should be(ValNumber(-1))
     eval(""" decimal(-1.1, 0, "HALF_EVEN") """) should be(ValNumber(-1))
-    eval(""" decimal(-1.1, 0, "UNNECESSARY") """) should be(
-      ValError(
-        "Failed to apply rounding mode 'UNNECESSARY': Rounding necessary"))
+    eval(""" decimal(-1.1, 0, "UNNECESSARY") """) should be(ValNull)
 
     eval(""" decimal(-1.6, 0, "UP") """) should be(ValNumber(-2))
     eval(""" decimal(-1.6, 0, "DOWN") """) should be(ValNumber(-1))
@@ -120,9 +110,7 @@ class BuiltinNumberFunctionsTest
     eval(""" decimal(-1.6, 0, "HALF_UP") """) should be(ValNumber(-2))
     eval(""" decimal(-1.6, 0, "HALF_DOWN") """) should be(ValNumber(-2))
     eval(""" decimal(-1.6, 0, "HALF_EVEN") """) should be(ValNumber(-2))
-    eval(""" decimal(-1.6, 0, "UNNECESSARY") """) should be(
-      ValError(
-        "Failed to apply rounding mode 'UNNECESSARY': Rounding necessary"))
+    eval(""" decimal(-1.6, 0, "UNNECESSARY") """) should be(ValNull)
 
     eval(""" decimal(-2.5, 0, "UP") """) should be(ValNumber(-3))
     eval(""" decimal(-2.5, 0, "DOWN") """) should be(ValNumber(-2))
@@ -131,9 +119,7 @@ class BuiltinNumberFunctionsTest
     eval(""" decimal(-2.5, 0, "HALF_UP") """) should be(ValNumber(-3))
     eval(""" decimal(-2.5, 0, "HALF_DOWN") """) should be(ValNumber(-2))
     eval(""" decimal(-2.5, 0, "HALF_EVEN") """) should be(ValNumber(-2))
-    eval(""" decimal(-2.5, 0, "UNNECESSARY") """) should be(
-      ValError(
-        "Failed to apply rounding mode 'UNNECESSARY': Rounding necessary"))
+    eval(""" decimal(-2.5, 0, "UNNECESSARY") """) should be(ValNull)
 
     eval(""" decimal(-5.5, 0, "UP") """) should be(ValNumber(-6))
     eval(""" decimal(-5.5, 0, "DOWN") """) should be(ValNumber(-5))
@@ -142,9 +128,7 @@ class BuiltinNumberFunctionsTest
     eval(""" decimal(-5.5, 0, "HALF_UP") """) should be(ValNumber(-6))
     eval(""" decimal(-5.5, 0, "HALF_DOWN") """) should be(ValNumber(-5))
     eval(""" decimal(-5.5, 0, "HALF_EVEN") """) should be(ValNumber(-6))
-    eval(""" decimal(-5.5, 0, "UNNECESSARY") """) should be(
-      ValError(
-        "Failed to apply rounding mode 'UNNECESSARY': Rounding necessary"))
+    eval(""" decimal(-5.5, 0, "UNNECESSARY") """) should be(ValNull)
   }
 
   it should "use the given rounding mode (case-insensitive)" in {
@@ -153,13 +137,8 @@ class BuiltinNumberFunctionsTest
     eval(""" decimal(1.5, 0, "CeiLing") """) should be(ValNumber(2))
   }
 
-  it should "fail if the rounding mode is not valid" in {
-    // invalid rounding mode
-    eval(""" decimal(1.5, 0, "unknown") """) should be(
-      ValError(
-        "Illegal argument 'unknown' for rounding mode. Must be one of: UP, DOWN, CEILING, FLOOR, HALF_UP, HALF_DOWN, HALF_EVEN, UNNECESSARY")
-    )
-
+  it should "return null if the rounding mode is not valid" in {
+    eval(""" decimal(1.5, 0, "unknown") """) should be(ValNull)
   }
 
   "A floor() function" should "return greatest integer <= _" in {