Network error mappings improvements:

 - Add NetworkConnectivityException to configureExceptionErrorMapping
 - Now configureExceptionErrorMapping sets expectSuccess = false
 - Changes on GenericNetworkDataSource
 - Add tests for network error mapping

Author: joselufo

harmony-kotlin/build.gradle

@@ -118,6 +118,7 @@ kotlin {
       dependencies {
         implementation kotlin('test-common')
         implementation kotlin('test-annotations-common')
+        implementation("io.ktor:ktor-client-mock:$ktor_version")
       }
     }
 

harmony-kotlin/src/commonMain/kotlin/com.harmony.kotlin/data/datasource/network/GenericNetworkDataSource.kt

@@ -4,6 +4,7 @@ import com.harmony.kotlin.data.datasource.DeleteDataSource
 import com.harmony.kotlin.data.datasource.GetDataSource
 import com.harmony.kotlin.data.datasource.PutDataSource
 import com.harmony.kotlin.data.error.QueryNotSupportedException
+import com.harmony.kotlin.data.mapper.IdentityMapper
 import com.harmony.kotlin.data.mapper.Mapper
 import com.harmony.kotlin.data.query.Query
 import io.ktor.client.HttpClient
@@ -12,23 +13,21 @@ import kotlinx.serialization.builtins.ListSerializer
 import kotlinx.serialization.builtins.serializer
 import kotlinx.serialization.json.Json
 
