Skip to content

Commit b4793a5

Browse files
committed
- fixes a bug where the retry handler could fail on chunked encoding responses
1 parent 909383f commit b4793a5

File tree

1 file changed

+32
-30
lines changed

1 file changed

+32
-30
lines changed

src/main/java/com/microsoft/graph/httpcore/RetryHandler.java

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@
1212
import okhttp3.Response;
1313

1414
public class RetryHandler implements Interceptor{
15-
15+
1616
public final MiddlewareType MIDDLEWARE_TYPE = MiddlewareType.RETRY;
1717

1818
private RetryOptions mRetryOption;
19-
19+
2020
/*
2121
* constant string being used
2222
*/
@@ -26,13 +26,13 @@ public class RetryHandler implements Interceptor{
2626
private final String TRANSFER_ENCODING_CHUNKED = "chunked";
2727
private final String APPLICATION_OCTET_STREAM = "application/octet-stream";
2828
private final String CONTENT_TYPE = "Content-Type";
29-
29+
3030
public static final int MSClientErrorCodeTooManyRequests = 429;
3131
public static final int MSClientErrorCodeServiceUnavailable = 503;
3232
public static final int MSClientErrorCodeGatewayTimeout = 504;
33-
33+
3434
private final long DELAY_MILLISECONDS = 1000;
35-
35+
3636
/*
3737
* @retryOption Create Retry handler using retry option
3838
*/
@@ -48,28 +48,28 @@ public RetryHandler(RetryOptions retryOption) {
4848
public RetryHandler() {
4949
this(null);
5050
}
51-
51+
5252
boolean retryRequest(Response response, int executionCount, Request request, RetryOptions retryOptions) {
53-
53+
5454
// Should retry option
5555
// Use should retry common for all requests
5656
IShouldRetry shouldRetryCallback = null;
5757
if(retryOptions != null) {
5858
shouldRetryCallback = retryOptions.shouldRetry();
5959
}
60-
60+
6161
boolean shouldRetry = false;
6262
// Status codes 429 503 504
6363
int statusCode = response.code();
64-
// Only requests with payloads that are buffered/rewindable are supported.
65-
// Payloads with forward only streams will be have the responses returned
64+
// Only requests with payloads that are buffered/rewindable are supported.
65+
// Payloads with forward only streams will be have the responses returned
6666
// without any retry attempt.
67-
shouldRetry =
68-
(executionCount <= retryOptions.maxRetries())
67+
shouldRetry =
68+
(executionCount <= retryOptions.maxRetries())
6969
&& checkStatus(statusCode) && isBuffered(response, request)
70-
&& shouldRetryCallback != null
70+
&& shouldRetryCallback != null
7171
&& shouldRetryCallback.shouldRetry(retryOptions.delay(), executionCount, request, response);
72-
72+
7373
if(shouldRetry) {
7474
long retryInterval = getRetryAfter(response, retryOptions.delay(), executionCount);
7575
try {
@@ -80,7 +80,7 @@ && checkStatus(statusCode) && isBuffered(response, request)
8080
}
8181
return shouldRetry;
8282
}
83-
83+
8484
/**
8585
* Get retry after in milliseconds
8686
* @param response Response
@@ -100,28 +100,28 @@ long getRetryAfter(Response response, long delay, int executionCount) {
100100
}
101101
return (long)Math.min(retryDelay, RetryOptions.MAX_DELAY * DELAY_MILLISECONDS);
102102
}
103-
103+
104104
boolean checkStatus(int statusCode) {
105-
return (statusCode == MSClientErrorCodeTooManyRequests || statusCode == MSClientErrorCodeServiceUnavailable
105+
return (statusCode == MSClientErrorCodeTooManyRequests || statusCode == MSClientErrorCodeServiceUnavailable
106106
|| statusCode == MSClientErrorCodeGatewayTimeout);
107107
}
108-
108+
109109
boolean isBuffered(Response response, Request request) {
110110
String methodName = request.method();
111-
if(methodName.equalsIgnoreCase("GET") || methodName.equalsIgnoreCase("DELETE") || methodName.equalsIgnoreCase("HEAD") || methodName.equalsIgnoreCase("OPTIONS"))
111+
if(methodName.equalsIgnoreCase("GET") || methodName.equalsIgnoreCase("DELETE") || methodName.equalsIgnoreCase("HEAD") || methodName.equalsIgnoreCase("OPTIONS"))
112112
return true;
113-
113+
114114
boolean isHTTPMethodPutPatchOrPost = methodName.equalsIgnoreCase("POST") ||
115115
methodName.equalsIgnoreCase("PUT") ||
116116
methodName.equalsIgnoreCase("PATCH");
117-
117+
118118
if(isHTTPMethodPutPatchOrPost) {
119119
boolean isStream = response.header(CONTENT_TYPE)!=null && response.header(CONTENT_TYPE).equalsIgnoreCase(APPLICATION_OCTET_STREAM);
120120
if(!isStream) {
121121
String transferEncoding = response.header(TRANSFER_ENCODING);
122-
boolean isTransferEncodingChunked = (transferEncoding != null) &&
122+
boolean isTransferEncodingChunked = (transferEncoding != null) &&
123123
transferEncoding.equalsIgnoreCase(TRANSFER_ENCODING_CHUNKED);
124-
124+
125125
if(request.body() != null && isTransferEncodingChunked)
126126
return true;
127127
}
@@ -132,24 +132,26 @@ boolean isBuffered(Response response, Request request) {
132132
@Override
133133
public Response intercept(Chain chain) throws IOException {
134134
Request request = chain.request();
135-
135+
136136
if(request.tag(TelemetryOptions.class) == null)
137137
request = request.newBuilder().tag(TelemetryOptions.class, new TelemetryOptions()).build();
138138
request.tag(TelemetryOptions.class).setFeatureUsage(TelemetryOptions.RETRY_HANDLER_ENABLED_FLAG);
139-
139+
140140
Response response = chain.proceed(request);
141-
141+
142142
// Use should retry pass along with this request
143143
RetryOptions retryOption = request.tag(RetryOptions.class);
144144
retryOption = retryOption != null ? retryOption : mRetryOption;
145-
145+
146146
int executionCount = 1;
147147
while(retryRequest(response, executionCount, request, retryOption)) {
148148
request = request.newBuilder().addHeader(RETRY_ATTEMPT_HEADER, String.valueOf(executionCount)).build();
149149
executionCount++;
150-
if(response != null && response.body() != null) {
151-
response.body().close();
152-
}
150+
if(response != null) {
151+
if(response.body() != null)
152+
response.body().close();
153+
response.close();
154+
}
153155
response = chain.proceed(request);
154156
}
155157
return response;

0 commit comments

Comments
 (0)