Skip to content

Conversation

@bazel-io
Copy link
Member

@bazel-io bazel-io commented Jan 7, 2026

Makes it easier to debug issues with this experimental feature and also matches the behavior of remote execution/caching.

Work towards #27965

Closes #27970.

PiperOrigin-RevId: 853238791
Change-Id: Id46ccbb105d93fd17114fab13b086d0b46139fb4

Commit fc5f160

…ailures`

Makes it easier to debug issues with this experimental feature and also matches the behavior of remote execution/caching.

Work towards bazelbuild#27965

Closes bazelbuild#27970.

PiperOrigin-RevId: 853238791
Change-Id: Id46ccbb105d93fd17114fab13b086d0b46139fb4
@bazel-io bazel-io requested a review from a team as a code owner January 7, 2026 14:48
@bazel-io bazel-io added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Jan 7, 2026
@bazel-io bazel-io requested a review from Wyverald January 7, 2026 14:48
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds the ability to show full stack traces for failures in the remote repository contents cache when --verbose_failures is enabled. This improves debuggability. The changes involve plumbing the verboseFailures flag down to RemoteRepoContentsCacheImpl and using it to decide whether to log the full stack trace or just the exception message. My review found one issue where an exception cause was unintentionally dropped, which would hinder debugging.

Comment on lines +259 to +260
throw new IllegalStateException(
"waitForBulkTransfer should have thrown: " + maybeGetStackTrace(e));

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);

@meteorcloudy meteorcloudy added this pull request to the merge queue Jan 7, 2026
Merged via the queue into bazelbuild:release-9.0.0 with commit f6a0f69 Jan 7, 2026
46 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Remote-Exec Issues and PRs for the Execution (Remote) team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants