From 5af9ba1ad642ec827577c96f4b699008ad444c9e Mon Sep 17 00:00:00 2001
From: Oliver Heger <oliver.heger@bosch.io>
Date: Wed, 4 May 2022 08:09:35 +0200
Subject: [PATCH] OkHttpClientHelper: Improve exception handling in
 downloadText()

Make sure that exceptions thrown when accessing the response body are
handled by replacing map with mapCatching. At the time the response is
available, the download of the body is not necessarily complete (for
instance when using chunked transfer encoding); so there is a risk of
potential I/O exceptions.

Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
---
 .../src/main/kotlin/OkHttpClientHelper.kt     |  2 +-
 .../src/test/kotlin/OkHttpClientHelperTest.kt | 24 +++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/utils/core/src/main/kotlin/OkHttpClientHelper.kt b/utils/core/src/main/kotlin/OkHttpClientHelper.kt
index 17b9124072018..96cc19268385a 100644
--- a/utils/core/src/main/kotlin/OkHttpClientHelper.kt
+++ b/utils/core/src/main/kotlin/OkHttpClientHelper.kt
@@ -219,7 +219,7 @@ fun OkHttpClient.downloadFile(url: String, directory: File): Result<File> =
  * [Result] wrapping an [IOException] (which might be a [HttpDownloadError]) on failure.
  */
 fun OkHttpClient.downloadText(url: String): Result<String> =
-    download(url).map { (_, body) ->
+    download(url).mapCatching { (_, body) ->
         body.use { it.string() }
     }
 
diff --git a/utils/core/src/test/kotlin/OkHttpClientHelperTest.kt b/utils/core/src/test/kotlin/OkHttpClientHelperTest.kt
index 9a5de32769f35..2cb4245ac63d3 100644
--- a/utils/core/src/test/kotlin/OkHttpClientHelperTest.kt
+++ b/utils/core/src/test/kotlin/OkHttpClientHelperTest.kt
@@ -23,6 +23,7 @@ import io.kotest.core.spec.style.StringSpec
 import io.kotest.engine.spec.tempdir
 import io.kotest.matchers.result.shouldBeFailure
 import io.kotest.matchers.should
+import io.kotest.matchers.shouldBe
 import io.kotest.matchers.types.beInstanceOf
 import io.kotest.matchers.types.shouldBeSameInstanceAs
 
@@ -81,4 +82,27 @@ class OkHttpClientHelperTest : StringSpec({
             it should beInstanceOf<IOException>()
         }
     }
+
+    "Exceptions when accessing the response body should be handled" {
+        val failureUrl = "https://example.org/fault/"
+        val exception = IOException("Connection closed")
+
+        mockkStatic(OkHttpClient::download)
+        val response = mockk<Response>(relaxed = true)
+        val request = mockk<Request>(relaxed = true)
+        val body = mockk<ResponseBody>(relaxed = true)
+
+        every { request.url } returns failureUrl.toHttpUrl()
+        every { response.headers(any()) } returns emptyList()
+        every { response.request } returns request
+        every { body.string() } throws exception
+
+        val client = OkHttpClientHelper.buildClient()
+        every { client.download(any(), any()) } returns Result.success(response to body)
+
+        val result = client.downloadText(failureUrl)
+        result.shouldBeFailure {
+            it shouldBe exception
+        }
+    }
 })