From e4f3d4ddfb54419bd5bb793a29b83ff26c82d9ce Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 28 Apr 2023 04:35:19 -0700 Subject: [PATCH] Fix message generation of `ActionExecutionException` This fixes an issue introduced by commit f95b80d166eb46ba4e4c0fb4b998f6e12206ee17, which resulted in certain exception message being printed twice. Closes #18243. PiperOrigin-RevId: 527848438 Change-Id: Ic0f7a4a0e3bdf07c1c520647dbb4b41d29e05648 --- .../lib/actions/ActionExecutionException.java | 12 +---- .../lib/skyframe/SkyframeActionExecutor.java | 6 ++- .../actions/ActionExecutionExceptionTest.java | 45 ------------------- .../google/devtools/build/lib/actions/BUILD | 1 - 4 files changed, 7 insertions(+), 57 deletions(-) delete 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 d81adfd419c5de..613a15187a6d47 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(combineMessages(message, cause), cause); + super(message, cause); this.action = action; this.catastrophe = catastrophe; this.detailedExitCode = checkNotNull(detailedExitCode); @@ -96,7 +96,7 @@ public ActionExecutionException( NestedSet rootCauses, boolean catastrophe, DetailedExitCode detailedExitCode) { - super(combineMessages(message, cause), cause); + super(message, cause); this.action = action; this.rootCauses = rootCauses; this.catastrophe = catastrophe; @@ -203,12 +203,4 @@ 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/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 9c25d0e568748e..6b37fd0dd102c2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -1593,7 +1593,11 @@ private ActionExecutionException toActionExecutionException( } else { ex = new ActionExecutionException(message, cause, action, false, code); } - printError(ex.getMessage(), action, actionOutput); + String reportMessage = ex.getMessage(); + if (cause != null && cause.getMessage() != null) { + reportMessage += ": " + cause.getMessage(); + } + printError(reportMessage, action, actionOutput); return ex; } 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 deleted file mode 100644 index 8fc50ff9e2f20b..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/actions/ActionExecutionExceptionTest.java +++ /dev/null @@ -1,45 +0,0 @@ -// 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 802d962ee57000..4820e181885906 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/test/java/com/google/devtools/build/lib/actions/BUILD @@ -64,7 +64,6 @@ 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",