Skip to content

Commit

Permalink
runtime-v2: hide stack trace and log location for ParallelExecutionEx…
Browse files Browse the repository at this point in the history
…ception (#1081)
  • Loading branch information
brig authored Feb 17, 2025
1 parent 14386b9 commit 34cbc1d
Show file tree
Hide file tree
Showing 18 changed files with 291 additions and 35 deletions.
3 changes: 1 addition & 2 deletions cli/src/main/java/com/walmartlabs/concord/cli/Run.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,10 @@
import com.walmartlabs.concord.runtime.v2.runner.Runner;
import com.walmartlabs.concord.runtime.v2.runner.guice.ProcessDependenciesModule;
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 com.walmartlabs.concord.sdk.Constants;
import com.walmartlabs.concord.sdk.MapUtils;
import com.walmartlabs.concord.svm.ParallelExecutionException;
import com.walmartlabs.concord.runtime.v2.runner.vm.ParallelExecutionException;
import picocli.CommandLine.Command;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Option;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -43,7 +43,7 @@ static String toErrorPrefix(Location location) {
if (location == null || location.fileName() == null) {
return "(n/a): Error.";
} else {
return "(" + location.fileName() + "): Error @ " + toShortString(location) + ".";
return "(" + location.fileName() + "): Error @ " + toShortString(location);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
import com.walmartlabs.concord.runtime.v2.runner.tasks.TaskV2Provider;
import com.walmartlabs.concord.runtime.v2.runner.vm.BlockCommand;
import com.walmartlabs.concord.runtime.v2.runner.vm.ParallelCommand;
import com.walmartlabs.concord.runtime.v2.runner.vm.WrappedException;
import com.walmartlabs.concord.runtime.v2.runner.vm.ParallelExecutionException;
import com.walmartlabs.concord.runtime.v2.sdk.*;
import com.walmartlabs.concord.sdk.Constants;
import com.walmartlabs.concord.svm.Runtime;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public void noStacktraceForUserDefinedExceptionFromTaskParallel() throws Excepti

// error
assertLog(runtime.lastLog(), ".*" + quote("(concord.yaml): Error @ line: 4, col: 11. boom!") + ".*");
assertMultiLineLog(runtime.lastLog(), ".*" + quote("(concord.yaml): Error @ line: 3, col: 7. Parallel execution errors: ") + "\n" + quote("boom!"));
assertMultiLineLog(runtime.lastLog(), ".*" + quote("(concord.yaml): Error @ line: 3, col: 7. Parallel execution errors: ") + "\n" + quote("(concord.yaml): Error @ line: 4, col: 11, thread: 1: boom!"));

assertLogExactMatch(runtime.lastLog(), 1, ".*" + quote("Parallel execution errors") + ".*");

Expand Down Expand Up @@ -234,4 +234,48 @@ public void noStackTraceForVariableNotFound() throws Exception {
// no stacktrace
assertNoLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.el.LazyExpressionEvaluator.evalExpr") + ".*");
}

@Test
public void noStacktraceForScriptException() throws Exception {
runtime.deploy("logExceptionTests/fromScript");

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. Something went wrong") + ".*");

// no stacktrace
assertNoLog(runtime.lastLog(), ".*DefaultScriptEvaluator.*");
}

@Test
@IgnoreSerializationAssert
public void noStacktraceForUserDefinedExceptionFromTaskParallelParallel() throws Exception {
runtime.deploy("logExceptionTests/fromParallelParallel");

runtime.save(ProcessConfiguration.builder()
.build());

try {
runtime.run();
fail("Exception expected");
} catch (Exception e) {
// ignore
}

// error
assertLogExactMatch(runtime.lastLog(), 2, ".*" + quote("(concord.yaml): Error @ line: 11, col: 11. boom!") + ".*");

// no stacktrace
assertNoLog(runtime.lastLog(), ".*" + quote("com.walmartlabs.concord.svm.ParallelExecutionException") + ".*");
assertNoLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.vm.JoinCommand.execute") + ".*");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,7 @@ public void testParallelLoopTask() throws Exception {
}

@Test
@IgnoreSerializationAssert
public void testParallelWithError() throws Exception {
deploy("parallelWithError");

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
flows:
default:
- call: inner
loop:
items: [1, 2]
mode: parallel
parallelism: 2

inner:
- parallel:
- task: "userDefinedExceptionTask"
- log: "OK"
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
flows:
default:
- script: "js"
body: |
log.info('ready...');
throw new java.lang.RuntimeException("Something went wrong");
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,12 @@
import com.walmartlabs.concord.runtime.v2.runner.guice.ObjectMapperProvider;
import com.walmartlabs.concord.runtime.v2.runner.logging.LoggingConfigurator;
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.ProcessConfiguration;
import com.walmartlabs.concord.runtime.v2.sdk.UserDefinedException;
import com.walmartlabs.concord.runtime.v2.sdk.WorkingDirectory;
import com.walmartlabs.concord.sdk.Constants;
import com.walmartlabs.concord.svm.Frame;
import com.walmartlabs.concord.svm.ParallelExecutionException;
import com.walmartlabs.concord.runtime.v2.runner.vm.ParallelExecutionException;
import com.walmartlabs.concord.svm.State;
import com.walmartlabs.concord.svm.ThreadStatus;
import org.slf4j.Logger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
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.UserDefinedException;
import com.walmartlabs.concord.sdk.Constants;
Expand Down Expand Up @@ -100,12 +99,10 @@ public ScriptResult eval(Context context, String language, Reader input, Map<Str
engine.eval(input, b);
return scriptResult;
} catch (ScriptException e) {
if (e.getCause() instanceof PolyglotException) {
if (e.getCause() != null) {
throw new UserDefinedException(e.getCause().getMessage());
}
throw new UserDefinedException(e.getMessage());
} catch (Exception e) {
throw new WrappedException(e);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package com.walmartlabs.concord.runtime.v2.runner.vm;

/*-
* *****
* Concord
* -----
* Copyright (C) 2017 - 2023 Walmart Inc.
* -----
* 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.
* See the License for the specific language governing permissions and
* limitations under the License.
* =====
*/

import com.walmartlabs.concord.runtime.v2.model.Location;
import com.walmartlabs.concord.svm.ThreadError;

import java.io.PrintStream;
import java.io.PrintWriter;
import java.io.Serial;
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;

/**
* An exception that is thrown when multiple exceptions are thrown
* in {@code parallel} blocks.
*/
public class ParallelExecutionException extends RuntimeException {

@Serial
private static final long serialVersionUID = 1L;

private static final int MAX_STACK_TRACE_ELEMENTS = 3;
private final List<ThreadError> exceptions;

public ParallelExecutionException(Collection<ThreadError> causes) {
super("Parallel execution errors: \n" + toMessage(causes));
this.exceptions = new ArrayList<>(causes);
}

public List<Exception> getExceptions() {
return exceptions.stream().map(ThreadError::exception).toList();
}

private static String toMessage(Collection<ThreadError> causes) {
return causes.stream()
.map(ParallelExecutionException::stacktraceToString)
.collect(Collectors.joining("\n"));
}

@Override
public void printStackTrace(PrintStream s) {
s.println(getMessage());
}

@Override
public void printStackTrace(PrintWriter s) {
s.println(getMessage());
}

private static String stacktraceToString(ThreadError e) {
StringWriter sw = new StringWriter();
sw.append(toString(e));

StackTraceElement[] elements = e.getStackTrace();
if (elements.length > 0) {
sw.append("\n");
}

int maxElements = Math.min(elements.length, MAX_STACK_TRACE_ELEMENTS);
for (int i = 0; i < maxElements; i++) {
StackTraceElement element = elements[i];
sw.append("\tat ").append(element.toString()).append("\n");
}
if (maxElements != elements.length) {
sw.append("\t...").append(String.valueOf(elements.length - maxElements)).append(" more\n");
}
return sw.toString();
}

@Override
public String toString() {
return getMessage();
}

@Override
public StackTraceElement[] getStackTrace() {
return new StackTraceElement[0];
}

private static String toString(ThreadError threadError) {
String prefix = "";
if (threadError.cmd() instanceof StepCommand<?> sc) {
prefix = Location.toErrorPrefix(sc.getStep().getLocation()) + ", ";
}
return prefix + "thread: " + threadError.threadId().id() + ": " + threadError.exception();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ 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()), e.getMessage());
log.error("{}. {}", Location.toErrorPrefix(step.getLocation()), e.getMessage());
} else {
log.error("{}", e.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -138,7 +138,7 @@ public Map<ThreadId, String> getEventRefs() {
}

@Override
public Exception getThreadError(ThreadId threadId) {
public ThreadError getThreadError(ThreadId threadId) {
throw new IllegalStateException("Not implemented");
}

Expand All @@ -148,7 +148,12 @@ public void setThreadError(ThreadId threadId, Exception error) {
}

@Override
public Exception clearThreadError(ThreadId threadId) {
public void setThreadError(ThreadId threadId, Command cmd, Exception error) {
throw new IllegalStateException("Not implemented");
}

@Override
public ThreadError clearThreadError(ThreadId threadId) {
throw new IllegalStateException("Not implemented");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class InMemoryState implements Serializable, State {
private final Map<ThreadId, ThreadStatus> threadStatus = new HashMap<>();
private final Map<ThreadId, Set<ThreadId>> children = new HashMap<>();
private final Map<ThreadId, String> eventRefs = new HashMap<>();
private final Map<ThreadId, Exception> threadErrors = new HashMap<>();
private final Map<ThreadId, ThreadError> threadErrors = new HashMap<>();
private final Map<ThreadId, List<StackTraceItem>> stackTrace = new HashMap<>();
private final Map<ThreadId, Map<String, Serializable>> threadLocals = new HashMap<>();

Expand Down Expand Up @@ -206,23 +206,30 @@ public Map<ThreadId, String> getEventRefs() {
}

@Override
public Exception getThreadError(ThreadId threadId) {
public ThreadError getThreadError(ThreadId threadId) {
synchronized (this) {
return threadErrors.get(threadId);
Object result = threadErrors.get(threadId);
return StateBackwardCompatibility.processThreadError(result, threadId);
}
}

@Override
public void setThreadError(ThreadId threadId, Exception error) {
setThreadError(threadId, null, error);
}

@Override
public void setThreadError(ThreadId threadId, Command cmd, Exception error) {
synchronized (this) {
threadErrors.put(threadId, error);
threadErrors.put(threadId, new ThreadError(threadId, cmd, error));
}
}

@Override
public Exception clearThreadError(ThreadId threadId) {
public ThreadError clearThreadError(ThreadId threadId) {
synchronized (this) {
return threadErrors.remove(threadId);
Object result = threadErrors.remove(threadId);
return StateBackwardCompatibility.processThreadError(result, threadId);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
* An exception that is thrown when multiple exceptions are thrown
* in {@code parallel} blocks.
*/
@Deprecated
public class ParallelExecutionException extends RuntimeException {

private static final long serialVersionUID = 1L;
Expand Down
Loading

0 comments on commit 34cbc1d

Please sign in to comment.