Skip to content

Conversation

@visz11
Copy link
Collaborator

@visz11 visz11 commented Apr 15, 2025

The downloads in AbstractActionInputPrefetcher#prefetchInputs reported lost inputs that are symlinks to other artifacts with the exec path of the target, which results in a crash when the target is not also an input to the action. This is fixed by always reporting the exec path of the symlink.

Fixes bazelbuild#25841

fmeum and others added 3 commits April 15, 2025 04:44
The downloads in `AbstractActionInputPrefetcher#prefetchInputs` reported lost inputs that are symlinks to other artifacts with the exec path of the target, which results in a crash when the target is not also an input to the action. This is fixed by always reporting the exec path of the symlink.

Fixes bazelbuild#25841

Closes bazelbuild#25847.

PiperOrigin-RevId: 747819887
Change-Id: Iff9125c95a2598851b8a31ed4ca18fd7218a97aa
The downloads in AbstractActionInputPrefetcher#prefetchInputs reported lost inputs that are symlinks to other artifacts with the exec path of the target, which results in a crash when the target is not also an input to the action. This is fixed by always reporting the exec path of the symlink.

Fixes bazelbuild#25841
@arvi18
Copy link

arvi18 commented Aug 10, 2025

/refacto-test

@refacto-test
Copy link

refacto-test bot commented Aug 10, 2025

Refacto is reviewing this PR. Please wait for the review comments to be posted.

@refacto-test
Copy link

refacto-test bot commented Aug 10, 2025

Code Review: Symlink Error Handling Enhancement

👍 Well Done
Improved Error Reporting

Enhanced error handling for symlinked inputs improves failure recovery and diagnostics.

Path Information Preservation

Setting exec path ensures accurate error reporting for rewinding.

📝 Additional Comments
src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java (1)
Error Propagation Enhancement

Error handler doesn't log the exception before propagating. Missing logging reduces observability of cache misses affecting symlinks, making troubleshooting more difficult during production incidents.

              .onErrorResumeNext(
                  t -> {
                    if (t instanceof CacheNotFoundException cacheNotFoundException) {
                      // Only the symlink itself is guaranteed to be an input to the action, so
                      // report its path for rewinding.
                      cacheNotFoundException.setExecPath(input.getExecPath());
                      logger.atFine().withCause(cacheNotFoundException)
                          .log("Cache miss for symlinked input %s", input.getExecPath());
                      return Completable.error(cacheNotFoundException);
                    }
                    return Completable.error(t);
                  });

Standards:

  • ISO-IEC-25010-Reliability-Recoverability
  • SRE-Observability

metadata,
priority,
reason);
action,
Copy link

Choose a reason for hiding this comment

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

Missing ExecPath Propagation

Cache errors lack proper path information for symlinked inputs. Without ExecPath propagation, action rewinding fails to identify missing files correctly, potentially causing build failures.

Suggested change
action,
downloadFileNoCheckRx(
action,
execRoot.getRelative(execPath),
treeRootExecPath != null ? execRoot.getRelative(treeRootExecPath) : null,
dirsWithOutputPermissions,
input,
metadata,
priority,
reason)
.onErrorResumeNext(
t -> {
if (t instanceof CacheNotFoundException cacheNotFoundException) {
// Only the symlink itself is guaranteed to be an input to the action, so
// report its path for rewinding.
cacheNotFoundException.setExecPath(input.getExecPath());
return Completable.error(cacheNotFoundException);
}
return Completable.error(t);
})
Standards
  • CWE-755
  • OWASP-A06

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AbstractParallelEvaluator RuntimeException in 8.2.0

3 participants