Provide more accurate ExitCode and FailureDetails when an exception is caught during the loading phase.
In each failure instance of PackageFunctionException, a specific FailureDetail code will be used to describe the exception. As the error gets bubbled up, we have to make sure the information is not lost and is properly passed up the call chain.
RELNOTES: None.
PiperOrigin-RevId: 318929283
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index c3a338d..d17899c 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -263,6 +263,7 @@
"//src/main/java/com/google/devtools/build/lib/packages:starlark_semantics_options",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness",
"//src/main/java/com/google/devtools/build/lib/skyframe:pattern_expanding_error",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
@@ -273,6 +274,8 @@
"//src/main/java/com/google/devtools/build/lib/syntax:frontend",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
@@ -281,6 +284,7 @@
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/common/options",
"//src/main/java/com/google/devtools/common/options:invocation_policy",
+ "//src/main/protobuf:failure_details_java_proto",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//src/test/java/com/google/devtools/build/lib/analysis/util:test-build-options",
"//src/test/java/com/google/devtools/build/lib/packages:testutil",
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 9ad28f9..88ffeb1 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -50,7 +50,9 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.util.MockToolsConfig;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
+import com.google.devtools.build.lib.skyframe.DetailedTargetParsingException;
import com.google.devtools.build.lib.skyframe.PatternExpandingError;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
@@ -58,6 +60,8 @@
import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.testutil.SkyframeExecutorTestHelper;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
@@ -1136,10 +1140,14 @@
assertThat(value.hasError()).isTrue();
tester.assertContainsWarning("Target pattern parsing failed");
} else {
- TargetParsingException exn =
- assertThrows(TargetParsingException.class, () -> tester.load(patterns));
+ DetailedTargetParsingException exn =
+ assertThrows(DetailedTargetParsingException.class, () -> tester.load(patterns));
assertThat(exn).hasCauseThat().isInstanceOf(BuildFileContainsErrorsException.class);
assertThat(exn).hasCauseThat().hasMessageThat().contains("Extension 'bad/f1.bzl' has errors");
+ DetailedExitCode detailedExitCode = exn.getDetailedExitCode();
+ assertThat(detailedExitCode.getExitCode()).isEqualTo(ExitCode.BUILD_FAILURE);
+ assertThat(detailedExitCode.getFailureDetail().getPackageLoading().getCode())
+ .isEqualTo(PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
tester.assertContainsError("/workspace/bad/f1.bzl:1:1: name 'nope' is not defined");
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
index d530430..80f22a8 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -180,6 +180,7 @@
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_progress_receiver",
"//src/main/java/com/google/devtools/build/lib/skyframe:containing_package_lookup_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:containing_package_lookup_value",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness",
"//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness_manager",
"//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_function",
@@ -239,6 +240,7 @@
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:crash_failure_details",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/java/com/google/devtools/build/lib/util/io:out-err",
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index 4412a28..8b3a2b2 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -45,9 +45,12 @@
import com.google.devtools.build.lib.pkgcache.PackageOptions;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.MoreAsserts;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.Dirent;
@@ -308,6 +311,8 @@
String errorMessage = errorInfo.getException().getMessage();
assertThat(errorMessage).contains("Inconsistent filesystem operations");
assertThat(errorMessage).contains(expectedMessage);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR);
}
@Test
@@ -344,6 +349,8 @@
String errorMessage = errorInfo.getException().getMessage();
assertThat(errorMessage).contains("Inconsistent filesystem operations");
assertThat(errorMessage).contains(expectedMessage);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR);
}
/** Regression test for unexpected exception type from PackageValue. */
@@ -372,6 +379,8 @@
String errorMessage = errorInfo.getException().getMessage();
assertThat(errorMessage).contains("Inconsistent filesystem operations");
assertThat(errorMessage).contains(expectedMessage);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.TRANSIENT_INCONSISTENT_FILESYSTEM_ERROR);
}
@SuppressWarnings("unchecked") // Cast of srcs attribute to Iterable<Label>.
@@ -617,6 +626,8 @@
"error loading package 'test/starlark': "
+ "cannot load '//test/starlark:bad_extension.bzl': no such file";
assertThat(errorInfo.getException()).hasMessageThat().isEqualTo(expectedMsg);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
@Test
@@ -646,6 +657,8 @@
"error loading package 'test/starlark': "
+ "in /workspace/test/starlark/extension.bzl: "
+ "cannot load '//test/starlark:bad_extension.bzl': no such file");
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
@Test
@@ -673,6 +686,8 @@
.isEqualTo(
"error loading package 'test/starlark': Encountered error while reading extension "
+ "file 'test/starlark/extension.bzl': Symlink cycle");
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
@Test
@@ -855,6 +870,7 @@
assertThat(errorMessage).contains("nope");
assertThat(errorInfo.getException()).isInstanceOf(NoSuchPackageException.class);
assertThat(errorInfo.getException()).hasCauseThat().isInstanceOf(IOException.class);
+ assertDetailedExitCode(errorInfo.getException(), PackageLoading.Code.BUILD_FILE_MISSING);
}
@Test
@@ -874,6 +890,8 @@
assertThat(errorMessage).contains("nope");
assertThat(errorInfo.getException()).isInstanceOf(NoSuchPackageException.class);
assertThat(errorInfo.getException()).hasCauseThat().isInstanceOf(IOException.class);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
@Test
@@ -1309,6 +1327,15 @@
// presence of a symlink cycle encountered during glob evaluation.
}
+ private static void assertDetailedExitCode(
+ Exception exception, PackageLoading.Code expectedPackageLoadingCode) {
+ assertThat(exception).isInstanceOf(DetailedException.class);
+ DetailedExitCode detailedExitCode = ((DetailedException) exception).getDetailedExitCode();
+ assertThat(detailedExitCode.getExitCode()).isEqualTo(ExitCode.BUILD_FAILURE);
+ assertThat(detailedExitCode.getFailureDetail().getPackageLoading().getCode())
+ .isEqualTo(expectedPackageLoadingCode);
+ }
+
private static class CustomInMemoryFs extends InMemoryFileSystem {
private abstract static class FileStatusOrException {
abstract FileStatus get() throws IOException;
