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 <[email protected]>
oss-review-toolkit · May 4, 2022 · 5af9ba1 · 5af9ba1
1 parent fed5ead
commit 5af9ba1
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 1 deletion.
diff --git 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
@@ -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
+        }
+    }
 })