Aws-sdk-java: Error logging with TransferManager

Created on 9 Jul 2017  路  12Comments  路  Source: aws/aws-sdk-java

It seems to be impossible to get exceptions from AWS when using the TransferManager to upload files to S3, without blocking the thread. See the following code:

public class S3Uploader {
    Upload upload;

    public static void main(String[] args) {
        S3Uploader uploader = new S3Uploader();
        uploader.upload("sample.txt");
    }

    public void upload(String fileName) {
        File file = new File(fileName);

        String bucketName = "bucket";

        ProfileCredentialsProvider credentialProvider = new ProfileCredentialsProvider();

        AmazonS3 s3 = AmazonS3ClientBuilder.standard()
                .withCredentials(credentialProvider)
                .withRegion(Regions.EU_WEST_2)
                .build();

        TransferManager tx = TransferManagerBuilder.standard()
                .withS3Client(s3)
                .build();

        upload = tx.upload(bucketName, file.getName(), file);
        upload.addProgressListener(new FailureListener());
    }

    public class FailureListener extends SyncProgressListener {
        public void progressChanged(ProgressEvent progressEvent) {
            if (progressEvent.getEventType() == ProgressEventType.TRANSFER_FAILED_EVENT) {
                try {
                    AmazonClientException e = upload.waitForException();
                    e.printStackTrace();
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
        }
    }
}

I'm imagining that the expected behaviour is for the stack trace to be printed to the log. Instead, the line AmazonClientException e = upload.waitForException() is reached and then the program gets stuck waiting for the Future to return. I'm not sure if this is a bug or my misunderstanding of the SDK, though.

feature-request

Most helpful comment

馃憤 - this is a major pain point for my TransferManager use cases.

All 12 comments

Wait for exception will explicitly block and wait for the transfer to finish before returning back any error that was encountered.

What are you looking to do? Do you want to log exceptions as they come up at any point during the transfer?

The idea is to log exceptions as they come up, yes. Calling waitForException in the main thread where we call tx.upload works absolutely fine, of course. However, calling it in the progress listener doesn't seem to work anymore (I believe it to have worked in past versions).

I'm looking for a way to log exceptions from the TransferManager without blocking the thread (as far as is possible).

You can provide your own ProgressListener that logs on events. Other than that, exceptions won't be thrown until the upload has completed.

But the only information you can get from a progress listener as-is is that some error occurred with the transfer, not what kind of error occurred. In previous versions of the SDK it was possible to call waitForException() in the progress listener to get the actual error message. Now, if you call waitForException() in a progress listener at any point, it blocks the thread completely - it does not return even when the upload is completed. No further events pass through the progress listener.

@spfink, shouldn't the exception be available when we get the ProgressEvent?
That way, we don't have to depend on the upload result.
Or any other better way of doing it..

I feel progress listeners will be in general used as an alternative to "upload" as people don't want to wait for the upload completion synchronously and get the exception details etc.
Thats the whole point of transfer manager I thought.
Calling TransferManager to upload and waiting for exception or completion synchronously makes it ordinary except the multi-part upload.

In that point of view, I feel progress listeners are weak and are not yet there.
Let me know you comments and thoughts

From @Josh-ES's use-case here, he wants to get the error/exception that happened using ProgressListener so that he doesn't have to wait synchronously for the exception/result/completion. He is using a SyncProgressListener which will block the current thread that is uploading the object. I can see from AmazonS3Client that the FAILED event is getting published here and here.

But the exceptions are being thrown after the publishing of event and hence in progressListener if u do upload.waitForUploadResult/Completion/Exception, it will keep blocking the thread that's uploading the object and hence you will never get the exception/result. (Deadlock)

This is applicable for all events and its pretty clear from code that we can never do upload.waitForUploadResult/waitForCompletion/waitForException from ProgressListener if its a synchronous one (Alternative is Asynchronous ProgressListener but its recommended to have a SyncProgressListener .

Isn't this contradictory/a limitation? Is there an alternative way of getting exception using or from ProgressListener?

Anyway, adding an additional field "ExceptionOccurred" in ProgressEvent would solve this issue temporary. But I feel that as part of SDK 2.0, we should support the generic result object in the event or come up with a different completion event listener thats more powerful

Any updates here?

@spfink

馃憤 - this is a major pain point for my TransferManager use cases.

This is definitely something that we will pull into 2.0 when we rewrite progress listeners so if you have any ideas, definitely let us know on that repo.

As far as v1 goes, we could add an exception occurred field to the progress listener but not likely we will be able to support upload.waitForException within the progress listeners. I'll look into the scope of this change this week. Easy enough for S3 but we use the progress listeners in more places and will need to be consistent.

Curious if there was any progress on this? This has come up for us recently. As far as I can tell, there's no way to capture the exception in a non-blocking manner.

I honestly would find it preferable to just expose a future (after all, a future appears to be used internally to monitor progress). Passing the exception to the progress listener would definitely be a decent compromise though.

@Josh-ES @AadithyaU @jarwol @jalaziz

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

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

Was this page helpful?
0 / 5 - 0 ratings