Repository: microsoftgraph/msgraph-sdk-java-core
Issue: RetryHandler does not retry in case of 429 error

<!-- Read me before you submit this issue

First off, thank you for taking the time to open this issue! We do appreciate it. Please bear with us if we don't get to this right away.

If this is a question about the Microsoft Graph service API, or a question about how to use this SDK, please post your question to StackOverflow with the [microsoftgraph] tag.
https://stackoverflow.com/questions/tagged/microsoft-graph

This repo is for the Microsoft Graph Java SDK. Issues opened in this repo should only target this SDK.

Before you open this issue, did you:
- Search the issues to determine whether someone already opened this issue?
- Search StackOverflow for an answer?
- Capture the repro steps and gather the information requested in the steps below to reproduce your scenario?
- Review the samples under github.com/microsoftgraph? They can help for some scenarios.
- Take a look at the functional tests in this repo? They may have an example for you. See the [functional tests](https://github.com/microsoftgraph/msgraph-sdk-java/tree/master/src/test/java/com/microsoft/graph/functional)

Please provide the following before submitting this issue:
- Expected behavior. Please provide **links to the specific [Microsoft Graph documentation](https://developer.microsoft.com/en-us/graph/docs/concepts/overview)** you used to determine the expected behavior. 
- Actual behavior. Provide error codes, stack information, and a [Fiddler](http://www.telerik.com/fiddler) capture of the request and response (please remove personally identifiable information before posting).
- Steps to reproduce the behavior. Include your code, IDE versions, client library versions, and any other information that might be helpful to understand your scenario.
-->

### Issue Class : com.microsoft.graph.httpcore.RetryHandler

When graph endpoint for adding tabs POST: /tabs gives throttling (429) error. When RetryHandler calls retryRequest it checks for isBuffered where it requires header Transfer-Encoding as Chunked, in order to send True and to set shouldRetry flag as True.

### Expected behavior
This "Transfer-Encoding = Chunked" header missing case should be handle and it shouldRetry flag should get set as 'True'. So that further it wait for given time and retry again.

### Actual behavior
Some times header "Transfer-Encoding = chunked" is missing in the response headers. Causing it to return False. As a result it sets shouldRetry flag as False and it never retry again.  

Response Headers : 
[
   Cache-Control : private, 
   client-request-id : 5a064a73-9106-40ec-8dff-4069c5752de4, 
   Content-Length : 885, 
   Content-Type : application/json, 
   Date : Fri, 26 Feb 2021 14:03:23 GMT, 
   request-id : 64638ad8-1f98-49d1-9185-238341d4938e, 
   Retry-After : 60, 
   Strict-Transport-Security : max-age=31536000, 
   x-ms-ags-diagnostic : 
      {"ServerInfo": {"DataCenter":"SouthIndia","Slice":"SliceC","Ring":"3", "ScaleUnit":"001","RoleInstance":"AGSFE_IN_7" }}
]

Response Body : 
{
	"error": {
		"code": "TooManyRequests",
		"message": "Too many requests from Identifier:9155ce04-4eae-42ae-905e-ac219c282f72+786c9214-bb88-448d-9a43-3afeb1518b52+786c9214-bb88-448d-9a43-3afeb1518b52 under category:throttle.teams.ags.api_complex_level_10.tenant_normal.app_normal.operation_write_sustained. Please try again later.",
		"innerError": {
			"date": "2021-02-26T14:03:23",
			"request-id": "64638ad8-1f98-49d1-9185-238341d4938e",
			"client-request-id": "5a064a73-9106-40ec-8dff-4069c5752de4",
			"status": 429,
			"code": "429",
			"message": "Too many requests from Identifier:9155ce04-4eae-42ae-905e-ac219c282f72+786c9214-bb88-448d-9a43-3afeb1518b52+786c9214-bb88-448d-9a43-3afeb1518b52 under category:throttle.teams.ags.api_complex_level_10.tenant_normal.app_normal.operation_write_sustained. Please try again later."
		}
	}
}

### Steps to reproduce the behavior
1. Throttle the /tabs end point, by sending multiple tab addition requests for microsoft teams channels.
2. Use microsoft graph SDK for java to add tab in channel.

<code>
IGraphServiceClient graphClient = GraphServiceClient.builder().authenticationProvider(authProvider).buildClient(); 
</code>
</br>
<code>
TeamsTab responseTeamsTab = mGraphClient.teams(teamId).channels(channelId).tabs().buildRequest().post(lTeamsTab)
</code>

  



 
[AB#8330](https://microsoftgraph.visualstudio.com/0985d294-5762-4bc2-a565-161ef349ca3e/_workitems/edit/8330)

Changed files:
src/main/java/com/microsoft/graph/httpcore/RetryHandler.java, src/test/java/com/microsoft/graph/httpcore/RetryHandlerTest.java

diff --git a/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java b/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java
index 6ba5900..314dffe 100644
--- a/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java
+++ b/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java
@@ -22,10 +22,8 @@ public class RetryHandler implements Interceptor{
      */
     private final String RETRY_ATTEMPT_HEADER = "Retry-Attempt";
     private final String RETRY_AFTER = "Retry-After";
-    private final String TRANSFER_ENCODING = "Transfer-Encoding";
-    private final String TRANSFER_ENCODING_CHUNKED = "chunked";
-    private final String APPLICATION_OCTET_STREAM = "application/octet-stream";
-    private final String CONTENT_TYPE = "Content-Type";
+    /** Content length request header value */
+    private final String CONTENT_LENGTH = "Content-Length";
 
     public static final int MSClientErrorCodeTooManyRequests = 429;
     public static final int MSClientErrorCodeServiceUnavailable  = 503;
@@ -66,7 +64,7 @@ public class RetryHandler implements Interceptor{
 		// without any retry attempt.
 		shouldRetry =
 				(executionCount <= retryOptions.maxRetries())
-				&& checkStatus(statusCode) && isBuffered(response, request)
+				&& checkStatus(statusCode) && isBuffered(request)
 				&& shouldRetryCallback != null
 				&& shouldRetryCallback.shouldRetry(retryOptions.delay(), executionCount, request, response);
 
@@ -106,27 +104,22 @@ public class RetryHandler implements Interceptor{
 	    		|| statusCode == MSClientErrorCodeGatewayTimeout);
 	}
 
-	boolean isBuffered(Response response, Request request) {
-		String methodName = request.method();
-		if(methodName.equalsIgnoreCase("GET") || methodName.equalsIgnoreCase("DELETE") || methodName.equalsIgnoreCase("HEAD") || methodName.equalsIgnoreCase("OPTIONS"))
-			return true;
+	boolean isBuffered(Request request) {
+		final String methodName = request.method();
 
-		boolean isHTTPMethodPutPatchOrPost = methodName.equalsIgnoreCase("POST") ||
+		final boolean isHTTPMethodPutPatchOrPost = methodName.equalsIgnoreCase("POST") ||
 				methodName.equalsIgnoreCase("PUT") ||
 				methodName.equalsIgnoreCase("PATCH");
 
-		if(isHTTPMethodPutPatchOrPost) {
-			boolean isStream = response.header(CONTENT_TYPE)!=null && response.header(CONTENT_TYPE).equalsIgnoreCase(APPLICATION_OCTET_STREAM);
-			if(!isStream) {
-				String transferEncoding = response.header(TRANSFER_ENCODING);
-				boolean isTransferEncodingChunked = (transferEncoding != null) &&
-						transferEncoding.equalsIgnoreCase(TRANSFER_ENCODING_CHUNKED);
-
-				if(request.body() != null && isTransferEncodingChunked)
-					return true;
-			}
+		if(isHTTPMethodPutPatchOrPost && request.body() != null) {
+            try {
+                return request.body().contentLength() != -1L;
+            } catch (IOException ex) {
+                // expected
+                return false;
+            }
 		}
-		return false;
+		return true;
 	}
 
 	@Override
diff --git a/src/test/java/com/microsoft/graph/httpcore/RetryHandlerTest.java b/src/test/java/com/microsoft/graph/httpcore/RetryHandlerTest.java
index d5ddc39..4f25ec0 100644
--- a/src/test/java/com/microsoft/graph/httpcore/RetryHandlerTest.java
+++ b/src/test/java/com/microsoft/graph/httpcore/RetryHandlerTest.java
@@ -4,6 +4,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
+import java.io.IOException;
 import java.net.HttpURLConnection;
 
 import org.junit.Test;
@@ -16,6 +17,7 @@ import okhttp3.Protocol;
 import okhttp3.Request;
 import okhttp3.RequestBody;
 import okhttp3.Response;
+import okio.BufferedSink;
 
 public class RetryHandlerTest {
 
@@ -76,7 +78,7 @@ public class RetryHandlerTest {
 		// Default retry options with Number of maxretries default to 3
 		RetryOptions retryOptions = new RetryOptions();
 		// Try to execute one more than allowed default max retries
-		int executionCount = RetryOptions.DEFAULT_MAX_RETRIES + 1; 
+		int executionCount = RetryOptions.DEFAULT_MAX_RETRIES + 1;
 		assertFalse(retryhandler.retryRequest(response, executionCount, httpget, retryOptions));
 	}
 
@@ -87,7 +89,7 @@ public class RetryHandlerTest {
 		Response response = new Response.Builder()
 				.protocol(Protocol.HTTP_1_1)
 				// For status code 500 which is not in (429 503 504), So NO retry
-				.code(HTTP_SERVER_ERROR)  
+				.code(HTTP_SERVER_ERROR)
 				.message( "Internal Server Error")
 				.request(httpget)
 				.build();
@@ -100,7 +102,7 @@ public class RetryHandlerTest {
 		Request httppost = new Request.Builder().url(testmeurl).post(RequestBody.create(MediaType.parse("application/json"), "TEST")).build();
 		Response response = new Response.Builder()
 				.protocol(Protocol.HTTP_1_1)
-				.code(HttpURLConnection.HTTP_GATEWAY_TIMEOUT)  
+				.code(HttpURLConnection.HTTP_GATEWAY_TIMEOUT)
 				.message( "gateway timeout")
 				.request(httppost)
 				.addHeader("Transfer-Encoding", "chunked")
@@ -114,15 +116,15 @@ public class RetryHandlerTest {
 		Request httppost = new Request.Builder().url(testmeurl).post(RequestBody.create(MediaType.parse("application/json"), "TEST")).build();
 		Response response = new Response.Builder()
 				.protocol(Protocol.HTTP_1_1)
-				.code(HttpURLConnection.HTTP_GATEWAY_TIMEOUT)  
+				.code(HttpURLConnection.HTTP_GATEWAY_TIMEOUT)
 				.message( "gateway timeout")
 				.request(httppost)
 				.addHeader("Transfer-Encoding", "chunked")
 				.build();
-		
+
 		assertTrue(retryhandler.retryRequest(response, 1, httppost, new RetryOptions()));
 	}
-	
+
 	@Test
 	public void testGetRetryAfterWithHeader() {
 		RetryHandler retryHandler = new RetryHandler();
@@ -131,7 +133,7 @@ public class RetryHandlerTest {
 		delay = retryHandler.getRetryAfter(TestResponse().newBuilder().addHeader("Retry-After", "1").build(), 2, 3);
 		assertTrue(delay == 1000);
 	}
-	
+
 	@Test
 	public void testGetRetryAfterOnFirstExecution() {
 		RetryHandler retryHandler = new RetryHandler();
@@ -140,14 +142,48 @@ public class RetryHandlerTest {
 		delay = retryHandler.getRetryAfter(TestResponse(), 3, 2);
 		assertTrue(delay > 3100);
 	}
-	
+
 	@Test
 	public void testGetRetryAfterMaxExceed() {
 		RetryHandler retryHandler = new RetryHandler();
 		long delay = retryHandler.getRetryAfter(TestResponse(), 190, 1);
 		assertTrue(delay == 180000);
 	}
-	
+    @Test
+    public void testIsBuffered() {
+		final RetryHandler retryHandler = new RetryHandler();
+        Request request = new Request.Builder().url("https://localhost").method("GET", null).build();
+        assertTrue("Get Request is buffered", retryHandler.isBuffered(request));
+
+        request = new Request.Builder().url("https://localhost").method("DELETE", null).build();
+        assertTrue("Delete Request is buffered", retryHandler.isBuffered(request));
+
+        request = new Request.Builder().url("https://localhost")
+                                        .method("POST",
+                                            RequestBody.create(MediaType.parse("application/json"),
+                                                                "{\\"key\\": 42 }"))
+                                        .build();
+        assertTrue("Post Request is buffered", retryHandler.isBuffered(request));
+
+        request = new Request.Builder().url("https://localhost")
+                                        .method("POST",
+                                            new RequestBody() {
+
+												@Override
+												public MediaType contentType() {
+													return MediaType.parse("application/octet-stream");
+												}
+
+												@Override
+												public void writeTo(BufferedSink sink) throws IOException {
+													// TODO Auto-generated method stub
+
+												}
+                                            })
+                                        .build();
+        assertFalse("Post Stream Request is not buffered", retryHandler.isBuffered(request));
+    }
+
 	Response TestResponse() {
 		return new Response.Builder()
 				.code(429)