Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance permission denied exception logs #875

Merged
merged 15 commits into from
Jul 6, 2020
Prev Previous commit
Next Next commit
Revert changes on ExecuteWithRetry, update tests
adamfilipow92 authored and pawelpasterz committed Jul 6, 2020
commit 0b9d4d01fd33140f3c6f4a1b56c43a39a9ec3a4a
58 changes: 20 additions & 38 deletions test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt
Original file line number Diff line number Diff line change
@@ -2,60 +2,42 @@ 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
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.
fun <T> AbstractGoogleJsonClientRequest<T>.executeWithRetry(): T = withRetry { this.execute() }

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)
6 changes: 3 additions & 3 deletions test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt
Original file line number Diff line number Diff line change
@@ -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.
pawelpasterz marked this conversation as resolved.
Show resolved Hide resolved
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
{