tests

segmentio · Jan 7, 2021 · 66c030e · 66c030e
1 parent 6d996cc
commit 66c030e
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 8 deletions.
diff --git a/analytics/src/main/java/com/segment/analytics/Analytics.java b/analytics/src/main/java/com/segment/analytics/Analytics.java
@@ -224,10 +224,10 @@ public Builder flushInterval(long flushInterval, TimeUnit unit) {
       return this;
     }
 
-    /** Set the interval at which the queue should be flushed. */
+    /** Set how many retries should happen before getting exhausted */
     public Builder retries(int maximumRetries) {
-      if (maximumFlushAttempts < 0) {
-        throw new IllegalArgumentException("retries must be greater than 0");
+      if (maximumRetries < 1) {
+        throw new IllegalArgumentException("retries must be at least 1");
       }
       this.maximumFlushAttempts = maximumRetries;
       return this;

diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java
@@ -269,7 +269,8 @@ boolean upload() {
 
     @Override
     public void run() {
-      for (int attempt = 0; attempt < maximumFlushAttempts; attempt++) {
+      int attempt = 0;
+      for (; attempt <= maximumFlushAttempts; attempt++) {
         boolean retry = upload();
         if (!retry) return;
         try {
@@ -282,7 +283,7 @@ public void run() {
       }
 
       client.log.print(ERROR, "Could not upload batch %s. Retries exhausted.", batch.sequence());
-      notifyCallbacksWithException(batch, new IOException(maximumFlushAttempts + " retries exhausted"));
+      notifyCallbacksWithException(batch, new IOException(Integer.toString(attempt) + " retries exhausted"));
     }
 
     private static boolean is5xx(int status) {

diff --git a/analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java b/analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java
@@ -95,6 +95,23 @@ public void nullLog() {
     }
   }
 
+  @Test
+  public void negativeRetryCount() {
+    try {
+      builder.retries(0);
+      fail("Should fail for retries less than 1");
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("retries must be at least 1");
+    }
+
+    try {
+      builder.retries(-1);
+      fail("Should fail for retries less than 1");
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("retries must be at least 1");
+    }
+  }
+
   @Test
   public void nullTransformer() {
     try {

diff --git a/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java b/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java
@@ -333,12 +333,38 @@ public Call<UploadResponse> answer(InvocationOnMock invocation) {
 		batchUploadTask.run();
 
 		// 10 == maximumFlushAttempts
-		// tries only 10 even though default is 50 in AnalyticsClient.java
-		verify(segmentService, times(10)).upload(batch);
+		// tries 11(one normal run + 10 retries) even though default is 50 in AnalyticsClient.java
+		verify(segmentService, times(11)).upload(batch);
 		verify(callback).failure(eq(trackMessage), argThat(new ArgumentMatcher<IOException>() {
 			@Override
 			public boolean matches(IOException exception) {
-				return exception.getMessage().equals("10 retries exhausted");
+				return exception.getMessage().equals("11 retries exhausted");
+			}
+		}));
+	}
+
+	@Test
+	public void neverRetries() {
+		AnalyticsClient client = newClient();
+		TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build();
+		Batch batch = batchFor(trackMessage);
+
+		when(segmentService.upload(batch)).thenAnswer(new Answer<Call<UploadResponse>>() {
+			public Call<UploadResponse> answer(InvocationOnMock invocation) {
+				Response<UploadResponse> failResponse = Response.error(429, ResponseBody.create(null, "Not Found"));
+				return Calls.response(failResponse);
+			}
+		});
+
+		BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, 0);
+		batchUploadTask.run();
+
+		// runs once but never retries
+		verify(segmentService, times(1)).upload(batch);
+		verify(callback).failure(eq(trackMessage), argThat(new ArgumentMatcher<IOException>() {
+			@Override
+			public boolean matches(IOException exception) {
+				return exception.getMessage().equals("1 retries exhausted");
 			}
 		}));
 	}