Skip to content

Improve error message when checksum validation fails. Make such failures retryable. #2798

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
merged 1 commit into from
Oct 26, 2021
Merged

Improve error message when checksum validation fails. Make such failures retryable. #2798

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
9 changes: 5 additions & 4 deletions ...main/java/software/amazon/awssdk/services/s3/checksums/ChecksumValidatingInputStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import java.util.Arrays;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.checksums.SdkChecksum;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.core.exception.RetryableException;
import software.amazon.awssdk.http.Abortable;
import software.amazon.awssdk.utils.BinaryUtils;

Expand Down Expand Up @@ -169,9 +169,10 @@ private void validateAndThrow() {
}

if (!Arrays.equals(computedChecksum, streamChecksum)) {
throw SdkClientException.builder().message(
String.format("Data read has a different checksum than expected. Was 0x%s, but expected 0x%s",
BinaryUtils.toHex(computedChecksum), BinaryUtils.toHex(streamChecksum))).build();
throw RetryableException.create(
String.format("Data read has a different checksum than expected. Was 0x%s, but expected 0x%s. " +
"This commonly means that the data was corrupted between the client and " +
"service.", BinaryUtils.toHex(computedChecksum), BinaryUtils.toHex(streamChecksum)));
}
}

Expand Down
11 changes: 8 additions & 3 deletions ...c/main/java/software/amazon/awssdk/services/s3/checksums/ChecksumValidatingPublisher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.async.SdkPublisher;
import software.amazon.awssdk.core.checksums.SdkChecksum;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.core.exception.RetryableException;
import software.amazon.awssdk.utils.BinaryUtils;

@SdkInternalApi
Expand Down Expand Up @@ -134,8 +134,13 @@ public void onComplete() {
if (strippedLength > 0) {
byte[] computedChecksum = sdkChecksum.getChecksumBytes();
if (!Arrays.equals(computedChecksum, streamChecksum)) {
onError(SdkClientException.create(
String.format("Data read has a different checksum than expected. Was 0x%s, but expected 0x%s",
onError(RetryableException.create(
String.format("Data read has a different checksum than expected. Was 0x%s, but expected 0x%s. "
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Out of scope of this change, but also consider "expected X, but was Y" -- this follows the traditional unit test style wording.

+ "Common causes: (1) You modified a request ByteBuffer before it could be "
+ "written to the service. Please ensure your data source does not modify the "
+ " byte buffers after you pass them to the SDK. (2) The data was corrupted between the "
+ "client and service. Note: Despite this error, the upload still completed and was "
+ "persisted in S3.",
BinaryUtils.toHex(computedChecksum), BinaryUtils.toHex(streamChecksum))));
return; // Return after onError and not call onComplete below
}
Expand Down
8 changes: 5 additions & 3 deletions ...src/main/java/software/amazon/awssdk/services/s3/checksums/ChecksumsEnabledValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import software.amazon.awssdk.core.ClientType;
import software.amazon.awssdk.core.SdkRequest;
import software.amazon.awssdk.core.checksums.SdkChecksum;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.core.exception.RetryableException;
import software.amazon.awssdk.core.interceptor.ExecutionAttribute;
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute;
Expand Down Expand Up @@ -144,8 +144,10 @@ public static void validatePutObjectChecksum(PutObjectResponse response, Executi
byte[] ssHash = Base16Lower.decode(response.eTag().replace("\"", ""));

if (!Arrays.equals(digest, ssHash)) {
throw SdkClientException.create(
String.format("Data read has a different checksum than expected. Was 0x%s, but expected 0x%s",
throw RetryableException.create(
String.format("Data read has a different checksum than expected. Was 0x%s, but expected 0x%s. " +
"This commonly means that the data was corrupted between the client and " +
"service. Note: Despite this error, the upload still completed and was persisted in S3.",
BinaryUtils.toHex(digest), BinaryUtils.toHex(ssHash)));
}
}
Expand Down