Fix socket timeout exception (#337)

* Don't crash on null test executions * Use 60s timeout on requests by default * Add executeWithRetry * Add ftl.http package * Use TimeoutHttpRequestInitializer in mock code path
Flank · Oct 9, 2018 · 86a530f · 86a530f
1 parent cba181e
commit 86a530f
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 17 deletions.
diff --git a/test_runner/src/main/kotlin/ftl/android/AndroidCatalog.kt b/test_runner/src/main/kotlin/ftl/android/AndroidCatalog.kt
@@ -1,6 +1,7 @@
 package ftl.android
 
 import com.google.api.services.testing.model.AndroidDevice
+import ftl.http.executeWithRetry
 import ftl.gc.GcTesting
 
 /**
@@ -9,7 +10,7 @@ import ftl.gc.GcTesting
  */
 object AndroidCatalog {
     private val androidDeviceCatalog by lazy {
-        GcTesting.get.testEnvironmentCatalog().get("android").execute().androidDeviceCatalog
+        GcTesting.get.testEnvironmentCatalog().get("android").executeWithRetry().androidDeviceCatalog
     }
 
     val androidModelIds by lazy { androidDeviceCatalog.models.map { it.id } }

diff --git a/test_runner/src/main/kotlin/ftl/config/FtlConstants.kt b/test_runner/src/main/kotlin/ftl/config/FtlConstants.kt
@@ -1,11 +1,13 @@
 package ftl.config
 
-import com.google.api.client.auth.oauth2.Credential
 import com.google.api.client.googleapis.auth.oauth2.GoogleCredential
 import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport
 import com.google.api.client.googleapis.testing.auth.oauth2.MockGoogleCredential
+import com.google.api.client.googleapis.util.Utils
+import com.google.api.client.http.HttpRequestInitializer
 import com.google.api.client.http.javanet.NetHttpTransport
-import com.google.api.client.json.jackson2.JacksonFactory
+import com.google.api.client.json.JsonFactory
+import ftl.http.TimeoutHttpRequestInitializer
 
 object FtlConstants {
     var useMock = false
@@ -18,7 +20,7 @@ object FtlConstants {
     const val matrixIdsFile = "matrix_ids.json"
     const val applicationName = "Flank"
     const val GCS_PREFIX = "gs://"
-    val JSON_FACTORY: JacksonFactory by lazy { JacksonFactory.getDefaultInstance() }
+    val JSON_FACTORY: JsonFactory by lazy { Utils.getDefaultJsonFactory() }
 
     val httpTransport: NetHttpTransport by lazy {
         try {
@@ -28,20 +30,22 @@ object FtlConstants {
         }
     }
 
-    val credential: Credential by lazy {
+    val credential: HttpRequestInitializer by lazy {
         try {
-            if (useMock) {
-                return@lazy MockGoogleCredential.Builder()
-                    .setTransport(MockGoogleCredential.newMockHttpTransportWithSampleTokenResponse())
-                    .build()
-            }
-
-            val defaultCredential = GoogleCredential.getApplicationDefault()
             // Scope is required.
             // https://developers.google.com/identity/protocols/googlescopes
             // https://developers.google.com/identity/protocols/application-default-credentials
             // https://cloud.google.com/sdk/gcloud/reference/alpha/compute/instances/set-scopes
-            return@lazy defaultCredential.createScoped(listOf("https://www.googleapis.com/auth/cloud-platform"))
+            val credential = if (useMock) {
+                MockGoogleCredential.Builder()
+                    .setTransport(MockGoogleCredential.newMockHttpTransportWithSampleTokenResponse())
+                    .build()
+            } else {
+                GoogleCredential.getApplicationDefault()
+                    .createScoped(listOf("https://www.googleapis.com/auth/cloud-platform"))
+            }
+
+            return@lazy TimeoutHttpRequestInitializer(credential)
         } catch (e: Exception) {
             throw RuntimeException(e)
         }

diff --git a/test_runner/src/main/kotlin/ftl/gc/GcTestMatrix.kt b/test_runner/src/main/kotlin/ftl/gc/GcTestMatrix.kt
@@ -2,6 +2,7 @@ package ftl.gc
 
 import com.google.api.services.testing.model.TestMatrix
 import ftl.args.IArgs
+import ftl.http.executeWithRetry
 import ftl.util.Utils.sleep
 import java.time.Duration.ofHours
 
@@ -38,7 +39,7 @@ object GcTestMatrix {
 
         while (failed < maxWait) {
             try {
-                return getMatrix.execute()
+                return getMatrix.executeWithRetry()
             } catch (e: Exception) {
                 sleep(1)
                 failed += 1

diff --git a/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt b/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt
@@ -10,6 +10,7 @@ import ftl.config.FtlConstants.JSON_FACTORY
 import ftl.config.FtlConstants.applicationName
 import ftl.config.FtlConstants.credential
 import ftl.config.FtlConstants.httpTransport
+import ftl.http.executeWithRetry
 
 object GcToolResults {
 
@@ -29,7 +30,7 @@ object GcToolResults {
             .histories()
             .list(args.projectId)
             .setFilterByName(args.resultsHistoryName)
-            .execute()
+            .executeWithRetry()
         return result?.histories ?: emptyList()
     }
 
@@ -64,7 +65,7 @@ object GcToolResults {
                 toolResultsStep.executionId,
                 toolResultsStep.stepId
             )
-            .execute()
+            .executeWithRetry()
     }
 
     fun getDefaultBucket(projectId: String): String? {

diff --git a/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt b/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt
@@ -0,0 +1,21 @@
+package ftl.http
+
+import com.google.api.client.googleapis.services.json.AbstractGoogleJsonClientRequest
+import java.io.IOException
+
+// Only use on calls that don't change state.
+// Fetching status is safe to retry on timeout. Creating a matrix is not.
+fun <T> AbstractGoogleJsonClientRequest<T>.executeWithRetry(): T {
+    var lastErr: IOException? = null
+
+    for (i in 1..10) {
+        try {
+            return this.execute()
+        } catch (err: IOException) {
+            lastErr = err
+            System.err.println("Request failed, retrying ${i}x $err")
+        }
+    }
+
+    throw IOException("Request failed", lastErr)
+}
diff --git a/test_runner/src/main/kotlin/ftl/http/TimeoutHttpRequestInitializer.kt b/test_runner/src/main/kotlin/ftl/http/TimeoutHttpRequestInitializer.kt
@@ -0,0 +1,14 @@
+package ftl.http
+
+import com.google.api.client.googleapis.auth.oauth2.GoogleCredential
+import com.google.api.client.http.HttpRequest
+import com.google.api.client.http.HttpRequestInitializer
+
+class TimeoutHttpRequestInitializer(private val googleCredential: GoogleCredential) : HttpRequestInitializer {
+    override fun initialize(request: HttpRequest?) {
+        googleCredential.initialize(request)
+        // timeout in milliseconds. wait 60s instead of default 20s
+        request?.connectTimeout = 60 * 1000
+        request?.readTimeout = 60 * 1000
+    }
+}
diff --git a/test_runner/src/main/kotlin/ftl/ios/IosCatalog.kt b/test_runner/src/main/kotlin/ftl/ios/IosCatalog.kt
@@ -1,5 +1,6 @@
 package ftl.ios
 
+import ftl.http.executeWithRetry
 import ftl.gc.GcTesting
 
 /**
@@ -14,7 +15,7 @@ object IosCatalog {
 
     private val iosDeviceCatalog by lazy {
         try {
-            GcTesting.get.testEnvironmentCatalog().get("ios").execute().iosDeviceCatalog
+            GcTesting.get.testEnvironmentCatalog().get("ios").executeWithRetry().iosDeviceCatalog
         } catch (e: java.lang.Exception) {
             throw java.lang.RuntimeException(
                 """

diff --git a/test_runner/src/main/kotlin/ftl/json/SavedMatrix.kt b/test_runner/src/main/kotlin/ftl/json/SavedMatrix.kt
@@ -54,6 +54,7 @@ class SavedMatrix(matrix: TestMatrix) {
         billableVirtualMinutes = 0
         billablePhysicalMinutes = 0
         outcome = success
+        if (matrix.testExecutions == null) return
         matrix.testExecutions.forEach {
             val step = GcToolResults.getResults(it.toolResultsStep)
             if (step.testExecutionStep == null) return