From aa61f6bfa210ebb52af2a6c0053bb603b6b76164 Mon Sep 17 00:00:00 2001
From: Adam <adam.filipowicz92@gmail.com>
Date: Thu, 2 Jul 2020 13:10:26 +0200
Subject: [PATCH] Revert changes on ExecuteWithRetry, update tests

---
 .../main/kotlin/ftl/http/ExecuteWithRetry.kt  | 58 +++++++------------
 .../test/kotlin/ftl/gc/GcToolResultsTest.kt   |  6 +-
 2 files changed, 23 insertions(+), 41 deletions(-)

diff --git a/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt b/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt
index 6e7426d207..64f699b609 100644
--- a/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt
+++ b/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt
@@ -2,7 +2,6 @@ package ftl.http
 
 import com.google.api.client.googleapis.services.json.AbstractGoogleJsonClientRequest
 import com.google.api.client.http.HttpResponseException
-import com.google.api.client.http.HttpStatusCodes
 import ftl.config.FtlConstants
 import ftl.util.PermissionDenied
 import ftl.util.ProjectNotFound
@@ -10,7 +9,7 @@ import kotlinx.coroutines.delay
 import kotlinx.coroutines.runBlocking
 import java.io.IOException
 import kotlin.math.exp
-import kotlin.math.roundToLong
+import kotlin.math.roundToInt
 
 // Only use on calls that don't change state.
 // Fetching status is safe to retry on timeout. Creating a matrix is not.
@@ -18,44 +17,27 @@ fun <T> AbstractGoogleJsonClientRequest<T>.executeWithRetry(): T = withRetry { t
 
 private inline fun <T> withRetry(crossinline block: () -> T): T = runBlocking {
     var lastError: IOException? = null
-    repeat(MAX_TRIES) { repeatCounter ->
-        val executionResult = tryExecuteBlock(block)
-        if (executionResult.isSuccess()) return@runBlocking executionResult.success()
-
-        val error = executionResult.error()
-        // We want to send every occurrence of Google API error for statistic purposes
-        // https://github.com/Flank/flank/issues/701
-        FtlConstants.bugsnag?.notify(FlankGoogleApiError(error))
-        lastError = error
-        if (handleFtlError(error)) return@repeat
-        delay(calculateDelayTime(repeatCounter))
+    repeat(4) {
+        try {
+            return@runBlocking block()
+        } catch (err: IOException) {
+            // We want to send every occurrence of Google API error for statistic purposes
+            // https://github.com/Flank/flank/issues/701
+            FtlConstants.bugsnag?.notify(FlankGoogleApiError(err))
+
+            lastError = err
+            if (err is HttpResponseException) {
+                // we want to handle some FTL errors with special care
+                when (err.statusCode) {
+                    429 -> return@repeat
+                    403 -> throw PermissionDenied(err)
+                    404 -> throw ProjectNotFound(err)
+                }
+            }
+            delay(exp(it - 1.0).roundToInt().toLong())
+        }
     }
     throw IOException("Request failed", lastError)
 }
-private const val MAX_TRIES = 4
-
-private inline fun <T> tryExecuteBlock(block: () -> T): ExecutionBlockResult<T> = try {
-    ExecutionBlockResult(block(), null)
-} catch (ioError: IOException) {
-    ExecutionBlockResult(null, ioError)
-}
-
-private data class ExecutionBlockResult<T>(private val success: T?, private val error: IOException?) {
-    fun isSuccess() = success != null
-    fun success() = success!!
-    fun error() = error!!
-}
-
-private fun handleFtlError(error: IOException) = (error as? HttpResponseException)?.let {
-    // we want to handle some FTL errors with special care
-    when (it.statusCode) {
-        HttpStatusCodes.STATUS_CODE_FORBIDDEN -> throw PermissionDenied(it)
-        HttpStatusCodes.STATUS_CODE_NOT_FOUND -> throw ProjectNotFound(it)
-        429 -> true
-        else -> false
-    }
-} ?: false
-
-private fun calculateDelayTime(repeatCounter: Int) = exp(repeatCounter - 1.0).roundToLong()
 
 private class FlankGoogleApiError(exception: Throwable) : Error(exception)
diff --git a/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt b/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt
index 1b6b5f00fd..083d408a46 100644
--- a/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt
+++ b/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt
@@ -57,8 +57,8 @@ class GcToolResultsTest {
     fun `getDefaultBucket on 403 error should throw exception with specific message`() {
         val expected = """
             Flank encountered a 403 error when running on project $projectId. Please verify this credential is authorized for the project.
-            Consider authentication with Service Account https://github.com/Flank/flank#authenticate-with-a-service-account
-            or with Google account https://github.com/Flank/flank#authenticate-with-a-google-account
+            Consider authentication with a Service Account https://github.com/Flank/flank#authenticate-with-a-service-account
+            or with a Google account https://github.com/Flank/flank#authenticate-with-a-google-account
             
             Caused by: com.google.api.client.googleapis.json.GoogleJsonResponseException: 403 Forbidden
             {
@@ -117,7 +117,7 @@ class GcToolResultsTest {
     @Test
     fun `getDefaultBucket on 404 error should throw exception with specific message`() {
         val expected = """
-            Flank was unable to find project $projectId. Please verify project id.
+            Flank was unable to find project $projectId. Please verify the project id.
             
             Caused by: com.google.api.client.googleapis.json.GoogleJsonResponseException: 404 Not Found
             {