Bugfix against retry logic (#576)

* Refactor

* BUGFIX Logic would never backoff

After adding the instant retry amount logic to the code this line of code could cause the transmissions to not back off.

* Changes requested

Author: debugthings

core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmissionNetworkOutput.java

@@ -50,8 +50,7 @@
  *
  * Created by gupele on 12/18/2014.
  */
-public final class TransmissionNetworkOutput implements TransmissionOutput {
-	private static final int MAX_RESEND = 3;
+public final class TransmissionNetworkOutput implements TransmissionOutput {	
 	private final static String CONTENT_TYPE_HEADER = "Content-Type";
 	private final static String CONTENT_ENCODING_HEADER = "Content-Encoding";
 	private final static String RESPONSE_THROTTLING_HEADER = "Retry-After";
@@ -189,10 +188,8 @@ public boolean send(Transmission transmission) {
 				respString = EntityUtils.toString(respEntity);
 				retryAfterHeader = response.getFirstHeader(RESPONSE_THROTTLING_HEADER);
 
-				// After the third time through this dispatcher we should reset the counter and
-				// then fail to second TransmissionOutput
-				if (code > HttpStatus.SC_PARTIAL_CONTENT && transmission.getNumberOfSends() >= MAX_RESEND) {
-					transmission.setNumberOfSends(0);
+				// After we reach our instant retry limit we should fail to second TransmissionOutput
+				if (code > HttpStatus.SC_PARTIAL_CONTENT && transmission.getNumberOfSends() > this.transmissionPolicyManager.getMaxInstantRetries()) {
 					return false;
 				} else if (code == HttpStatus.SC_OK) {
 					// If we've completed then clear the back off flags as the channel does not need
@@ -231,7 +228,7 @@ public boolean send(Transmission transmission) {
 				}
 				httpClient.dispose(response);
 
-				if (code != HttpStatus.SC_OK && transmission.getNumberOfSends() < MAX_RESEND) {
+				if (code != HttpStatus.SC_OK) {
 					// Invoke the listeners for handling things like errors
 					// The listeners will handle the back off logic as well as the dispatch
 					// operation

core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmissionPolicyManager.java

@@ -52,15 +52,15 @@
  */
 public final class TransmissionPolicyManager implements Stoppable, TransmissionHandlerObserver {
 
-	private int INSTANT_RETRY_AMOUNT = 3; // Should always be set by the creator of this class
-	private int INSTANT_RETRY_MAX = 10;   // Stops us from getting into an endless loop
-	
-	// Current thread backoff manager
-	private SenderThreadsBackOffManager backoffManager;
-	
-	// List of transmission policies implemented as handlers
-	private List<TransmissionHandler> transmissionHandlers;
-	
+    private int instantRetryAmount = 3;         // Should always be set by the creator of this class
+    private final int INSTANT_RETRY_MAX = 10;   // Stops us from getting into an endless loop
+
+    // Current thread backoff manager
+    private SenderThreadsBackOffManager backoffManager;
+
+    // List of transmission policies implemented as handlers
+    private List<TransmissionHandler> transmissionHandlers;
+
     // The future date the the transmission is blocked
     private Date suspensionDate;
 
@@ -95,7 +95,7 @@ public void run() {
 
     /**
      * Create the {@link TransmissionPolicyManager} and set the ability to throttle.
-     * @param throttlingIsEnabled Set whether the {@link TransmissionPolicyManager} can be throttled.  
+     * @param throttlingIsEnabled Set whether the {@link TransmissionPolicyManager} can be throttled.
      */
     public TransmissionPolicyManager(boolean throttlingIsEnabled) {
         suspensionDate = null;
@@ -104,32 +104,32 @@ public TransmissionPolicyManager(boolean throttlingIsEnabled) {
         this.backoffManager = new SenderThreadsBackOffManager(new ExponentialBackOffTimesPolicy());
     }
 
-    /** 
+    /**
      * Suspend the transmission thread according to the current back off policy.
      */
     public void backoff() {
-    	policyState.setCurrentState(TransmissionPolicy.BACKOFF);
-    	long backOffMillis = backoffManager.backOffCurrentSenderThreadValue();
-        if (backOffMillis > 0)
+        policyState.setCurrentState(TransmissionPolicy.BACKOFF);
+        long backOffMillis = backoffManager.backOffCurrentSenderThreadValue();
+        if (backOffMillis > 0) 
         {
             long backOffSeconds = backOffMillis / 1000;
             InternalLogger.INSTANCE.info("App is throttled, telemetry will be blocked for %s seconds.", backOffSeconds);
             this.suspendInSeconds(TransmissionPolicy.BACKOFF, backOffSeconds);
-        } 
+        }
     }
-    
+
     /**
      * Clear the current thread state and and reset the back off counter.
      */
     public void clearBackoff() {
-    	policyState.setCurrentState(TransmissionPolicy.UNBLOCKED);
+        policyState.setCurrentState(TransmissionPolicy.UNBLOCKED);
         backoffManager.onDoneSending();
         InternalLogger.INSTANCE.info("Backoff has been reset.");
     }
-    
+
     /**
      * Suspend this transmission thread using the specified policy
-     * @param policy The {@link TransmissionPolicy} to use for suspension 
+     * @param policy The {@link TransmissionPolicy} to use for suspension
      * @param suspendInSeconds The number of seconds to suspend.
      */
     public void suspendInSeconds(TransmissionPolicy policy, long suspendInSeconds) {
@@ -162,12 +162,12 @@ public TransmissionPolicyStateFetcher getTransmissionPolicyState() {
 
     private synchronized void doSuspend(TransmissionPolicy policy, long suspendInSeconds) {
         try {
-            if (policy == TransmissionPolicy.UNBLOCKED ) {
+            if (policy == TransmissionPolicy.UNBLOCKED) {
                 return;
             }
-                       
+
             Date date = Calendar.getInstance().getTime();
-            date.setTime(date.getTime() + 1000 * suspendInSeconds);
+            date.setTime(date.getTime() + (1000 * suspendInSeconds));
             if (this.suspensionDate != null) {
                 long diff = date.getTime() - suspensionDate.getTime();
                 if (diff <= 0) {
@@ -214,36 +214,36 @@ public Thread newThread(Runnable r) {
 
         SDKShutdownActivity.INSTANCE.register(this);
     }
-    
-	@Override
-	public void onTransmissionSent(TransmissionHandlerArgs transmissionArgs) {
-		for (TransmissionHandler handler : this.transmissionHandlers) {
-			handler.onTransmissionSent(transmissionArgs);
-		}
-	}
-	
-	@Override
-	public void addTransmissionHandler(TransmissionHandler handler) {
-		if(handler != null) {
-			this.transmissionHandlers.add(handler);
-		}
-	}
-	
-	/**
-	 * Set the number of retries before performing a back off operation.
-	 * @param maxInstantRetries Number of retries
-	 */
-	public void setMaxInstantRetries(int maxInstantRetries) {
-		if (maxInstantRetries >= 0 && maxInstantRetries < INSTANT_RETRY_MAX) {
-			INSTANT_RETRY_AMOUNT = maxInstantRetries;
-		}
-	}
-	
-	/**
-	 * Get the number of retries before performing a back off operation.
-	 * @return Number of retries
-	 */
-	public int getMaxInstantRetries() {
-		return INSTANT_RETRY_AMOUNT;
-	}
+
+    @Override
+    public void onTransmissionSent(TransmissionHandlerArgs transmissionArgs) {
+        for (TransmissionHandler handler : this.transmissionHandlers) {
+            handler.onTransmissionSent(transmissionArgs);
+        }
+    }
+
+    @Override
+    public void addTransmissionHandler(TransmissionHandler handler) {
+        if (handler != null) {
+            this.transmissionHandlers.add(handler);
+        }
+    }
+
+    /**
+     * Set the number of retries before performing a back off operation.
+     * @param maxInstantRetries Number of retries
+     */
+    public void setMaxInstantRetries(int maxInstantRetries) {
+        if (maxInstantRetries > 0 && maxInstantRetries < INSTANT_RETRY_MAX) {
+            instantRetryAmount = maxInstantRetries;
+        }
+    }
+
+    /**
+     * Get the number of retries before performing a back off operation.
+     * @return Number of retries
+     */
+    public int getMaxInstantRetries() {
+        return instantRetryAmount;
+    }
 }