Skip to content

Commit 803d578

Browse files
Vampirekrzema12
andauthored
fix(server): treat failed binding generation as internal server error (#1924)
Before this PR, it was treated equally if a binding could not be found, and if some exception happened during initial creating of the binding. The exceptions were swallowed and the client received a 404 response. If there was for example a compile error you already got internal server error. But if for example Kotlin Poet throws an illegal argument exception, then this did not propagate properly Now exceptions are logged and the client gets an internal server error if some exception happened and only a 404 if indeed the action was not found. One such example is, if a typing has an `enum` with some option that contains a colon. This resulted in an identifier generated that contains a colon which then produces an exception due to the illegal identifier character. --------- Co-authored-by: Piotr Krzemiński <[email protected]> Co-authored-by: Piotr Krzeminski <[email protected]>
1 parent 5258e6a commit 803d578

File tree

2 files changed

+4
-6
lines changed

2 files changed

+4
-6
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import kotlin.time.Duration.Companion.hours
3232

3333
private val logger = logger { }
3434

35-
typealias ArtifactResult = Result<Map<String, Artifact>>
35+
typealias ArtifactResult = Result<Map<String, Artifact>?>
3636

3737
private val prefetchScope = CoroutineScope(Dispatchers.IO)
3838

@@ -43,7 +43,7 @@ internal fun buildBindingsCache(
4343
.newBuilder()
4444
.refreshAfterWrite(1.hours)
4545
.recordStats()
46-
.asLoadingCache<ActionCoords, ArtifactResult> { runCatching { buildVersionArtifacts(it)!! } }
46+
.asLoadingCache<ActionCoords, ArtifactResult> { runCatching { buildVersionArtifacts(it) } }
4747

4848
fun Routing.artifactRoutes(
4949
bindingsCache: LoadingCache<ActionCoords, ArtifactResult>,
@@ -133,7 +133,7 @@ private suspend fun ApplicationCall.toBindingArtifacts(
133133
if (refresh) {
134134
bindingsCache.invalidate(actionCoords)
135135
}
136-
return bindingsCache.get(actionCoords).getOrNull()
136+
return bindingsCache.get(actionCoords).getOrThrow()
137137
}
138138

139139
private fun PrometheusMeterRegistry.incrementArtifactCounter(call: ApplicationCall) {

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@ class ArtifactRoutesTest :
2727
val response = client.get("some-owner/some-action/v4/some-action-v4.pom")
2828

2929
// Then
30-
// This is incorrect, and will be fixed in
31-
// https://github.com/typesafegithub/github-workflows-kt/pull/1924/
32-
response.status shouldBe HttpStatusCode.NotFound
30+
response.status shouldBe HttpStatusCode.InternalServerError
3331
}
3432
}
3533
}

0 commit comments

Comments
 (0)