Skip to content

Commit ea957e0

Browse files
neozwuneozwu
authored
fix retry logic on HttpResponseException (#1677)
* fix retry logic on HttpResponseException (#1545)
1 parent c81077a commit ea957e0

3 files changed

Lines changed: 85 additions & 18 deletions

File tree

‎google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryExceptionTest.java‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@
2626
import static org.junit.Assert.assertSame;
2727
import static org.junit.Assert.assertTrue;
2828

29+
import com.google.api.client.http.HttpHeaders;
30+
import com.google.api.client.http.HttpResponseException;
2931
import com.google.cloud.BaseServiceException;
3032
import com.google.cloud.RetryHelper.RetryHelperException;
31-
32-
import org.junit.Test;
33-
3433
import java.io.IOException;
3534
import java.net.SocketTimeoutException;
35+
import org.junit.Test;
3636

3737
public class BigQueryExceptionTest {
3838

@@ -112,6 +112,34 @@ public void testBigqueryException() {
112112
assertTrue(exception.isRetryable());
113113
assertTrue(exception.isIdempotent());
114114
assertSame(cause, exception.getCause());
115+
116+
117+
HttpResponseException httpResponseException =
118+
new HttpResponseException.Builder(404, "Service Unavailable", new HttpHeaders()).build();
119+
exception = new BigQueryException(httpResponseException);
120+
assertEquals(404, exception.getCode());
121+
assertFalse(exception.isRetryable());
122+
123+
httpResponseException = new HttpResponseException.Builder(504, null, new HttpHeaders()).build();
124+
exception = new BigQueryException(httpResponseException);
125+
assertEquals(504, exception.getCode());
126+
assertTrue(exception.isRetryable());
127+
128+
httpResponseException = new HttpResponseException.Builder(503, null, new HttpHeaders()).build();
129+
exception = new BigQueryException(httpResponseException);
130+
assertEquals(503, exception.getCode());
131+
assertTrue(exception.isRetryable());
132+
133+
httpResponseException = new HttpResponseException.Builder(502, null, new HttpHeaders()).build();
134+
exception = new BigQueryException(httpResponseException);
135+
assertEquals(502, exception.getCode());
136+
assertTrue(exception.isRetryable());
137+
138+
httpResponseException = new HttpResponseException.Builder(500, null, new HttpHeaders()).build();
139+
exception = new BigQueryException(httpResponseException);
140+
assertEquals(500, exception.getCode());
141+
assertTrue(exception.isRetryable());
142+
115143
}
116144

117145
@Test

‎google-cloud-core/src/main/java/com/google/cloud/BaseServiceException.java‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.google.api.client.googleapis.json.GoogleJsonError;
2020
import com.google.api.client.googleapis.json.GoogleJsonResponseException;
21+
import com.google.api.client.http.HttpResponseException;
2122
import com.google.api.gax.grpc.ApiException;
2223
import com.google.common.base.MoreObjects;
2324

@@ -140,20 +141,27 @@ public BaseServiceException(IOException exception, boolean idempotent) {
140141
String location = null;
141142
String debugInfo = null;
142143
Boolean retryable = null;
143-
if (exception instanceof GoogleJsonResponseException) {
144-
GoogleJsonError jsonError = ((GoogleJsonResponseException) exception).getDetails();
145-
if (jsonError != null) {
146-
Error error = new Error(jsonError.getCode(), reason(jsonError));
147-
code = error.code;
148-
reason = error.reason;
149-
retryable = isRetryable(idempotent, error);
150-
if (reason != null) {
151-
GoogleJsonError.ErrorInfo errorInfo = jsonError.getErrors().get(0);
152-
location = errorInfo.getLocation();
153-
debugInfo = (String) errorInfo.get("debugInfo");
144+
if (exception instanceof HttpResponseException) {
145+
if (exception instanceof GoogleJsonResponseException) {
146+
GoogleJsonError jsonError = ((GoogleJsonResponseException) exception).getDetails();
147+
if (jsonError != null) {
148+
Error error = new Error(jsonError.getCode(), reason(jsonError));
149+
code = error.code;
150+
reason = error.reason;
151+
retryable = isRetryable(idempotent, error);
152+
if (reason != null) {
153+
GoogleJsonError.ErrorInfo errorInfo = jsonError.getErrors().get(0);
154+
location = errorInfo.getLocation();
155+
debugInfo = (String) errorInfo.get("debugInfo");
156+
}
157+
} else {
158+
code = ((GoogleJsonResponseException) exception).getStatusCode();
154159
}
155160
} else {
156-
code = ((GoogleJsonResponseException) exception).getStatusCode();
161+
// In cases where an exception is an instance of HttpResponseException but not
162+
// an instance of GoogleJsonResponseException, check the status code to determine whether it's retryable
163+
code = ((HttpResponseException) exception).getStatusCode();
164+
retryable = isRetryable(idempotent, new Error(code, null));
157165
}
158166
}
159167
this.retryable = MoreObjects.firstNonNull(retryable, isRetryable(idempotent, exception));

‎google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionTest.java‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@
2727
import static org.junit.Assert.assertTrue;
2828

2929
import com.google.api.client.googleapis.json.GoogleJsonError;
30+
import com.google.api.client.http.HttpHeaders;
31+
import com.google.api.client.http.HttpResponseException;
3032
import com.google.cloud.BaseServiceException;
3133
import com.google.cloud.RetryHelper.RetryHelperException;
32-
33-
import org.junit.Test;
34-
3534
import java.io.IOException;
3635
import java.net.SocketTimeoutException;
36+
import org.junit.Test;
3737

3838
public class StorageExceptionTest {
3939

@@ -112,6 +112,37 @@ public void testStorageException() {
112112
assertFalse(exception.isRetryable());
113113
assertTrue(exception.isIdempotent());
114114
assertSame(cause, exception.getCause());
115+
116+
HttpResponseException httpResponseException =
117+
new HttpResponseException.Builder(404, "Service Unavailable", new HttpHeaders()).build();
118+
exception = new StorageException(httpResponseException);
119+
assertEquals(404, exception.getCode());
120+
assertFalse(exception.isRetryable());
121+
122+
httpResponseException = new HttpResponseException.Builder(503, null, new HttpHeaders()).build();
123+
exception = new StorageException(httpResponseException);
124+
assertEquals(503, exception.getCode());
125+
assertTrue(exception.isRetryable());
126+
127+
httpResponseException = new HttpResponseException.Builder(502, null, new HttpHeaders()).build();
128+
exception = new StorageException(httpResponseException);
129+
assertEquals(502, exception.getCode());
130+
assertTrue(exception.isRetryable());
131+
132+
httpResponseException = new HttpResponseException.Builder(500, null, new HttpHeaders()).build();
133+
exception = new StorageException(httpResponseException);
134+
assertEquals(500, exception.getCode());
135+
assertTrue(exception.isRetryable());
136+
137+
httpResponseException = new HttpResponseException.Builder(429, null, new HttpHeaders()).build();
138+
exception = new StorageException(httpResponseException);
139+
assertEquals(429, exception.getCode());
140+
assertTrue(exception.isRetryable());
141+
142+
httpResponseException = new HttpResponseException.Builder(408, null, new HttpHeaders()).build();
143+
exception = new StorageException(httpResponseException);
144+
assertEquals(408, exception.getCode());
145+
assertTrue(exception.isRetryable());
115146
}
116147

117148
@Test

0 commit comments

Comments
 (0)