From f95b80d166eb46ba4e4c0fb4b998f6e12206ee17 Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Fri, 21 Apr 2023 19:55:04 -0700
Subject: [PATCH] Include cause when reporting `ActionExecutionException`

`SkyframeActionExecutor#toActionExecutionException` claimed to combine the user-provided message and the exception's message when reporting an error, but did not.

This is fixed so that errors can be diagnosed directly from the build logs, without having to look into `java.log`.

Work towards #10363

Closes #18169.

PiperOrigin-RevId: 526195991
Change-Id: I978a6d739c37384121acccccf95e8dcb80ac5d25
---
 .../lib/actions/ActionExecutionException.java | 12 ++++-
 .../actions/ActionExecutionExceptionTest.java | 45 +++++++++++++++++++
 .../google/devtools/build/lib/actions/BUILD   |  1 +
 3 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100644 src/test/java/com/google/devtools/build/lib/actions/ActionExecutionExceptionTest.java

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
index 613a15187a6d47..d81adfd419c5de 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
@@ -57,7 +57,7 @@ public ActionExecutionException(
       ActionAnalysisMetadata action,
       boolean catastrophe,
       DetailedExitCode detailedExitCode) {
-    super(message, cause);
+    super(combineMessages(message, cause), cause);
     this.action = action;
     this.catastrophe = catastrophe;
     this.detailedExitCode = checkNotNull(detailedExitCode);
@@ -96,7 +96,7 @@ public ActionExecutionException(
       NestedSet<Cause> rootCauses,
       boolean catastrophe,
       DetailedExitCode detailedExitCode) {
-    super(message, cause);
+    super(combineMessages(message, cause), cause);
     this.action = action;
     this.rootCauses = rootCauses;
     this.catastrophe = catastrophe;
@@ -203,4 +203,12 @@ public DetailedExitCode getDetailedExitCode() {
   public boolean showError() {
     return getMessage() != null;
   }
+
+  @Nullable
+  private static String combineMessages(String message, @Nullable Throwable cause) {
+    if (cause == null || cause.getMessage() == null) {
+      return message;
+    }
+    return message + ": " + cause.getMessage();
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionExecutionExceptionTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionExecutionExceptionTest.java
new file mode 100644
index 00000000000000..8fc50ff9e2f20b
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/actions/ActionExecutionExceptionTest.java
@@ -0,0 +1,45 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// 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.
+
+package com.google.devtools.build.lib.actions;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
+import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
+import com.google.devtools.build.lib.server.FailureDetails.Execution;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** {@link ActionExecutionException}Test */
+@RunWith(JUnit4.class)
+public final class ActionExecutionExceptionTest {
+
+  @Test
+  public void containsCauseMessage() {
+    Exception e =
+        new ActionExecutionException(
+            "message",
+            new Exception("cause"),
+            new DummyAction(ActionsTestUtil.DUMMY_ARTIFACT, ActionsTestUtil.DUMMY_ARTIFACT),
+            false,
+            DetailedExitCode.of(
+                FailureDetail.newBuilder().setExecution(Execution.getDefaultInstance()).build()));
+    assertThat(e).hasMessageThat().contains("message");
+    assertThat(e).hasMessageThat().contains("cause");
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/BUILD b/src/test/java/com/google/devtools/build/lib/actions/BUILD
index 4820e181885906..802d962ee57000 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/actions/BUILD
@@ -64,6 +64,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils:depsutils",
         "//src/main/java/com/google/devtools/build/lib/util",
+        "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
         "//src/main/java/com/google/devtools/build/lib/util:filetype",
         "//src/main/java/com/google/devtools/build/lib/util:string",
         "//src/main/java/com/google/devtools/build/lib/vfs",