merge: #9695

9695: [Backport stable/1.3] Escalate errors in output mapping evaluation r=pihme a=backport-action

# Description
Backport of #9668 to `stable/1.3`.

relates to #9543

Co-authored-by: pihme <pihme@users.noreply.github.com>

engine/src/main/java/io/camunda/zeebe/engine/processing/deployment/model/transformer/VariableMappingTransformer.java

@@ -50,8 +50,8 @@
  * </pre>
  *
  * <p>Output variable mappings differ from input mappings that the result variables needs to be
- * merged with the existing variables if the variable is a JSON object. The merging is done by
- * calling the FEEL function 'put all()' and referencing the variable.
+ * merged with the existing variables if the variable is a JSON object. The merging is done, as a
+ * first draft, by calling the FEEL function 'put all()' and referencing the variable.
  *
  * <pre>
  *   {
@@ -67,6 +67,27 @@
  *       })
  *   }
  * </pre>
+ *
+ * <p>There is one edge case, though, which was revealed in #9543: At the time of this writing the
+ * two branches of the if statement behave differently with respect to evaluation errors. The first
+ * branch (if the target variable is null) will propagate any error that occurs to the result of the
+ * evaluation. However, the second branch uses the put all(...) method which will transform any
+ * error in one of its parameters into a null as result. Therefore, the two branches will produce
+ * inconsistent results if there are errors in the expression.
+ *
+ * <p>To account for this the expression, was amended to
+ *
+ * <pre>
+ *  if (targetVar != null and is defined(outputMappingResult))
+ *    then put all(targetVar,outputMappingResult)
+ *    else outputMappingResult
+ * </pre>
+ *
+ * <p>This is a workaround which hopefully can be removed in the future. It first checks whether the
+ * result of the output mapping is defined, and the target variable exists. If this is true, the put
+ * all(...) method is called to merge the result into the existing target variable. In all other
+ * cases the output mapping result is returned as is. If it evaluates to en error, this error is
+ * propagated to the final result of the evaluation
  */
 public final class VariableMappingTransformer {
 
@@ -173,8 +194,8 @@ private String mergeContextExpression(
     // example: x = 1 and a = {'c':2} results in a = {'b':1, 'c':2}
     final var existingContext = String.join(".", contextPath);
     return String.format(
-        "if (%s = null) then %s else put all(%s,%s)",
-        existingContext, nestedContext, existingContext, nestedContext);
+        "if (%s != null and is defined(%s)) then put all(%s,%s) else %s",
+        existingContext, nestedContext, existingContext, nestedContext, nestedContext);
   }
 
   private Expression parseExpression(

engine/src/test/java/io/camunda/zeebe/engine/processing/variable/mapping/VariableOutputMappingTransformerTest.java

@@ -19,30 +19,17 @@
 import java.util.List;
 import java.util.Map;
 import org.agrona.DirectBuffer;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-import org.junit.runners.Parameterized.Parameters;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 
-@RunWith(Parameterized.class)
 public final class VariableOutputMappingTransformerTest {
 
-  @Parameter(0)
-  public List<ZeebeMapping> mappings;
-
-  @Parameter(1)
-  public Map<String, DirectBuffer> variables;
-
-  @Parameter(2)
-  public String expectedOutput;
-
   private final VariableMappingTransformer transformer = new VariableMappingTransformer();
   private final ExpressionLanguage expressionLanguage =
       ExpressionLanguageFactory.createExpressionLanguage();
 
-  @Parameters(name = "with {0} to {2}")
-  public static Object[][] parameters() {
+  public static Object[][] parametersSuccessfulEvaluationToObject() {
     return new Object[][] {
       // no mappings
       {List.of(), Map.of(), "{}"},
@@ -143,8 +130,22 @@ public static Object[][] parameters() {
     };
   }
 
-  @Test
-  public void shouldApplyMappings() {
+  public static Object[][] parametersEvaluationToFailure() {
+    return new Object[][] {
+      {
+        List.of(mapping("x", "a.b")),
+        Map.of(),
+        "failed to evaluate expression '{a:if (a != null and is defined({b: x})) then put all(a,{b: x}) else {b: x}}': no variable found for name 'x'"
+      }, // #9543
+    };
+  }
+
+  @ParameterizedTest(name = "{index}: with {0} to {2}")
+  @MethodSource("parametersSuccessfulEvaluationToObject")
+  public void shouldEvaluateToObject(
+      final List<ZeebeMapping> mappings,
+      final Map<String, DirectBuffer> variables,
+      final String expectedOutput) {
     // given
     final var expression = transformer.transformOutputMappings(mappings, expressionLanguage);
 
@@ -161,6 +162,28 @@ public void shouldApplyMappings() {
     MsgPackUtil.assertEquality(result.toBuffer(), expectedOutput);
   }
 
+  @ParameterizedTest(name = "{index}: mapping {0} fails with: {2}")
+  @MethodSource("parametersEvaluationToFailure")
+  public void shouldEvaluateToFailure(
+      final List<ZeebeMapping> mappings,
+      final Map<String, DirectBuffer> variables,
+      final String failureMessage) {
+    // given
+    final var expression = transformer.transformOutputMappings(mappings, expressionLanguage);
+
+    assertThat(expression.isValid())
+        .describedAs("Expected valid expression: %s", expression.getFailureMessage())
+        .isTrue();
+
+    // when
+    final var result = expressionLanguage.evaluateExpression(expression, variables::get);
+
+    // then
+    assertThat(result.isFailure()).isTrue();
+
+    Assertions.assertThat(result.getFailureMessage()).isEqualTo(failureMessage);
+  }
+
   private static ZeebeMapping mapping(final String source, final String target) {
     return new ZeebeMapping() {
       @Override