From 445168c4d8bc140d730ac054499bc576ca27c644 Mon Sep 17 00:00:00 2001 From: Yury Brigadirenko Date: Sun, 9 Feb 2025 14:40:57 -0500 Subject: [PATCH] runtime-v2: do not log stack trace for EL MethodNotFound exception and unify the error messages (#1076) --- .../concord/common/ExceptionUtils.java | 21 ++-- .../runtime/v2/runner/LogExceptionsTest.java | 50 ++++++++- .../runtime/v2/runner/LogSegmentsTest.java | 13 ++- .../concord/runtime/v2/runner/MainTest.java | 12 +- .../methodNotFound/concord.yaml | 3 + .../variableNotFound/concord.yaml | 3 + .../runtime/v2/runner/el/LazyEvalList.java | 8 +- .../v2/runner/el/LazyExpressionEvaluator.java | 103 ++++++++++-------- .../el/resolvers/TaskMethodResolver.java | 8 +- .../runner/script/DefaultScriptEvaluator.java | 13 ++- .../v2/runner/tasks/TaskCallInterceptor.java | 18 +-- .../runner/tasks/TaskCallPolicyChecker.java | 7 +- .../runtime/v2/runner/vm/JoinCommand.java | 2 + .../runtime/v2/runner/vm/StepCommand.java | 24 +--- .../v2/runner/vm/WrappedException.java | 20 +++- .../v2/runner/el/ExpressionEvaluatorTest.java | 12 +- 16 files changed, 188 insertions(+), 129 deletions(-) create mode 100644 runtime/v2/runner-test/src/test/resources/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/methodNotFound/concord.yaml create mode 100644 runtime/v2/runner-test/src/test/resources/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/variableNotFound/concord.yaml diff --git a/common/src/main/java/com/walmartlabs/concord/common/ExceptionUtils.java b/common/src/main/java/com/walmartlabs/concord/common/ExceptionUtils.java index 3a5a3ab872..5848a6af0c 100644 --- a/common/src/main/java/com/walmartlabs/concord/common/ExceptionUtils.java +++ b/common/src/main/java/com/walmartlabs/concord/common/ExceptionUtils.java @@ -9,9 +9,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -36,12 +36,17 @@ public static List getExceptionList(Throwable e) { } @SuppressWarnings("unchecked") - public static T filterException(Exception e, Class clazz) { - return getExceptionList(e).stream() - .filter(clazz::isInstance) - .map(c -> (T)c) - .findAny() - .orElse(null); + public static T findLastException(T e, Class clazz) { + var exceptions = getExceptionList(e); + + for (int i = exceptions.size() - 1; i >= 0; i--) { + var ex = exceptions.get(i); + if (clazz.isInstance(ex)) { + return (T) ex; + } + } + + return e; } private ExceptionUtils() { diff --git a/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogExceptionsTest.java b/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogExceptionsTest.java index 902218ceec..c680864963 100644 --- a/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogExceptionsTest.java +++ b/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogExceptionsTest.java @@ -50,7 +50,7 @@ public void shouldLogExceptionStackTraceWhenTaskThrowsException() throws Excepti // error assertLog(runtime.lastLog(), ".*" + quote("(concord.yaml): Error @ line: 3, col: 7. boom!") + ".*"); // stacktrace - assertLog(runtime.lastLog(), ".*at com.walmartlabs.concord.runtime.v2.runner.LogExceptionsTest.shouldLogExceptionStackTraceWhenTaskThrowsException.*"); + assertLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.tasks.Tasks$FaultyTask2.execute") + ".*"); } @Test @@ -69,12 +69,13 @@ public void shouldLogExceptionStackTraceWhenExpressionThrowsException() throws E // error // TODO: "javax.el.ELException: java.lang.Exception: ..." remove javax.el.ELException? - assertLog(runtime.lastLog(), ".*" + quote("Error @ line: 3, col: 7. while evaluating expression '${faultyTask.exception('BOOM')}': javax.el.ELException: java.lang.Exception: BOOM") + ".*"); + assertLog(runtime.lastLog(), ".*" + quote("Error @ line: 3, col: 7. while evaluating expression '${faultyTask.exception('BOOM')}': BOOM") + ".*"); // stacktrace - assertLog(runtime.lastLog(), ".*at com.walmartlabs.concord.runtime.v2.runner.LogExceptionsTest.shouldLogExceptionStackTraceWhenExpressionThrowsException.*"); + assertLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.tasks.Tasks$FaultyTask") + ".*"); } @Test + @IgnoreSerializationAssert public void shouldLogExceptionStackTraceWhenTaskThrowsExceptionFromParallel() throws Exception { runtime.deploy("logExceptionTests/fromParallel"); @@ -144,6 +145,7 @@ public void noStacktraceForUserDefinedExceptionFromExpression() throws Exception } @Test + @IgnoreSerializationAssert public void noStacktraceForUserDefinedExceptionFromTaskParallel() throws Exception { runtime.deploy("logExceptionTests/userDefinedExceptionFromTaskParallel"); @@ -190,4 +192,46 @@ public void noStacktraceForTaskFailReturn() throws Exception { assertNoLog(runtime.lastLog(), ".*noStacktraceForTaskFailReturn.*"); assertNoLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.tasks.Tasks$FaultyTask.execute") + ".*"); } + + @Test + public void noStackTraceForMethodNotFound() throws Exception { + runtime.deploy("logExceptionTests/methodNotFound"); + + runtime.save(ProcessConfiguration.builder() + .build()); + + try { + runtime.run(); + fail("Exception expected"); + } catch (Exception e) { + // ignore + } + + // error + assertLog(runtime.lastLog(), ".*" + quote("(concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${faultyTask.unknownMethod('BOOM')}': Can't find 'unknownMethod()' method in com.walmartlabs.concord.runtime.v2.runner.tasks.Tasks$FaultyTask") + ".*"); + + // no stacktrace + assertNoLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.el.LazyExpressionEvaluator.evalExpr") + ".*"); + } + + @Test + public void noStackTraceForVariableNotFound() throws Exception { + runtime.deploy("logExceptionTests/variableNotFound"); + + runtime.save(ProcessConfiguration.builder() + .build()); + + try { + runtime.run(); + fail("Exception expected"); + } catch (Exception e) { + // ignore + } + + // error + assertLog(runtime.lastLog(), ".*" + quote("(concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${unknown}': Can't find a variable 'unknown'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'unknown'") + ".*"); + + // no stacktrace + assertNoLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.el.LazyExpressionEvaluator.evalExpr") + ".*"); + } } diff --git a/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogSegmentsTest.java b/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogSegmentsTest.java index 7587d76894..edc328d262 100644 --- a/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogSegmentsTest.java +++ b/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogSegmentsTest.java @@ -82,6 +82,7 @@ public void throwStepShouldContainErrorDescription() throws Exception { } @Test + @IgnoreSerializationAssert public void loopStepShouldLogErrorInProperLogSegment() throws Exception { deploy("logSegments/taskErrorWithLoop"); @@ -256,7 +257,7 @@ public void taskOutInvalidTest() throws Exception { // invalid task out definition -> log into task segment assertSegmentLog(runtime.lastLog(), 1, "[INFO ] test"); - assertSegmentLog(runtime.lastLog(), 1, "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); + assertSegmentLog(runtime.lastLog(), 1, "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); assertSegmentStatusError(runtime.lastLog(), 1); assertNoMoreSegments(); @@ -276,7 +277,7 @@ public void taskLoopInvalidTest() throws Exception { // ignore } - assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); + assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); assertNoMoreSegments(); } @@ -389,7 +390,7 @@ public void taskWithInvalidName() throws Exception { // ignore } - assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 4, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); + assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 4, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); assertNoMoreSegments(); } @@ -431,7 +432,7 @@ public void taskWithRetryInvalid() throws Exception { } // logs to the system segment - assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); + assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); assertNoMoreSegments(); } @@ -451,7 +452,7 @@ public void checkpointInvalidTest() throws Exception { } // error into system segment - assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); + assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); } @Test @@ -499,7 +500,7 @@ public void exprInvalidTest() throws Exception { } // error into system segment - assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); + assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'"); } @Test diff --git a/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/MainTest.java b/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/MainTest.java index 6f26b385af..b2ada9aded 100644 --- a/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/MainTest.java +++ b/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/MainTest.java @@ -508,7 +508,7 @@ public void testScriptErrorBlock() throws Exception { .build()); byte[] log = run(); - assertLog(log, ".*error occurred: java.lang.RuntimeException: Error: this is an error.*"); + assertLog(log, ".*" + quote("(concord.yml): Error @ line: 3, col: 7. Error: this is an error") + ".*"); } @Test @@ -1412,7 +1412,7 @@ public void testInvalidExpressionError() throws Exception { } String logString = new String(runtime.lastLog()); - String expected = "[ERROR] (concord.yml): Error @ line: 9, col: 7. Error Parsing: ${str.split('\\n')}. Encountered \"\\'\\\\n\" at line 1, column 13.\n" + + String expected = "[ERROR] (concord.yml): Error @ line: 9, col: 7. while parsing expression '${str.split('\\n')}': Encountered \"\\'\\\\n\" at line 1, column 13.\n" + "Was expecting one of:\n" + " \"{\" ...\n" + " ...\n" + @@ -1483,7 +1483,7 @@ public void testExpressionThrowException() throws Exception { // ignore } - String expected = "[ERROR] (concord.yml): Error @ line: 3, col: 7. while evaluating expression '${faultyTask.exception('BOOM')}': javax.el.ELException: java.lang.Exception: BOOM"; + String expected = "[ERROR] (concord.yml): Error @ line: 3, col: 7. while evaluating expression '${faultyTask.exception('BOOM')}': BOOM"; assertLog(runtime.lastLog(), ".*" + Pattern.quote(expected)); } @@ -1517,7 +1517,7 @@ public void testUnresolvedVarInStepName() throws Exception { } catch (Exception e) { } - assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 4, col: 7. Can't find a variable 'undefined' used in '${undefined}'") + ".*"); + assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 4, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'") + ".*"); } @Test @@ -1533,7 +1533,7 @@ public void testUnresolvedVarInLoop() throws Exception { } catch (Exception e) { } - assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 6, col: 7. Can't find a variable 'undefined' used in '${undefined}'") + ".*"); + assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 6, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'") + ".*"); } @Test @@ -1549,7 +1549,7 @@ public void testUnresolvedVarInRetry() throws Exception { } catch (Exception e) { } - assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 6, col: 7. Can't find a variable 'undefined' used in '${undefined}'") + ".*"); + assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 6, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'") + ".*"); } @Test diff --git a/runtime/v2/runner-test/src/test/resources/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/methodNotFound/concord.yaml b/runtime/v2/runner-test/src/test/resources/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/methodNotFound/concord.yaml new file mode 100644 index 0000000000..b16ba6597c --- /dev/null +++ b/runtime/v2/runner-test/src/test/resources/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/methodNotFound/concord.yaml @@ -0,0 +1,3 @@ +flows: + default: + - expr: "${faultyTask.unknownMethod('BOOM')}" diff --git a/runtime/v2/runner-test/src/test/resources/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/variableNotFound/concord.yaml b/runtime/v2/runner-test/src/test/resources/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/variableNotFound/concord.yaml new file mode 100644 index 0000000000..feb2fc2ab8 --- /dev/null +++ b/runtime/v2/runner-test/src/test/resources/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/variableNotFound/concord.yaml @@ -0,0 +1,3 @@ +flows: + default: + - expr: "${unknown}" diff --git a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyEvalList.java b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyEvalList.java index 2a752a85db..83ffde937a 100644 --- a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyEvalList.java +++ b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyEvalList.java @@ -9,9 +9,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -27,10 +27,10 @@ public class LazyEvalList extends AbstractList { private final LazyExpressionEvaluator evaluator; private final LazyEvalContext context; private final Set inflightKeys = new HashSet<>(); - private final List originalValues; + private final List originalValues; private final Map evaluatedValues = new HashMap<>(); - public LazyEvalList(LazyExpressionEvaluator evaluator, LazyEvalContext context, List src) { + public LazyEvalList(LazyExpressionEvaluator evaluator, LazyEvalContext context, List src) { this.evaluator = evaluator; this.context = context; this.originalValues = src; diff --git a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyExpressionEvaluator.java b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyExpressionEvaluator.java index afdf75171a..102b78ff2c 100644 --- a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyExpressionEvaluator.java +++ b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyExpressionEvaluator.java @@ -27,6 +27,7 @@ import com.walmartlabs.concord.runtime.v2.runner.el.resolvers.MapELResolver; import com.walmartlabs.concord.runtime.v2.runner.el.resolvers.*; import com.walmartlabs.concord.runtime.v2.runner.tasks.TaskProviders; +import com.walmartlabs.concord.runtime.v2.runner.vm.WrappedException; import com.walmartlabs.concord.runtime.v2.sdk.*; import javax.el.*; @@ -65,12 +66,12 @@ public T eval(EvalContext ctx, Object value, Class expectedType) { } if (value instanceof Map) { - Map m = nestedToMap((Map)value); + var m = nestedToMap((Map) value); value = mergeWithVariables(ctx, m, ((Map) value).keySet().stream().filter(ConfigurationUtils::isNestedKey).collect(Collectors.toSet())); } if (ctx.useIntermediateResults() && value instanceof Map) { - Map m = (Map) value; + var m = (Map) value; if (m.isEmpty()) { return expectedType.cast(m); } @@ -88,23 +89,20 @@ T evalValue(LazyEvalContext ctx, Object value, Class expectedType) { } if (value instanceof Map) { - Map m = (Map) value; + var m = (Map) value; return expectedType.cast(new LazyEvalMap(this, ctx, m)); - } else if (value instanceof List) { - List src = (List) value; + } else if (value instanceof List src) { return expectedType.cast(new LazyEvalList(this, ctx, src)); - } else if (value instanceof Set) { - Set src = (Set) value; - + } else if (value instanceof Set src) { // use LinkedHashSet to preserve the order of keys - Set dst = new LinkedHashSet<>(src.size()); - for (Object vv : src) { + var dst = new LinkedHashSet<>(src.size()); + for (var vv : src) { dst.add(evalValue(ctx, vv, Object.class)); } return expectedType.cast(dst); } else if (value.getClass().isArray()) { - Object[] src = (Object[]) value; + var src = (Object[]) value; if (src.length == 0) { return expectedType.cast(src); } @@ -124,9 +122,9 @@ T evalValue(LazyEvalContext ctx, Object value, Class expectedType) { } private T evalExpr(LazyEvalContext ctx, String expr, Class type) { - ELResolver resolver = createResolver(ctx, expressionFactory); + var resolver = createResolver(ctx, expressionFactory); - StandardELContext sc = new StandardELContext(expressionFactory) { + var sc = new StandardELContext(expressionFactory) { @Override public ELResolver getELResolver() { return resolver; @@ -139,33 +137,38 @@ public FunctionMapper getFunctionMapper() { }; sc.putContext(ExpressionFactory.class, expressionFactory); - ValueExpression x = expressionFactory.createValueExpression(sc, expr, type); try { - Object v = withEvalContext(ctx, () -> x.getValue(sc)); + var x = expressionFactory.createValueExpression(sc, expr, type); + var v = withEvalContext(ctx, () -> x.getValue(sc)); return type.cast(v); } catch (PropertyNotFoundException e) { if (ctx.undefinedVariableAsNull()) { return null; } - String errorMessage; - - String propName = propertyNameFromException(e); - if (propName != null) { - errorMessage = String.format("Can't find a variable %s used in '%s'. " + - "Check if it is defined in the current scope. Details: %s", propName, expr, e.getMessage()); - } else { - errorMessage = String.format("Can't find the specified variable in '%s'. " + - "Check if it is defined in the current scope. Details: %s", expr, e.getMessage()); + var errorMessage = propertyNameFromException(e) + .map(propName -> String.format("Can't find a variable %s. " + + "Check if it is defined in the current scope. Details: %s", propName, e.getMessage())) + .orElse(String.format("Can't find the specified variable. " + + "Check if it is defined in the current scope. Details: %s", e.getMessage())); + + throw new UserDefinedException(exceptionPrefix(expr) + errorMessage); + } catch (MethodNotFoundException e) { + throw new UserDefinedException(exceptionPrefix(expr) + e.getMessage()); + } catch (UserDefinedException e) { + throw e; + } catch (javax.el.ELException e) { + var lastElException = ExceptionUtils.findLastException(e, javax.el.ELException.class); + if (lastElException.getCause() instanceof UserDefinedException ue) { + throw ue; + } else if (e.getCause() instanceof com.sun.el.parser.ParseException pe) { + throw new UserDefinedException("while parsing expression '" + expr + "': " + pe.getMessage()); + } else if (lastElException.getCause() instanceof Exception ee) { + throw new WrappedException(exceptionPrefix(expr), ee); } - - throw new UserDefinedException(errorMessage); + throw lastElException; } catch (Exception e) { - UserDefinedException u = ExceptionUtils.filterException(e, UserDefinedException.class); - if (u != null) { - throw u; - } - throw new RuntimeException("while evaluating expression '" + expr + "': " + e.getMessage()); + throw new WrappedException(exceptionPrefix(expr), e); } } @@ -176,7 +179,7 @@ public FunctionMapper getFunctionMapper() { private ELResolver createResolver(LazyEvalContext evalContext, ExpressionFactory expressionFactory) { - CompositeELResolver r = new CompositeELResolver(); + var r = new CompositeELResolver(); if (evalContext.scope() != null) { r.add(new VariableResolver(evalContext.scope())); } @@ -199,7 +202,7 @@ private ELResolver createResolver(LazyEvalContext evalContext, } private static FunctionMapper createFunctionMapper() { - Map functions = new HashMap<>(); + var functions = new HashMap(); functions.put("hasVariable", HasVariableFunction.getMethod()); functions.put("hasNonNullVariable", HasNonNullVariableFunction.getMethod()); functions.put("orDefault", OrDefaultFunction.getMethod()); @@ -219,10 +222,10 @@ private static boolean hasExpression(String s) { } private static Map nestedToMap(Map value) { - Map result = new LinkedHashMap<>(); - for (Map.Entry e : value.entrySet()) { + Map result = new LinkedHashMap(); + for (var e : value.entrySet()) { if (isNestedKey(e.getKey())) { - Map m = toNested(e.getKey(), e.getValue()); + var m = toNested(e.getKey(), e.getValue()); result = deepMerge(result, m); } else { result.put(e.getKey(), e.getValue()); @@ -233,16 +236,16 @@ private static Map nestedToMap(Map value) { @SuppressWarnings("unchecked") private static Map mergeWithVariables(EvalContext ctx, Map m, Set nestedKeys) { - Map result = new LinkedHashMap<>(); - for (Map.Entry e : m.entrySet()) { - String key = e.getKey(); - Object value = e.getValue(); - boolean isNested = nestedKeys.stream().anyMatch(s -> s.startsWith(key + ".")); + var result = new LinkedHashMap(); + for (var e : m.entrySet()) { + var key = e.getKey(); + var value = e.getValue(); + var isNested = nestedKeys.stream().anyMatch(s -> s.startsWith(key + ".")); if (isNested && ctx.variables().has(key)) { - Object o = ctx.variables().get(key); + var o = ctx.variables().get(key); if (o instanceof Map && e.getValue() instanceof Map) { - Map valuesFromVars = (Map)o; - value = deepMerge(valuesFromVars, (Map)value); + var valuesFromVars = (Map) o; + value = deepMerge(valuesFromVars, (Map) value); } } result.put(key, value); @@ -250,17 +253,21 @@ private static Map mergeWithVariables(EvalContext ctx, Map propertyNameFromException(PropertyNotFoundException e) { if (e.getMessage() == null) { - return null; + return Optional.empty(); } if (e.getMessage().startsWith(PROP_NOT_FOUND_EL_MESSAGE)) { - return e.getMessage().substring(PROP_NOT_FOUND_EL_MESSAGE.length()); + return Optional.of(e.getMessage().substring(PROP_NOT_FOUND_EL_MESSAGE.length())); } - return null; + return Optional.empty(); } } diff --git a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/resolvers/TaskMethodResolver.java b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/resolvers/TaskMethodResolver.java index 19815249f4..36d8cc994d 100644 --- a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/resolvers/TaskMethodResolver.java +++ b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/resolvers/TaskMethodResolver.java @@ -90,14 +90,12 @@ public Object invoke(ELContext elContext, Object base, Object method, Class[] }); } catch (javax.el.MethodNotFoundException e) { throw new MethodNotFoundException(invocation.taskClass(), method, paramTypes); - } catch (javax.el.ELException e) { - if (e.getCause() instanceof RuntimeException) { - throw (RuntimeException) e.getCause(); - } - throw e; } catch (RuntimeException e) { throw e; } catch (TaskException e) { + if (e.getCause() instanceof RuntimeException re) { + throw re; + } throw new RuntimeException(e.getCause()); } catch (Exception e) { throw new RuntimeException(e); diff --git a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/script/DefaultScriptEvaluator.java b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/script/DefaultScriptEvaluator.java index 72002d5fe4..0c6d6a18d9 100644 --- a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/script/DefaultScriptEvaluator.java +++ b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/script/DefaultScriptEvaluator.java @@ -23,8 +23,9 @@ import com.oracle.truffle.js.scriptengine.GraalJSEngineFactory; import com.oracle.truffle.js.scriptengine.GraalJSScriptEngine; import com.walmartlabs.concord.runtime.v2.runner.tasks.TaskProviders; +import com.walmartlabs.concord.runtime.v2.runner.vm.WrappedException; import com.walmartlabs.concord.runtime.v2.sdk.Context; -import com.walmartlabs.concord.runtime.v2.sdk.ProcessConfiguration; +import com.walmartlabs.concord.runtime.v2.sdk.UserDefinedException; import com.walmartlabs.concord.sdk.Constants; import org.graalvm.polyglot.Engine; import org.graalvm.polyglot.HostAccess; @@ -100,11 +101,11 @@ public ScriptResult eval(Context context, String language, Reader input, Map esVersion = Arrays.stream(GRAAL_ES_VERSIONS) .filter(it -> it.equals(value)) .findFirst(); - if (!esVersion.isPresent()) { - throw new RuntimeException("unsupported esVersion: " + value.toString()); + if (esVersion.isEmpty()) { + throw new UserDefinedException("unsupported esVersion: " + value.toString()); } ctx.option("js.ecmascript-version", esVersion.get().toString()); } diff --git a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/tasks/TaskCallInterceptor.java b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/tasks/TaskCallInterceptor.java index f5f1815f88..634397b914 100644 --- a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/tasks/TaskCallInterceptor.java +++ b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/tasks/TaskCallInterceptor.java @@ -49,24 +49,24 @@ public TaskCallInterceptor(Set listeners) { this.listeners = listeners; } - public T invoke(CallContext ctx, Method method, Callable callable) throws Exception { + public T invoke(CallContext ctx, Method method, Callable callable) throws TaskException { // record the PRE event - TaskCallEvent preEvent = eventBuilder(Phase.PRE, method, ctx).build(); + var preEvent = eventBuilder(Phase.PRE, method, ctx).build(); listeners.forEach(l -> l.onEvent(preEvent)); // call the callable and measure the duration T result = null; Exception error = null; - long startedAt = System.currentTimeMillis(); + var startedAt = System.currentTimeMillis(); try { result = callable.call(); } catch (Exception e) { error = e; } - long duration = System.currentTimeMillis() - startedAt; + var duration = System.currentTimeMillis() - startedAt; // record the POST event - TaskCallEvent postEvent = eventBuilder(Phase.POST, method, ctx) + var postEvent = eventBuilder(Phase.POST, method, ctx) .error(errorMessage(error)) .duration(duration) .result(result instanceof Serializable ? (Serializable) result : null) @@ -115,17 +115,17 @@ public interface Method { @AllowNulls @Value.Default default List arguments() { - return Collections.emptyList(); + return List.of(); } @AllowNulls @Value.Default default List> annotations() { - return Collections.emptyList(); + return List.of(); } - static Method of(Class taskClass, String methodName, List params) { - List> annotations = Collections.emptyList(); + static Method of(Class taskClass, String methodName, List params) throws javax.el.MethodNotFoundException { + List> annotations = List.of(); var m = ReflectionUtil.findMethod(taskClass, methodName, null, params.toArray()); if (m != null && !m.isVarArgs()) { annotations = Arrays.stream(m.getParameterAnnotations()) diff --git a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/tasks/TaskCallPolicyChecker.java b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/tasks/TaskCallPolicyChecker.java index ab593b1b9e..95bf07a4cb 100644 --- a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/tasks/TaskCallPolicyChecker.java +++ b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/tasks/TaskCallPolicyChecker.java @@ -9,9 +9,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -24,6 +24,7 @@ import com.walmartlabs.concord.policyengine.PolicyEngine; import com.walmartlabs.concord.policyengine.TaskRule; import com.walmartlabs.concord.runtime.v2.runner.TaskResultService; +import com.walmartlabs.concord.runtime.v2.sdk.UserDefinedException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,7 +59,7 @@ public void onEvent(TaskCallEvent event) { result.getDeny().forEach(d -> log.error("Task call '{}' is forbidden by the task policy {}", event.taskName(), d.getRule())); if (!result.getDeny().isEmpty()) { - throw new RuntimeException("Found forbidden tasks"); + throw new UserDefinedException("Found forbidden tasks"); } } } diff --git a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/JoinCommand.java b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/JoinCommand.java index 9837d6c242..50e60898f6 100644 --- a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/JoinCommand.java +++ b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/JoinCommand.java @@ -29,6 +29,7 @@ import java.io.Serial; import java.util.Collection; import java.util.Map; +import java.util.Objects; public class JoinCommand extends StepCommand { @@ -89,6 +90,7 @@ protected void execute(Runtime runtime, State state, ThreadId threadId) { if (!failed.isEmpty() && !anyReady) { throw new ParallelExecutionException(failed.stream() .map(state::clearThreadError) + .filter(Objects::nonNull) .toList()); } diff --git a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/StepCommand.java b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/StepCommand.java index f05cbce2b8..91d6a831e1 100644 --- a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/StepCommand.java +++ b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/StepCommand.java @@ -20,7 +20,6 @@ * ===== */ -import com.walmartlabs.concord.common.ExceptionUtils; import com.walmartlabs.concord.runtime.v2.model.Location; import com.walmartlabs.concord.runtime.v2.model.Step; import com.walmartlabs.concord.runtime.v2.runner.context.ContextFactory; @@ -28,13 +27,11 @@ import com.walmartlabs.concord.runtime.v2.runner.logging.RunnerLogger; import com.walmartlabs.concord.runtime.v2.runner.tasks.ContextProvider; import com.walmartlabs.concord.runtime.v2.sdk.Context; -import com.walmartlabs.concord.runtime.v2.sdk.UserDefinedException; import com.walmartlabs.concord.svm.Runtime; import com.walmartlabs.concord.svm.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.el.ELException; import java.util.List; import java.util.UUID; import java.util.stream.Collectors; @@ -105,9 +102,9 @@ private void executeWithContext(Context ctx, Runtime runtime, State state, Threa public static void logException(Step step, State state, ThreadId threadId, Exception e) { // for backward compatibility... if (step != null) { - log.error("{} {}", Location.toErrorPrefix(step.getLocation()), getExceptionMessage(e)); + log.error("{} {}", Location.toErrorPrefix(step.getLocation()), e.getMessage()); } else { - log.error("{}", getExceptionMessage(e)); + log.error("{}", e.getMessage()); } List stackTrace = state.getStackTrace(threadId); @@ -117,21 +114,4 @@ public static void logException(Step step, State state, ThreadId threadId, Excep } protected abstract void execute(Runtime runtime, State state, ThreadId threadId); - - private static String getExceptionMessage(Exception e) { - UserDefinedException u = ExceptionUtils.filterException(e, UserDefinedException.class); - if (u != null) { - return u.getMessage(); - } - - if (e instanceof ELException) { - return - ExceptionUtils.getExceptionList(e).stream() - .map(Throwable::getMessage) - .collect(Collectors.joining(". ")); - } - - List exceptions = ExceptionUtils.getExceptionList(e); - return exceptions.get(exceptions.size() - 1).getMessage(); - } } diff --git a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/WrappedException.java b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/WrappedException.java index 8001c7392c..3160c471c7 100644 --- a/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/WrappedException.java +++ b/runtime/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/vm/WrappedException.java @@ -26,8 +26,15 @@ public class WrappedException extends RuntimeException { + private final String prefix; + public WrappedException(Exception cause) { + this(null, Objects.requireNonNull(cause)); + } + + public WrappedException(String prefix, Exception cause) { super(Objects.requireNonNull(cause)); + this.prefix = prefix; } @Override @@ -52,16 +59,23 @@ public void printStackTrace(PrintStream s) { @Override public String getLocalizedMessage() { - return getCause().getLocalizedMessage(); + return messagePrefix() + getCause().getLocalizedMessage(); } @Override public String getMessage() { - return getCause().getMessage(); + return messagePrefix() + getCause().getMessage(); } @Override public String toString() { - return getCause().toString(); + return messagePrefix() + getCause().toString(); + } + + private String messagePrefix() { + if (prefix != null) { + return prefix; + } + return ""; } } diff --git a/runtime/v2/runner/src/test/java/com/walmartlabs/concord/runtime/v2/runner/el/ExpressionEvaluatorTest.java b/runtime/v2/runner/src/test/java/com/walmartlabs/concord/runtime/v2/runner/el/ExpressionEvaluatorTest.java index fa0f87cbaa..fe8d036032 100644 --- a/runtime/v2/runner/src/test/java/com/walmartlabs/concord/runtime/v2/runner/el/ExpressionEvaluatorTest.java +++ b/runtime/v2/runner/src/test/java/com/walmartlabs/concord/runtime/v2/runner/el/ExpressionEvaluatorTest.java @@ -73,7 +73,7 @@ public void testStrictUndef() { ee.eval(ctx, "Hello ${name}", String.class); fail("exception expected"); } catch (RuntimeException e) { - assertThat(e.getMessage(), containsString("variable 'name' used in 'Hello ${name}'")); + assertThat(e.getMessage(), containsString("while evaluating expression 'Hello ${name}': Can't find a variable 'name'")); } // undef as null @@ -155,7 +155,7 @@ public void testEval1() { ee.evalAsMap(global(vars), input); fail("exception expected"); } catch (RuntimeException e) { - assertThat(e.getMessage(), containsString("variable 'y' used in '${y}'")); + assertThat(e.getMessage(), containsString("while evaluating expression '${y}': Can't find a variable 'y'")); } // undef -> x = null, z = null, y ...y3 = null @@ -183,7 +183,7 @@ public void testEval2() { ee.evalAsMap(global(vars), input); fail("exception expected"); } catch (RuntimeException e) { - assertThat(e.getMessage(), containsString("variable 'y' used in '${y}'")); + assertThat(e.getMessage(), containsString("while evaluating expression '${y}': Can't find a variable 'y'")); } // scope @@ -192,7 +192,7 @@ public void testEval2() { ee.evalAsMap(scope(vars), input); fail("exception expected"); } catch (RuntimeException e) { - assertThat(e.getMessage(), containsString("variable 'x' used in '${x}'")); + assertThat(e.getMessage(), containsString("while evaluating expression '${x}': Can't find a variable 'x'")); } } @@ -229,7 +229,7 @@ public void testEval3() { ee.evalAsMap(global(vars), input); fail("exception expected"); } catch (Exception e) { - assertThat(e.getMessage(), containsString("variable 'y' used in '${y}'")); + assertThat(e.getMessage(), containsString("while evaluating expression '${y}': Can't find a variable 'y'")); } verify(task, times(0)).foo(anyString()); @@ -298,7 +298,7 @@ public void testEval5() { try { ee.evalAsMap(scope(vars), input); } catch (RuntimeException e) { - assertThat(e.getMessage(), containsString("variable 'y1' used in '${y1}'")); + assertThat(e.getMessage(), containsString("while evaluating expression '${y1}': Can't find a variable 'y1'")); } }