Improve error message when checksum validation fails. Make such failures retryable. #2798
Conversation
| String.format("Data read has a different checksum than expected. Was 0x%s, but expected 0x%s. " | ||
| + "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 you pass to the SDK. (2) The data was corrupted between the client and " |
There was a problem hiding this comment.
Nit: Should we clarify to not modify after passing to the SDK?
.../src/main/java/software/amazon/awssdk/services/s3/checksums/ChecksumValidatingPublisher.java
Show resolved
Hide resolved
| 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. Please retry your request.", |
There was a problem hiding this comment.
We have 3 slightly different wordings for this. Should they all be the same? Should we vend a consistent error message from a util method?
There was a problem hiding this comment.
There's 2 wordings. One includes the byte buffer being modified because that's a potential cause. The other two places do not have this risk, so it excludes that as a potential cause.
There was a problem hiding this comment.
Got it. As a nit: one wording said to "please retry", and the other did not.
There was a problem hiding this comment.
Ah, good catch! I meant to remove "please retry", because we should just retry instead of asking them to do it.
| 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. " |
There was a problem hiding this comment.
Nit: Out of scope of this change, but also consider "expected X, but was Y" -- this follows the traditional unit test style wording.
millems
force-pushed
the
millem/improve-checksum-exception
branch
2 times, most recently
from
eb80efe to
947d576
Compare
millems
force-pushed
the
millem/improve-checksum-exception
branch
from
947d576 to
a5de2cd
Compare
|
Kudos, SonarCloud Quality Gate passed! |
No description provided.