Skip to content

Commit e5db377

Browse files
authored
Do not retry NoSuchFileException and AccessDeniedException (#5470)
* Update ResponseTransformer so download attempts to a directory that does not exist or does not have write permissions are not retried * Update ResponseTransformer so download attempts to a directory that does not exist or does not have write permissions are not retried * Update ResponseTransformer so download attempts to a directory that does not exist or does not have write permissions are not retried * Update ResponseTransformer so download attempts to a directory that does not exist or does not have write permissions are not retried
1 parent 881c0d9 commit e5db377

File tree

3 files changed

+35
-5
lines changed

3 files changed

+35
-5
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Update ResponseTransformer so download attempts to a directory that does not exist or does not have write permissions are not retried"
6+
}

core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/ResponseTransformer.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
import java.io.InputStream;
2121
import java.io.OutputStream;
2222
import java.nio.ByteBuffer;
23+
import java.nio.file.AccessDeniedException;
2324
import java.nio.file.DirectoryNotEmptyException;
2425
import java.nio.file.FileAlreadyExistsException;
2526
import java.nio.file.Files;
27+
import java.nio.file.NoSuchFileException;
2628
import java.nio.file.Path;
2729
import software.amazon.awssdk.annotations.SdkPublicApi;
2830
import software.amazon.awssdk.core.ResponseBytes;
@@ -108,8 +110,7 @@ static <ResponseT> ResponseTransformer<ResponseT, ResponseT> toFile(Path path) {
108110
} catch (IOException copyException) {
109111
String copyError = "Failed to read response into file: " + path;
110112

111-
// If the write failed because of the state of the file, don't retry the request.
112-
if (copyException instanceof FileAlreadyExistsException || copyException instanceof DirectoryNotEmptyException) {
113+
if (shouldThrowIOException(copyException)) {
113114
throw new IOException(copyError, copyException);
114115
}
115116

@@ -133,6 +134,17 @@ static <ResponseT> ResponseTransformer<ResponseT, ResponseT> toFile(Path path) {
133134
};
134135
}
135136

137+
/**
138+
* If the write failed because of the state of the file, if the specified directory doesn't exist, or if the specified
139+
* directory does not have write permissions, don't retry the request.
140+
*/
141+
static boolean shouldThrowIOException(IOException copyException) {
142+
return copyException instanceof FileAlreadyExistsException ||
143+
copyException instanceof DirectoryNotEmptyException ||
144+
copyException instanceof NoSuchFileException ||
145+
copyException instanceof AccessDeniedException;
146+
}
147+
136148
/**
137149
* Creates a response transformer that writes all response content to the specified file. If the file already exists
138150
* then a {@link java.nio.file.FileAlreadyExistsException} will be thrown.

test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/ResponseTransformerTest.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.junit.Test;
4040
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
4141
import software.amazon.awssdk.core.ResponseBytes;
42+
import software.amazon.awssdk.core.exception.RetryableException;
4243
import software.amazon.awssdk.core.exception.SdkClientException;
4344
import software.amazon.awssdk.core.sync.ResponseTransformer;
4445
import software.amazon.awssdk.http.apache.ApacheHttpClient;
@@ -103,17 +104,28 @@ public void downloadToFileRetriesCorrectly() throws IOException {
103104
}
104105

105106
@Test
106-
public void downloadToExistingFileDoesNotRetry() throws IOException {
107+
public void downloadToExistingFileDoesNotRetry() {
107108
stubForRetriesTimeoutReadingFromStreams();
108109

109110
assertThatThrownBy(() -> testClient().streamingOutputOperation(StreamingOutputOperationRequest.builder().build(),
110111
ResponseTransformer
111112
.toFile(new File(".."))))
112-
.isInstanceOf(SdkClientException.class);
113+
.isInstanceOf(SdkClientException.class)
114+
.isNotInstanceOf(RetryableException.class);
115+
}
116+
117+
@Test
118+
public void downloadToNonExistentDirectoryDoesNotRetry() {
119+
stubForRetriesTimeoutReadingFromStreams();
120+
121+
assertThatThrownBy(() -> testClient().streamingOutputOperation(StreamingOutputOperationRequest.builder().build(),
122+
ResponseTransformer.toFile(new File("/nonExistentDir/myFile"))))
123+
.isInstanceOf(SdkClientException.class)
124+
.isNotInstanceOf(RetryableException.class);
113125
}
114126

115127
@Test
116-
public void downloadToOutputStreamDoesNotRetry() throws IOException {
128+
public void downloadToOutputStreamDoesNotRetry() {
117129
stubForRetriesTimeoutReadingFromStreams();
118130

119131
assertThatThrownBy(() -> testClient().streamingOutputOperation(StreamingOutputOperationRequest.builder().build(),

0 commit comments

Comments
 (0)