From 86a530f0778f8ce9f8c260f852b1524c767d6c5a Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Tue, 9 Oct 2018 00:11:33 -0400 Subject: [PATCH] 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 --- .../main/kotlin/ftl/android/AndroidCatalog.kt | 3 +- .../main/kotlin/ftl/config/FtlConstants.kt | 28 +++++++++++-------- .../src/main/kotlin/ftl/gc/GcTestMatrix.kt | 3 +- .../src/main/kotlin/ftl/gc/GcToolResults.kt | 5 ++-- .../main/kotlin/ftl/http/ExecuteWithRetry.kt | 21 ++++++++++++++ .../ftl/http/TimeoutHttpRequestInitializer.kt | 14 ++++++++++ .../src/main/kotlin/ftl/ios/IosCatalog.kt | 3 +- .../src/main/kotlin/ftl/json/SavedMatrix.kt | 1 + 8 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt create mode 100644 test_runner/src/main/kotlin/ftl/http/TimeoutHttpRequestInitializer.kt diff --git a/test_runner/src/main/kotlin/ftl/android/AndroidCatalog.kt b/test_runner/src/main/kotlin/ftl/android/AndroidCatalog.kt index 421df4781a..7521ad4d34 100644 --- 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 index a34e37ce56..3b4a369dec 100644 --- 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 index fdb66d8fb8..680897c7fe 100644 --- 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 index 90653deba2..8ccc548ad8 100644 --- 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 new file mode 100644 index 0000000000..3cd79ccd7a --- /dev/null +++ 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 AbstractGoogleJsonClientRequest.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 new file mode 100644 index 0000000000..83049a320c --- /dev/null +++ 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 index a790208d7e..c905128fa3 100644 --- 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 index 9c4743d96d..378f1545f7 100644 --- 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