TransferManager doesn't handle constraint failures gracefully

[bencampion]

I'm using TransferManager to conditionally copy an object if it has a matching ETag. I would like to be able to identify when a transfer has failed because the constraints weren't met (as opposed to failing for some other reason), but that doesn't appear to be possible due to the way TransferManager handles failing constraints.

I'm copying the object like this:

CopyObjectRequest copyRequest = new CopyObjectRequest(srcBucket, srcKey, destBucket, destKey);
copyRequest.setMatchingETagConstraints(Collections.singletonList(expectedETag));
s3TransferManager.copy(copyRequest).waitForCopyResult();

If the ETag doesn't match then this exception is throw:

com.amazonaws.AmazonClientException: Unable to complete transfer: null
	at com.amazonaws.services.s3.transfer.internal.AbstractTransfer.unwrapExecutionException(AbstractTransfer.java:286) ~[aws-java-sdk-s3-1.11.351.jar!/:na]
	at com.amazonaws.services.s3.transfer.internal.AbstractTransfer.rethrowExecutionException(AbstractTransfer.java:265) ~[aws-java-sdk-s3-1.11.351.jar!/:na]
	at com.amazonaws.services.s3.transfer.internal.CopyImpl.waitForCopyResult(CopyImpl.java:67) ~[aws-java-sdk-s3-1.11.351.jar!/:na]
<SNIP>
Caused by: java.lang.NullPointerException: null
	at com.amazonaws.services.s3.transfer.internal.CopyCallable.copyInOneChunk(CopyCallable.java:154) ~[aws-java-sdk-s3-1.11.351.jar!/:na]
	at com.amazonaws.services.s3.transfer.internal.CopyCallable.call(CopyCallable.java:134) ~[aws-java-sdk-s3-1.11.351.jar!/:na]
	at com.amazonaws.services.s3.transfer.internal.CopyMonitor.call(CopyMonitor.java:132) ~[aws-java-sdk-s3-1.11.351.jar!/:na]
	at com.amazonaws.services.s3.transfer.internal.CopyMonitor.call(CopyMonitor.java:43) ~[aws-java-sdk-s3-1.11.351.jar!/:na]
	at java.util.concurrent.FutureTask.run(FutureTask.java:266) [na:1.8.0_111]
	... 3 common frames omitted

The NullPointerException occurs because the return value from s3.copyObject(copyObjectRequest) (which returns null when constraints aren't met) is not checked before calling copyObjectResult.getETag(): https://github.com/aws/aws-sdk-java/blob/1.11.351/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/internal/CopyCallable.java#L146-L154.

It looks like there may be other parts of the code that aren't checking nullable return values: https://github.com/aws/aws-sdk-java/blob/1.11.351/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/internal/CopyPartCallable.java#L41

I'm not sure what the correct behaviour here should be. I was expecting a AmazonS3Exception with the appropriate status code, but I'm not sure how you'd achieve that as AmazonS3Client is documented to swallow these exceptions and return null. Throwing a AmazonClientException with a message about constraints would be an improvement for debugging, but it's not a good way to programmatically detect the reason the transfer had failed.

[dagnir]

This seems reasonable to me; I think the S3 client should not have been special casing constraint validation failure, but that behavior can't be changed now. I'll go ahead and mark this as a feature request.

[steveloughran]

Note that in the hadoop AWS s3a client we see this problem with version checking and have to catch the NPE and consider that the specific failure. we consider the current behaviour to be a bug

[debora-ito]

@bencampion @steveloughran

the SDK team has reviewed the feature request list for V1, and since they're concentrating efforts on V2 new features they decided to not implement this one in V1. It's still being considered for the TransferManager refactor in V2, see the referenced issue above. I'll go ahead and close this.

Please feel free to comment on the V2 issue with your use case, and reach out if you have further questions.

[steveloughran]

Well, that is your choice and -I recognise the need to prioritise.

Could you at least document this, so other people don't end up encountering the same problem?