fix(server): don't cache artifact generation failure (#1940)

Resolves
#1938.

Before this change, if there was a failure during binding generation
which result in an exception, it was actually cached and returned to the
user for the next hour.
This change causes the server to not cache the failure, and instead let
the user retry.

Author: krzema12

jit-binding-server/src/main/kotlin/io/github/typesafegithub/workflows/jitbindingserver/ArtifactRoutes.kt

@@ -27,7 +27,7 @@ import kotlinx.coroutines.launch
 
 private val logger = logger { }
 
-typealias CachedVersionArtifact = Result<Map<String, Artifact>?>
+typealias CachedVersionArtifact = Map<String, Artifact>?
 
 private val prefetchScope = CoroutineScope(Dispatchers.IO)
 
@@ -119,7 +119,7 @@ private suspend fun ApplicationCall.toBindingArtifacts(
     if (refresh) {
         bindingsCache.invalidate(actionCoords)
     }
-    return bindingsCache.get(actionCoords).getOrThrow()
+    return bindingsCache.get(actionCoords)
 }
 
 private fun PrometheusMeterRegistry.incrementArtifactCounter(call: ApplicationCall) {

jit-binding-server/src/main/kotlin/io/github/typesafegithub/workflows/jitbindingserver/Main.kt

@@ -82,7 +82,7 @@ private fun buildBindingsCache(
         .newBuilder()
         .refreshAfterWrite(1.hours)
         .recordStats()
-        .asLoadingCache<ActionCoords, CachedVersionArtifact> { runCatching { buildVersionArtifacts(it) } }
+        .asLoadingCache<ActionCoords, CachedVersionArtifact> { buildVersionArtifacts(it) }
 
 @Suppress("ktlint:standard:function-signature") // Conflict with detekt.
 private fun buildMetadataCache(
@@ -95,13 +95,11 @@ private fun buildMetadataCache(
         .refreshAfterWrite(1.hours)
         .recordStats()
         .asLoadingCache<ActionCoords, CachedMetadataArtifact> {
-            runCatching {
-                buildPackageArtifacts(
-                    it,
-                    getGithubAuthToken(),
-                    { coords -> prefetchBindingArtifacts(coords, bindingsCache) },
-                )
-            }
+            buildPackageArtifacts(
+                it,
+                getGithubAuthToken(),
+                { coords -> prefetchBindingArtifacts(coords, bindingsCache) },
+            )
         }
 
 val deliverOnRefreshRoute = System.getenv("GWKT_DELIVER_ON_REFRESH").toBoolean()

jit-binding-server/src/main/kotlin/io/github/typesafegithub/workflows/jitbindingserver/MetadataRoutes.kt

@@ -14,7 +14,7 @@ import io.micrometer.prometheusmetrics.PrometheusMeterRegistry
 
 private val logger = logger { }
 
-typealias CachedMetadataArtifact = Result<Map<String, String>>
+typealias CachedMetadataArtifact = Map<String, String>
 
 fun Routing.metadataRoutes(
     metadataCache: LoadingCache<ActionCoords, CachedMetadataArtifact>,
@@ -45,7 +45,7 @@ private fun Route.metadata(
         if (refresh) {
             metadataCache.invalidate(actionCoords)
         }
-        val metadataArtifacts = metadataCache.get(actionCoords).getOrThrow()
+        val metadataArtifacts = metadataCache.get(actionCoords)
 
         if (refresh && !deliverOnRefreshRoute) return@get call.respondText(text = "OK")
 

jit-binding-server/src/test/kotlin/io/github/typesafegithub/workflows/jitbindingserver/ArtifactRoutesTest.kt

@@ -102,15 +102,9 @@ class ArtifactRoutesTest :
                     // When
                     val response2 = client.get("some-owner/some-action/v4/some-action-v4.pom")
                     // Then
-                    // This assertion represents an undesired behavior - it should retry generating the binding
-                    // and return the actual POM.
-                    // TODO fix in https://github.com/typesafegithub/github-workflows-kt/issues/1938
-                    response2.status shouldBe HttpStatusCode.InternalServerError
+                    response2.status shouldBe HttpStatusCode.OK
 
-                    // This assertion represents an undesired behavior - it should call the binding generation twice,
-                    // first with exception, and then when it's successful.
-                    // TODO fix in https://github.com/typesafegithub/github-workflows-kt/issues/1938
-                    verify(exactly = 1) { mockBuildVersionArtifacts(any()) }
+                    verify(exactly = 2) { mockBuildVersionArtifacts(any()) }
                 }
             }
         }

jit-binding-server/src/test/kotlin/io/github/typesafegithub/workflows/jitbindingserver/MetadataRoutesTest.kt

@@ -112,15 +112,9 @@ class MetadataRoutesTest :
                     // When
                     val response2 = client.get("some-owner/some-action/maven-metadata.xml")
                     // Then
-                    // This assertion represents an undesired behavior - it should retry generating the artifact
-                    // and return the actual POM.
-                    // TODO fix in https://github.com/typesafegithub/github-workflows-kt/issues/1938
-                    response2.status shouldBe HttpStatusCode.InternalServerError
+                    response2.status shouldBe HttpStatusCode.OK
 
-                    // This assertion represents an undesired behavior - it should call the artifact generation twice,
-                    // first with exception, and then when it's successful.
-                    // TODO fix in https://github.com/typesafegithub/github-workflows-kt/issues/1938
-                    verify(exactly = 1) { mockBuildPackageArtifacts(any(), any(), any()) }
+                    verify(exactly = 2) { mockBuildPackageArtifacts(any(), any(), any()) }
                 }
             }
         }