Skip to content

[9.0.0] Show stack traces in the remote repo contents cache with --verbose_failures #28179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
meteorcloudy merged 1 commit into from
Jan 7, 2026
+36 −7
Merged

[9.0.0] Show stack traces in the remote repo contents cache with --verbose_failures #28179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,8 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
env.getWorkspaceName(),
remoteOptions.remoteInstanceName,
remoteOptions.remoteAcceptCached,
remoteOptions.remoteUploadLocalResults));
remoteOptions.remoteUploadLocalResults,
verboseFailures));
if (env.getDirectories().getOutputBase().getFileSystem()
instanceof RemoteExternalOverlayFileSystem remoteFs) {
remoteFs.beforeCommand(
Expand Down
33 changes: 29 additions & 4 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteRepoContentsCacheImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import build.bazel.remote.execution.v2.Directory;
import build.bazel.remote.execution.v2.Tree;
import com.google.common.base.Splitter;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.Futures;
Expand Down Expand Up @@ -93,6 +94,7 @@ public final class RemoteRepoContentsCacheImpl implements RemoteRepoContentsCach
private final String commandId;
private final boolean acceptCached;
private final boolean uploadLocalResults;
private final boolean verboseFailures;
private final DigestUtil digestUtil;
private final Action baseAction;

Expand All @@ -101,12 +103,14 @@ public RemoteRepoContentsCacheImpl(
String buildRequestId,
String commandId,
boolean acceptCached,
boolean uploadLocalResults) {
boolean uploadLocalResults,
boolean verboseFailures) {
this.buildRequestId = buildRequestId;
this.commandId = commandId;
this.cache = cache;
this.acceptCached = acceptCached;
this.uploadLocalResults = uploadLocalResults;
this.verboseFailures = verboseFailures;
this.digestUtil = cache.digestUtil;
this.baseAction =
Action.newBuilder()
Expand Down Expand Up @@ -142,7 +146,7 @@ public void addToCache(
reporter.handle(
Event.warn(
"Failed to read marker file repo %s, skipping: %s"
.formatted(repoName, e.getMessage())));
.formatted(repoName, maybeGetStackTrace(e))));
}
var action = buildAction(predeclaredInputHash);
var actionKey = new ActionKey(digestUtil.compute(action));
Expand All @@ -168,7 +172,7 @@ public void addToCache(
reporter.handle(
Event.warn(
"Failed to upload repo contents to remote cache for repo %s: %s"
.formatted(repoName, e.getMessage())));
.formatted(repoName, maybeGetStackTrace(e))));
}
}

Expand All @@ -179,6 +183,22 @@ public boolean lookupCache(
String predeclaredInputHash,
ExtendedEventHandler reporter)
throws IOException, InterruptedException {
try {
return doLookupCache(repoName, repoDir, predeclaredInputHash, reporter);
} catch (IOException e) {
throw new IOException(
"Failed to look up repo %s in the remote repo contents cache: %s"
.formatted(repoName, maybeGetStackTrace(e)),
e);
}
}

private boolean doLookupCache(
RepositoryName repoName,
Path repoDir,
String predeclaredInputHash,
ExtendedEventHandler reporter)
throws IOException, InterruptedException {
if (!(repoDir.getFileSystem() instanceof RemoteExternalOverlayFileSystem remoteFs)) {
return false;
}
Expand Down Expand Up @@ -236,7 +256,8 @@ public boolean lookupCache(
markerFileContent = new String(markerFileContentFuture.get(), StandardCharsets.ISO_8859_1);
repoDirectoryContent = repoDirectoryContentFuture.get();
} catch (ExecutionException e) {
throw new IllegalStateException("waitForBulkTransfer should have thrown", e);
throw new IllegalStateException(
"waitForBulkTransfer should have thrown: " + maybeGetStackTrace(e));
Comment on lines +259 to +260
Copy link

@gemini-code-assist gemini-code-assist bot Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The original ExecutionException e is no longer passed as the cause to the IllegalStateException. This is a regression that loses valuable debugging information, as the original stack trace will be lost. The maybeGetStackTrace(e) method only provides the stack trace as a string in the message when verboseFailures is enabled. It's better to always preserve the original exception as the cause.

Suggested change
throw new IllegalStateException(
"waitForBulkTransfer should have thrown: " + maybeGetStackTrace(e));
throw new IllegalStateException(
"waitForBulkTransfer should have thrown: " + maybeGetStackTrace(e), e);

}
var markerFileLines =
Splitter.on('\n')
Expand Down Expand Up @@ -285,6 +306,10 @@ private Action buildAction(String predeclaredInputHash) {
.build();
}

private String maybeGetStackTrace(Exception e) {
return verboseFailures ? Throwables.getStackTraceAsString(e) : e.getMessage();
}

private record RepoRemotePathResolver(Path fetchedRepoMarkerFile, Path fetchedRepoDir)
implements RemotePathResolver {

Expand Down
7 changes: 5 additions & 2 deletions src/main/java/com/google/devtools/build/lib/remote/RepositoryRemoteHelpersFactoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class RepositoryRemoteHelpersFactoryImpl implements RepositoryRemoteHelpersFacto
private final String remoteInstanceName;
private final boolean acceptCached;
private final boolean uploadLocalResults;
private final boolean verboseFailures;

RepositoryRemoteHelpersFactoryImpl(
CombinedCache cache,
Expand All @@ -40,7 +41,8 @@ class RepositoryRemoteHelpersFactoryImpl implements RepositoryRemoteHelpersFacto
String workspaceName,
String remoteInstanceName,
boolean acceptCached,
boolean uploadLocalResults) {
boolean uploadLocalResults,
boolean verboseFailures) {
this.cache = cache;
this.remoteExecutor = remoteExecutor;
this.buildRequestId = buildRequestId;
Expand All @@ -49,6 +51,7 @@ class RepositoryRemoteHelpersFactoryImpl implements RepositoryRemoteHelpersFacto
this.remoteInstanceName = remoteInstanceName;
this.acceptCached = acceptCached;
this.uploadLocalResults = uploadLocalResults;
this.verboseFailures = verboseFailures;
}

@Nullable
Expand All @@ -72,6 +75,6 @@ public RepositoryRemoteExecutor createExecutor() {
@Override
public RemoteRepoContentsCache createRepoContentsCache() {
return new RemoteRepoContentsCacheImpl(
cache, buildRequestId, commandId, acceptCached, uploadLocalResults);
cache, buildRequestId, commandId, acceptCached, uploadLocalResults, verboseFailures);
}
}
Loading