-open class GetNetworkDataSource<T>(
+class GetNetworkDataSource<T>(
   private val url: String,
   private val httpClient: HttpClient,
   private val serializer: KSerializer<T>,
   private val json: Json,
   private val globalHeaders: List<Pair<String, String>> = emptyList(),
-  private val exceptionMapper: Mapper<Exception, Exception> = GenericNetworkExceptionMapper()
+  private val exceptionMapper: Mapper<Exception, Exception> = IdentityMapper()
 ) : GetDataSource<T> {
 
   /**
    * GET request returning an object
    */
   override suspend fun get(query: Query): T {
-    val response: String = try {
+    val response: String = tryOrThrow(exceptionMapper) {
       executeGetRequest(query)
-    } catch (e: Exception) {
-      throw exceptionMapper.map(e)
     }
     return json.decodeFromString(serializer, response)
 
@@ -38,11 +37,10 @@ open class GetNetworkDataSource<T>(
    * GET request returning a list of objects
    */
   override suspend fun getAll(query: Query): List<T> {
-    val response: String = try {
+    val response: String = tryOrThrow(exceptionMapper) {
       executeGetRequest(query)
-    } catch (e: Exception) {
-      throw exceptionMapper.map(e)
     }
+
     return json.decodeFromString(ListSerializer(serializer), response)
 
   }
@@ -63,26 +61,25 @@ open class GetNetworkDataSource<T>(
   }
 }
 
-open class PutNetworkDataSource<T>(
+class PutNetworkDataSource<T>(
   private val url: String,
   private val httpClient: HttpClient,
   private val serializer: KSerializer<T>,
   private val json: Json,
   private val globalHeaders: List<Pair<String, String>> = emptyList(),
-  private val exceptionMapper: Mapper<Exception, Exception> = GenericNetworkExceptionMapper()
+  private val exceptionMapper: Mapper<Exception, Exception> = IdentityMapper()
+
 ) : PutDataSource<T> {
 
   /**
    * POST or PUT request returning an object
    * @throws IllegalArgumentException if both value and content-type of the query method are defined
    */
   override suspend fun put(query: Query, value: T?): T {
-    val response: String = try {
+    val response: String = tryOrThrow(exceptionMapper) {
       validateQuery(query)
         .sanitizeContentType(value)
         .executeKtorRequest(httpClient = httpClient, baseUrl = url, globalHeaders = globalHeaders)
-    } catch (e: Exception) {
-      throw exceptionMapper.map(e)
     }
 
     return if (serializer.descriptor != Unit.serializer().descriptor) {
@@ -97,14 +94,11 @@ open class PutNetworkDataSource<T>(
    * @throws IllegalArgumentException if both value and content-type of the query method are defined
    */
   override suspend fun putAll(query: Query, value: List<T>?): List<T> {
-    val response: String = try {
+    val response: String = tryOrThrow(exceptionMapper) {
       validateQuery(query)
         .sanitizeContentType(value)
         .executeKtorRequest(httpClient = httpClient, baseUrl = url, globalHeaders = globalHeaders)
-    } catch (e: Exception) {
-      throw exceptionMapper.map(e)
     }
-
     return if (serializer.descriptor != Unit.serializer().descriptor) {
       json.decodeFromString(ListSerializer(serializer), response)
     } else { // If Unit.serializer() is used is because we want to ignore the response and just return an empty list of Unit
@@ -153,17 +147,15 @@ class DeleteNetworkDataSource(
   private val url: String,
   private val httpClient: HttpClient,
   private val globalHeaders: List<Pair<String, String>> = emptyList(),
-  private val exceptionMapper: Mapper<Exception, Exception> = GenericNetworkExceptionMapper()
+  private val exceptionMapper: Mapper<Exception, Exception> = IdentityMapper()
 ) : DeleteDataSource {
 
   /**
    * DELETE request
    */
   override suspend fun delete(query: Query) {
-    try {
+    tryOrThrow(exceptionMapper) {
       validateQuery(query).executeKtorRequest(httpClient = httpClient, baseUrl = url, globalHeaders = globalHeaders)
-    } catch (e: Exception) {
-      throw exceptionMapper.map(e)
     }
   }
 
@@ -179,3 +171,11 @@ class DeleteNetworkDataSource(
     return query
   }
 }
+
+private suspend fun <T> tryOrThrow(exceptionMapper: Mapper<Exception, Exception>, block: suspend () -> T): T {
+  return try {
+    block()
+  } catch (e: Exception) {
+    throw exceptionMapper.map(e)
+  }
+}

harmony-kotlin/src/commonMain/kotlin/com.harmony.kotlin/data/datasource/network/GenericNetworkExceptionMapper.kt

@@ -4,24 +4,40 @@ import com.harmony.kotlin.common.exceptions.tryOrNull
 import com.harmony.kotlin.data.datasource.network.DefaultUnauthorizedResolution
 import com.harmony.kotlin.data.datasource.network.UnauthorizedResolution
 import com.harmony.kotlin.data.datasource.network.error.HttpException
+import com.harmony.kotlin.data.datasource.network.error.NetworkConnectivityException
 import com.harmony.kotlin.data.error.DataException
 import com.harmony.kotlin.data.error.DataNotFoundException
 import com.harmony.kotlin.data.error.UnauthorizedException
 import io.ktor.client.HttpClientConfig
-import io.ktor.client.call.receive
 import io.ktor.client.features.HttpResponseValidator
 import io.ktor.client.statement.HttpResponse
+import io.ktor.client.statement.readText
 import io.ktor.http.HttpStatusCode
 import io.ktor.http.isSuccess
+import io.ktor.utils.io.charsets.Charsets
+import io.ktor.utils.io.errors.IOException
 
+/**
+ * Helper that configures the error mapping by:
+ *  - Setting expectSuccess to false
+ *  - Adding a HttpResponseValidator used to map errors
+ */
 fun HttpClientConfig<*>.configureExceptionErrorMapping(unauthorizedResolution: UnauthorizedResolution = DefaultUnauthorizedResolution) {
+  expectSuccess = false
+
   HttpResponseValidator {
     validateResponse { response ->
       val httpStatus = response.status
       if (!httpStatus.isSuccess()) {
         throw response.toDataException(unauthorizedResolution)
       }
     }
+
+    handleResponseException {
+      if (it is IOException) {
+        throw NetworkConnectivityException(cause = it)
+      }
+    }
   }
 }
 
@@ -39,8 +55,8 @@ suspend fun HttpResponse.toDataException(unauthorizedResolution: UnauthorizedRes
   }
 }
 
-suspend fun HttpResponse.contentAsString(): String? {
+private suspend fun HttpResponse.contentAsString(): String? {
   return tryOrNull {
-    this.receive<String>()
+    this.readText(Charsets.UTF_8)
   }
 }

harmony-kotlin/src/commonMain/kotlin/com.harmony.kotlin/data/datasource/network/ktor/KtorConfig.kt

@@ -0,0 +1,10 @@
+package com.harmony.kotlin.data.mapper
+
+/**
+ * Mapper that returns the same object obtained
+ */
+class IdentityMapper<T> : Mapper<T, T> {
+  override fun map(from: T): T {
+    return from
+  }
+}

harmony-kotlin/src/commonMain/kotlin/com.harmony.kotlin/data/mapper/IdentityMapper.kt

@@ -0,0 +1,108 @@
+package com.harmony.kotlin.data.datasource.network
+
+import com.harmony.kotlin.data.datasource.network.ktor.configureExceptionErrorMapping
+import io.ktor.client.HttpClient
+import io.ktor.client.engine.mock.MockEngine
+import io.ktor.client.engine.mock.respond
+import io.ktor.client.features.json.JsonFeature
+import io.ktor.client.features.json.serializer.KotlinxSerializer
+import io.ktor.client.request.request
+import io.ktor.client.request.url
+import io.ktor.http.ContentType
+import io.ktor.http.Headers
+import io.ktor.http.HttpMethod
+import io.ktor.http.HttpStatusCode
+import io.ktor.http.Url
+import io.ktor.http.headersOf
+import kotlinx.serialization.json.Json
+
+class ApiMock {
+  private val client = HttpClient(ApiMockEngine().get()) {
+    install(JsonFeature) {
+      serializer = KotlinxSerializer(
+        Json {
+          isLenient = false
+          ignoreUnknownKeys = true
+        }
+      )
+    }
+
+    configureExceptionErrorMapping()
+  }
+
+  suspend fun executeRequest(request: MockRequest) {
+    request.executeRequest(client)
+  }
+
+}
+
+private class ApiMockEngine {
+  fun get() = client.engine
+
+  private val client = HttpClient(MockEngine) {
+    val mockRequests = listOf(
+      UnauthorizedRequest,
+      NotFoundRequest,
+      BadRequest
+    )
+
+    engine {
+      addHandler { request ->
+        try {
+          val mockRequest = mockRequests.first {
+            it.url == request.url
+          }
+          respond(mockRequest.responseBody, mockRequest.statusCode, mockRequest.responseHeaders)
+        } catch (_: Exception) {
+          error("Unhandled ${request.url.encodedPath}")
+        }
+      }
+    }
+  }
+}
+
+interface MockRequest {
+  val method: HttpMethod
+  val url: Url
+  val responseHeaders: Headers
+    get() {
+      return headersOf("Content-Type" to listOf(ContentType.Application.Json.toString()))
+    }
+  val responseBody: String
+    get() = ""
+  val statusCode: HttpStatusCode
+
+  suspend fun executeRequest(client: HttpClient) {
+    client.request<String> {
+      method = HttpMethod.Get
+      url(this@MockRequest.url)
+    }
+  }
+}
+
+object UnauthorizedRequest : MockRequest {
+  override val method: HttpMethod = HttpMethod.Get
+  override val url = Url("https://mockrequest.com/unauthorized")
+  override val statusCode: HttpStatusCode = HttpStatusCode.Unauthorized
+}
+
+object NotFoundRequest : MockRequest {
+  override val method: HttpMethod = HttpMethod.Get
+  override val url = Url("https://mockrequest.com/not_found")
+  override val statusCode: HttpStatusCode = HttpStatusCode.NotFound
+}
+
+object BadRequest : MockRequest {
+  override val method: HttpMethod = HttpMethod.Get
+  override val url = Url("https://mockrequest.com/bad_request")
+  override val responseBody: String = "bad_request"
+  override val statusCode: HttpStatusCode = HttpStatusCode.BadRequest
+}
+
+object InvalidJsonResponseRequest : MockRequest {
+  override val method: HttpMethod = HttpMethod.Get
+  override val url = Url("https://mockrequest.com/invalid_json_response")
+  override val responseBody: String = "{ \"fo_o\": \"bar\" }"
+  override val statusCode: HttpStatusCode = HttpStatusCode.OK
+}
+

harmony-kotlin/src/commonTest/kotlin/com/harmony/kotlin/data/datasource/network/ApiMock.kt

@@ -0,0 +1,49 @@
+@file:Suppress("IllegalIdentifier")
+
+package com.harmony.kotlin.data.datasource.network
+
+import com.harmony.kotlin.common.BaseTest
+import com.harmony.kotlin.data.datasource.network.error.HttpException
+import com.harmony.kotlin.data.error.DataNotFoundException
+import com.harmony.kotlin.data.error.UnauthorizedException
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
+import kotlin.test.fail
+
+class NetworkErrorMappingTests : BaseTest() {
+
+  private val apiMock: ApiMock = ApiMock()
+
+  @Test
+  fun `should throw UnauthorizedException when backend returns 401`() {
+    assertFailsWith<UnauthorizedException> {
+      runTest {
+        apiMock.executeRequest(UnauthorizedRequest)
+      }
+    }
+  }
+
+  @Test
+  fun `should throw DataNotFound when backend returns 404`() {
+    assertFailsWith<DataNotFoundException> {
+      runTest {
+        apiMock.executeRequest(NotFoundRequest)
+      }
+    }
+  }
+
+  @Test
+  fun `should throw HttpException when backend returns any 40X (minus 401 & 404) & 50X`() {
+    try {
+      runTest {
+        apiMock.executeRequest(BadRequest)
+      }
+    } catch (e: HttpException) {
+      assertEquals(e.statusCode, BadRequest.statusCode.value)
+      assertEquals(e.response, BadRequest.responseBody)
+    } catch (e: Exception) {
+      fail()
+    }
+  }
+}

harmony-kotlin/src/commonTest/kotlin/com/harmony/kotlin/data/datasource/network/NetworkErrorMappingTests.kt

@@ -2,6 +2,7 @@ package com.mobilejazz.kmmsample.core
 
 import com.harmony.kotlin.common.logger.KtorHarmonyLogger
 import com.harmony.kotlin.common.logger.Logger
+import com.harmony.kotlin.data.datasource.network.ktor.configureExceptionErrorMapping
 import io.ktor.client.HttpClient
 import io.ktor.client.features.json.JsonFeature
 import io.ktor.client.features.json.serializer.KotlinxSerializer
@@ -44,7 +45,7 @@ class NetworkDefaultModule(private val coreLogger: Logger) : NetworkComponent {
         level = LogLevel.HEADERS
       }
 
-      expectSuccess = false
+      configureExceptionErrorMapping()
     }
   }
 }