From 997be1ea6fa289d350230ce4fdfbcb240efaf0e0 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Mon, 22 May 2023 02:57:37 -0700 Subject: [PATCH] Enrich local BEP upload errors with file path and digest possible. Closes #18461. PiperOrigin-RevId: 534006853 Change-Id: I256297fe494393f13e178bf74f967b9b691db281 --- .../ByteStreamBuildEventArtifactUploader.java | 20 ++++++++++++------- ...eStreamBuildEventArtifactUploaderTest.java | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java index 8eab0e6666479f..31b3ba6bf5cf7b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java @@ -256,7 +256,7 @@ private Single> queryRemoteCache( return toSingle(() -> remoteCache.findMissingDigests(context, digestsToQuery), executor) .onErrorResumeNext( error -> { - reporterUploadError(error); + reportUploadError(error, null, null); // Assuming all digests are missing if failed to query return Single.just(ImmutableSet.copyOf(digestsToQuery)); }) @@ -267,13 +267,19 @@ private Single> queryRemoteCache( }); } - private void reporterUploadError(Throwable error) { + private void reportUploadError(Throwable error, Path path, Digest digest) { if (error instanceof CancellationException) { return; } - String errorMessage = - "Uploading BEP referenced local files: " + grpcAwareErrorMessage(error, verboseFailures); + String errorMessage = "Uploading BEP referenced local file"; + if (path != null) { + errorMessage += " " + path; + } + if (digest != null) { + errorMessage += " " + digest; + } + errorMessage += ": " + grpcAwareErrorMessage(error, verboseFailures); reporter.handle(Event.warn(errorMessage)); } @@ -298,11 +304,11 @@ private Single> uploadLocalFiles( path.isDirectory(), // set remote to true so the PathConverter will use bytestream:// // scheme to convert the URI for this file - /*remote=*/ true, + /* remote= */ true, path.isOmitted())) .onErrorResumeNext( error -> { - reporterUploadError(error); + reportUploadError(error, path.getPath(), path.getDigest()); return Single.just(path); }); }) @@ -329,7 +335,7 @@ private Single upload(Set files) { try { return readPathMetadata(file); } catch (IOException e) { - reporterUploadError(e); + reportUploadError(e, file, null); return new PathMetadata( file, /* digest= */ null, diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java index 4cbca9b6ece401..9c90f6dd1d2564 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java @@ -334,7 +334,7 @@ public void onCompleted() { assertThat(eventHandler.getEvents()).isNotEmpty(); assertThat(eventHandler.getEvents().get(0).getMessage()) - .contains("Uploading BEP referenced local files: "); + .contains("Uploading BEP referenced local file /file"); artifactUploader.release();