Fix function dispatch failure error messages to be properly formatted

PiperOrigin-RevId: 868911649

Author: l46kok

runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel

@@ -199,10 +199,10 @@ java_library(
     name = "eval_zero_arity",
     srcs = ["EvalZeroArity.java"],
     deps = [
+        ":eval_helpers",
         ":execution_frame",
         ":planned_interpretable",
         "//common/values",
-        "//common/values:cel_value",
         "//runtime:evaluation_exception",
         "//runtime:interpretable",
         "//runtime:resolved_overload",
@@ -217,7 +217,6 @@ java_library(
         ":execution_frame",
         ":planned_interpretable",
         "//common/values",
-        "//common/values:cel_value",
         "//runtime:evaluation_exception",
         "//runtime:interpretable",
         "//runtime:resolved_overload",
@@ -232,7 +231,6 @@ java_library(
         ":execution_frame",
         ":planned_interpretable",
         "//common/values",
-        "//common/values:cel_value",
         "//runtime:evaluation_exception",
         "//runtime:interpretable",
         "//runtime:resolved_overload",
@@ -248,7 +246,6 @@ java_library(
         ":planned_interpretable",
         "//common/exceptions:overload_not_found",
         "//common/values",
-        "//common/values:cel_value",
         "//runtime:evaluation_exception",
         "//runtime:interpretable",
         "//runtime:resolved_overload",
@@ -377,7 +374,11 @@ java_library(
         "//common:error_codes",
         "//common/exceptions:runtime_exception",
         "//common/values",
+        "//common/values:cel_value",
+        "//runtime:evaluation_exception",
         "//runtime:interpretable",
+        "//runtime:resolved_overload",
+        "@maven//:com_google_guava_guava",
     ],
 )
 

runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java

@@ -14,9 +14,14 @@
 
 package dev.cel.runtime.planner;
 
+import com.google.common.base.Joiner;
 import dev.cel.common.CelErrorCode;
 import dev.cel.common.exceptions.CelRuntimeException;
+import dev.cel.common.values.CelValue;
+import dev.cel.common.values.CelValueConverter;
 import dev.cel.common.values.ErrorValue;
+import dev.cel.runtime.CelEvaluationException;
+import dev.cel.runtime.CelResolvedOverload;
 import dev.cel.runtime.GlobalResolver;
 
 final class EvalHelpers {
@@ -51,5 +56,26 @@ static Object evalStrictly(
     }
   }
 
+  static Object dispatch(CelResolvedOverload overload, CelValueConverter valueConverter, Object[] args) throws CelEvaluationException {
+    try {
+      Object result = overload.getDefinition().apply(args);
+      Object runtimeValue = valueConverter.toRuntimeValue(result);
+      if (runtimeValue instanceof CelValue) {
+        return valueConverter.unwrap((CelValue) runtimeValue);
+      }
+
+      return runtimeValue;
+    } catch (CelRuntimeException e) {
+      // Function dispatch failure that's already been handled -- just propagate.
+      throw e;
+    } catch (RuntimeException e) {
+      // Unexpected function dispatch failure.
+      throw new IllegalArgumentException(String.format(
+          "Function '%s' failed with arg(s) '%s'",
+          overload.getOverloadId(), Joiner.on(", ").join(args)),
+          e);
+    }
+  }
+
   private EvalHelpers() {}
 }

runtime/src/main/java/dev/cel/runtime/planner/EvalLateBoundCall.java

@@ -18,7 +18,6 @@
 
 import com.google.common.collect.ImmutableList;
 import dev.cel.common.exceptions.CelOverloadNotFoundException;
-import dev.cel.common.values.CelValue;
 import dev.cel.common.values.CelValueConverter;
 import dev.cel.runtime.CelEvaluationException;
 import dev.cel.runtime.CelResolvedOverload;
@@ -48,12 +47,7 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEval
             .findOverload(functionName, overloadIds, argVals)
             .orElseThrow(() -> new CelOverloadNotFoundException(functionName, overloadIds));
 
-    Object result = resolvedOverload.getDefinition().apply(argVals);
-    Object runtimeValue = celValueConverter.toRuntimeValue(result);
-    if (runtimeValue instanceof CelValue) {
-      return celValueConverter.unwrap((CelValue) runtimeValue);
-    }
-    return runtimeValue;
+    return EvalHelpers.dispatch(resolvedOverload, celValueConverter, argVals);
   }
 
   static EvalLateBoundCall create(

runtime/src/main/java/dev/cel/runtime/planner/EvalUnary.java

@@ -17,7 +17,6 @@
 import static dev.cel.runtime.planner.EvalHelpers.evalNonstrictly;
 import static dev.cel.runtime.planner.EvalHelpers.evalStrictly;
 
-import dev.cel.common.values.CelValue;
 import dev.cel.common.values.CelValueConverter;
 import dev.cel.runtime.CelEvaluationException;
 import dev.cel.runtime.CelResolvedOverload;
@@ -37,12 +36,7 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEval
             : evalNonstrictly(arg, resolver, frame);
     Object[] arguments = new Object[] {argVal};
 
-    Object result = resolvedOverload.getDefinition().apply(arguments);
-    Object runtimeValue = celValueConverter.toRuntimeValue(result);
-    if (runtimeValue instanceof CelValue) {
-      return celValueConverter.unwrap((CelValue) runtimeValue);
-    }
-    return runtimeValue;
+    return EvalHelpers.dispatch(resolvedOverload, celValueConverter, arguments);
   }
 
   static EvalUnary create(

runtime/src/main/java/dev/cel/runtime/planner/EvalVarArgsCall.java

@@ -17,7 +17,6 @@
 import static dev.cel.runtime.planner.EvalHelpers.evalNonstrictly;
 import static dev.cel.runtime.planner.EvalHelpers.evalStrictly;
 
-import dev.cel.common.values.CelValue;
 import dev.cel.common.values.CelValueConverter;
 import dev.cel.runtime.CelEvaluationException;
 import dev.cel.runtime.CelResolvedOverload;
@@ -43,12 +42,7 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEval
               : evalNonstrictly(arg, resolver, frame);
     }
 
-    Object result = resolvedOverload.getDefinition().apply(argVals);
-    Object runtimeValue = celValueConverter.toRuntimeValue(result);
-    if (runtimeValue instanceof CelValue) {
-      return celValueConverter.unwrap((CelValue) runtimeValue);
-    }
-    return runtimeValue;
+    return EvalHelpers.dispatch(resolvedOverload, celValueConverter, argVals);
   }
 
   static EvalVarArgsCall create(

runtime/src/main/java/dev/cel/runtime/planner/EvalZeroArity.java

@@ -14,7 +14,6 @@
 
 package dev.cel.runtime.planner;
 
-import dev.cel.common.values.CelValue;
 import dev.cel.common.values.CelValueConverter;
 import dev.cel.runtime.CelEvaluationException;
 import dev.cel.runtime.CelResolvedOverload;
@@ -28,12 +27,7 @@ final class EvalZeroArity extends PlannedInterpretable {
 
   @Override
   public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEvaluationException {
-    Object result = resolvedOverload.getDefinition().apply(EMPTY_ARRAY);
-    Object runtimeValue = celValueConverter.toRuntimeValue(result);
-    if (runtimeValue instanceof CelValue) {
-      return celValueConverter.unwrap((CelValue) runtimeValue);
-    }
-    return runtimeValue;
+    return EvalHelpers.dispatch(resolvedOverload, celValueConverter, EMPTY_ARRAY);
   }
 
   static EvalZeroArity create(

runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java

@@ -115,7 +115,11 @@ private CelEvaluationException newCelEvaluationException(long exprId, Exception
     } else if (e instanceof CelRuntimeException) {
       builder = CelEvaluationExceptionBuilder.newBuilder((CelRuntimeException) e);
     } else {
-      builder = CelEvaluationExceptionBuilder.newBuilder(e.getMessage()).setCause(e);
+      // Unhandled function dispatch failures wraps the original exception with a descriptive message
+      // (e.g: "Function foo failed with...")
+      // We need to unwrap the cause here to preserve the original exception message and its cause.
+      Throwable cause = e.getCause() != null ? e.getCause() : e;
+      builder = CelEvaluationExceptionBuilder.newBuilder(e.getMessage()).setCause(cause);
     }
 
     return builder.setMetadata(metadata(), exprId).build();

runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java

@@ -52,12 +52,6 @@ public void typeComparisons() {
     skipBaselineVerification();
   }
 
-  @Override
-  public void optional_errors() {
-    // TODO: Fix error message for function dispatch failures
-    skipBaselineVerification();
-  }
-
   @Override
   public void jsonFieldNames() throws Exception {
     // TODO: Support JSON field names for planner

runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java

@@ -397,10 +397,12 @@ public void plan_call_zeroArgs() throws Exception {
   public void plan_call_throws() throws Exception {
     CelAbstractSyntaxTree ast = compile("error()");
     Program program = PLANNER.plan(ast);
+    String expectedOverloadId = isParseOnly ? "error" : "error_overload";
 
     CelEvaluationException e = assertThrows(CelEvaluationException.class, program::eval);
-    assertThat(e).hasMessageThat().contains("evaluation error at <input>:5: Intentional error");
-    assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
+    assertThat(e).hasMessageThat().contains("evaluation error at <input>:5: Function '" + expectedOverloadId + "' failed with arg(s) ''");
+    assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class);
+    assertThat(e.getCause().getMessage()).contains("Intentional error");
   }
 
   @Test