feat: pass on errors from SILO

GenSpectrum · Oct 17, 2023 · 7f6153d · 7f6153d
1 parent a513ce8
commit 7f6153d
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 27 deletions.
diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/ExceptionHandler.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/ExceptionHandler.kt
@@ -70,18 +70,17 @@ class ExceptionHandler : ResponseEntityExceptionHandler() {
     }
 
     @ExceptionHandler(SiloException::class)
-    @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
     fun handleSiloException(e: SiloException): ErrorResponse {
-        log.error(e) { "Caught SiloException: ${e.message}" }
+        log.error(e) { "Caught SiloException: ${e.statusCode} - ${e.message}" }
 
         return ResponseEntity
-            .status(HttpStatus.INTERNAL_SERVER_ERROR)
+            .status(e.statusCode)
             .contentType(MediaType.APPLICATION_JSON)
             .body(
                 LapisErrorResponse(
                     LapisError(
-                        "Silo error",
-                        "${e.message}",
+                        e.title,
+                        e.message,
                     ),
                 ),
             )

diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloClient.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloClient.kt
@@ -1,10 +1,12 @@
 package org.genspectrum.lapis.silo
 
 import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.kotlin.readValue
 import mu.KotlinLogging
 import org.genspectrum.lapis.request.DataVersion
 import org.springframework.beans.factory.annotation.Value
 import org.springframework.http.HttpHeaders
+import org.springframework.http.HttpStatus
 import org.springframework.http.MediaType
 import org.springframework.stereotype.Component
 import java.net.URI
@@ -24,26 +26,41 @@ class SiloClient(
     fun <ResponseType> sendQuery(
         query: SiloQuery<ResponseType>,
     ): ResponseType {
+        val queryJson = objectMapper.writeValueAsString(query)
+
+        log.info { "Calling SILO: $queryJson" }
+
         val client = HttpClient.newHttpClient()
         val request = HttpRequest.newBuilder(URI("$siloUrl/query"))
             .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
-            .POST(HttpRequest.BodyPublishers.ofString(objectMapper.writeValueAsString(query)))
+            .POST(HttpRequest.BodyPublishers.ofString(queryJson))
             .build()
 
-        log.info { "Calling SILO: $query" }
-
-        val response =
-            try {
-                client.send(request, BodyHandlers.ofString())
-            } catch (exception: Exception) {
-                val message = "Could not connect to silo: " + exception::class.toString() + " " + exception.message
-                throw SiloException(message, exception)
-            }
+        val response = try {
+            client.send(request, BodyHandlers.ofString())
+        } catch (exception: Exception) {
+            val message = "Could not connect to silo: " + exception::class.toString() + " " + exception.message
+            throw RuntimeException(message, exception)
+        }
 
         log.info { "Response from SILO: ${response.statusCode()} - ${response.body()}" }
 
         if (response.statusCode() != 200) {
-            throw SiloException(response.body(), null)
+            val siloErrorResponse = try {
+                objectMapper.readValue<SiloErrorResponse>(response.body())
+            } catch (e: Exception) {
+                log.error { "Failed to deserialize error response from SILO: $e" }
+                throw SiloException(
+                    HttpStatus.INTERNAL_SERVER_ERROR.value(),
+                    "Internal Server Error",
+                    "Unexpected error from SILO: ${response.body()}",
+                )
+            }
+            throw SiloException(
+                response.statusCode(),
+                siloErrorResponse.error,
+                "Error from SILO: " + siloErrorResponse.message,
+            )
         }
 
         addDataVersion(response)
@@ -52,7 +69,7 @@ class SiloClient(
             return objectMapper.readValue(response.body(), query.action.typeReference).queryResult
         } catch (exception: Exception) {
             val message = "Could not parse response from silo: " + exception::class.toString() + " " + exception.message
-            throw SiloException(message, exception)
+            throw RuntimeException(message, exception)
         }
     }
 
@@ -61,8 +78,10 @@ class SiloClient(
     }
 }
 
-class SiloException(message: String?, cause: Throwable?) : Exception(message, cause)
+class SiloException(val statusCode: Int, val title: String, override val message: String) : Exception(message)
 
 data class SiloQueryResponse<ResponseType>(
     val queryResult: ResponseType,
 )
+
+data class SiloErrorResponse(val error: String, val message: String)
diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/ExceptionHandlerTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/ExceptionHandlerTest.kt
@@ -65,18 +65,18 @@ class ExceptionHandlerTest(@Autowired val mockMvc: MockMvc) {
     }
 
     @Test
-    fun `throw INTERNAL_SERVER_ERROR(500) with additional info for SiloExceptions`() {
-        every { validControllerCall() } throws SiloException("SomeMessage", Exception("SomeCause"))
+    fun `Passes through exception with status code from SILO`() {
+        every { validControllerCall() } throws SiloException(123, "SomeTitle", "SomeMessage",)
 
         mockMvc.perform(get(validRoute))
-            .andExpect(status().isInternalServerError)
+            .andExpect(status().`is`(123))
             .andExpect(content().contentType(MediaType.APPLICATION_JSON))
             .andExpect(
                 content().json(
                     """
                     {
                         "error": {
-                            "title": "Silo error",
+                            "title": "SomeTitle",
                             "message": "SomeMessage"
                          }
                     }

diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloClientTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloClientTest.kt
@@ -249,18 +249,38 @@ class SiloClientTest {
     }
 
     @Test
-    fun `given server returns error then throws exception`() {
+    fun `given server returns error in unexpected format then throws exception`() {
         expectQueryRequestAndRespondWith(
             response()
                 .withContentType(MediaType.APPLICATION_JSON_UTF_8)
-                .withStatusCode(500)
-                .withBody("""{"someError":  "some message"}"""),
+                .withStatusCode(432)
+                .withBody("""{"unexpectedKey":  "some unexpected message"}"""),
         )
 
         val query = SiloQuery(SiloAction.aggregated(), StringEquals("theColumn", "theValue"))
 
         val exception = assertThrows<SiloException> { underTest.sendQuery(query) }
-        assertThat(exception.message, equalTo("""{"someError":  "some message"}"""))
+        assertThat(exception.statusCode, equalTo(500))
+        assertThat(
+            exception.message,
+            equalTo("""Unexpected error from SILO: {"unexpectedKey":  "some unexpected message"}"""),
+        )
+    }
+
+    @Test
+    fun `given server returns SILO error then throws exception with details and response code`() {
+        expectQueryRequestAndRespondWith(
+            response()
+                .withContentType(MediaType.APPLICATION_JSON_UTF_8)
+                .withStatusCode(432)
+                .withBody("""{"error":  "Test Error", "message": "test message with details"}"""),
+        )
+
+        val query = SiloQuery(SiloAction.aggregated(), StringEquals("theColumn", "theValue"))
+
+        val exception = assertThrows<SiloException> { underTest.sendQuery(query) }
+        assertThat(exception.statusCode, equalTo(432))
+        assertThat(exception.message, equalTo("Error from SILO: test message with details"))
     }
 
     @Test
@@ -274,7 +294,7 @@ class SiloClientTest {
 
         val query = SiloQuery(SiloAction.aggregated(), StringEquals("theColumn", "theValue"))
 
-        val exception = assertThrows<SiloException> { underTest.sendQuery(query) }
+        val exception = assertThrows<RuntimeException> { underTest.sendQuery(query) }
         assertThat(exception.message, containsString("value failed for JSON property"))
     }