RUM-5977 include optional exception in Upload Status

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/data/upload/DataOkHttpUploader.kt

@@ -16,6 +16,7 @@ import okhttp3.Call
 import okhttp3.MediaType.Companion.toMediaTypeOrNull
 import okhttp3.Request
 import okhttp3.RequestBody.Companion.toRequestBody
+import java.io.IOException
 import java.net.UnknownHostException
 import java.util.Locale
 import com.datadog.android.api.net.Request as DatadogRequest
@@ -38,7 +39,7 @@ internal class DataOkHttpUploader(
     ): UploadStatus {
         val request = try {
             requestFactory.create(context, batch, batchMeta)
-                ?: return UploadStatus.RequestCreationError
+                ?: return UploadStatus.RequestCreationError(null)
         } catch (e: Exception) {
             internalLogger.log(
                 InternalLogger.Level.ERROR,
@@ -49,7 +50,7 @@ internal class DataOkHttpUploader(
                 },
                 e
             )
-            return UploadStatus.RequestCreationError
+            return UploadStatus.RequestCreationError(e)
         }
 
         val uploadStatus = try {
@@ -61,15 +62,23 @@ internal class DataOkHttpUploader(
                 { "Unable to find host for site ${context.site}; we will retry later." },
                 e
             )
-            UploadStatus.DNSError
+            UploadStatus.DNSError(e)
+        } catch (e: IOException) {
+            internalLogger.log(
+                InternalLogger.Level.ERROR,
+                InternalLogger.Target.USER,
+                { "Unable to execute the request; we will retry later." },
+                e
+            )
+            UploadStatus.NetworkError(e)
         } catch (e: Throwable) {
             internalLogger.log(
                 InternalLogger.Level.ERROR,
                 InternalLogger.Target.USER,
                 { "Unable to execute the request; we will retry later." },
                 e
             )
-            UploadStatus.NetworkError
+            UploadStatus.UnknownException(throwable = e)
         }
 
         uploadStatus.logStatus(
@@ -181,7 +190,7 @@ internal class DataOkHttpUploader(
                     listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY),
                     { "Unexpected status code $code on upload request: ${request.description}" }
                 )
-                UploadStatus.UnknownError(code)
+                UploadStatus.UnknownHttpError(code)
             }
         }
     }

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/data/upload/UploadStatus.kt

@@ -8,114 +8,133 @@ package com.datadog.android.core.internal.data.upload
 
 import com.datadog.android.api.InternalLogger
 
-@Suppress("StringLiteralDuplication")
-internal sealed class UploadStatus(val shouldRetry: Boolean = false, val code: Int = UNKNOWN_RESPONSE_CODE) {
+internal sealed class UploadStatus(
+    val shouldRetry: Boolean = false,
+    val code: Int = UNKNOWN_RESPONSE_CODE,
+    val throwable: Throwable? = null
+) {
 
     internal class Success(responseCode: Int) : UploadStatus(shouldRetry = false, code = responseCode)
-    internal object NetworkError : UploadStatus(shouldRetry = true)
-    internal object DNSError : UploadStatus(shouldRetry = true)
-    internal object RequestCreationError : UploadStatus(shouldRetry = false)
+    internal class NetworkError(throwable: Throwable) : UploadStatus(shouldRetry = true, throwable = throwable)
+    internal class DNSError(throwable: Throwable) : UploadStatus(shouldRetry = true, throwable = throwable)
+    internal class RequestCreationError(throwable: Throwable?) :
+        UploadStatus(shouldRetry = false, throwable = throwable)
+
     internal class InvalidTokenError(responseCode: Int) : UploadStatus(shouldRetry = false, code = responseCode)
     internal class HttpRedirection(responseCode: Int) : UploadStatus(shouldRetry = false, code = responseCode)
     internal class HttpClientError(responseCode: Int) : UploadStatus(shouldRetry = false, code = responseCode)
     internal class HttpServerError(responseCode: Int) : UploadStatus(shouldRetry = true, code = responseCode)
     internal class HttpClientRateLimiting(responseCode: Int) : UploadStatus(shouldRetry = true, code = responseCode)
-    internal class UnknownError(responseCode: Int) : UploadStatus(shouldRetry = false, code = responseCode)
+    internal class UnknownHttpError(responseCode: Int) : UploadStatus(shouldRetry = false, code = responseCode)
+    internal class UnknownException(throwable: Throwable) : UploadStatus(shouldRetry = true, throwable = throwable)
 
     internal object UnknownStatus : UploadStatus(shouldRetry = false, code = UNKNOWN_RESPONSE_CODE)
 
-    @SuppressWarnings("LongMethod")
     fun logStatus(
         context: String,
         byteSize: Int,
         logger: InternalLogger,
         requestId: String? = null
     ) {
-        val batchInfo = if (requestId == null) {
-            "Batch [$byteSize bytes] ($context)"
-        } else {
-            "Batch $requestId [$byteSize bytes] ($context)"
+        val level = when (this) {
+            is HttpClientError,
+            is HttpServerError,
+            is InvalidTokenError,
+            is RequestCreationError,
+            is UnknownException,
+            is UnknownHttpError -> InternalLogger.Level.ERROR
+
+            is DNSError,
+            is HttpClientRateLimiting,
+            is HttpRedirection,
+            is NetworkError -> InternalLogger.Level.WARN
+
+            is Success -> InternalLogger.Level.INFO
+
+            else -> InternalLogger.Level.VERBOSE
         }
-        when (this) {
-            is NetworkError -> logger.log(
-                InternalLogger.Level.WARN,
-                InternalLogger.Target.USER,
-                { "$batchInfo failed because of a network error; we will retry later." }
-            )
-
-            is DNSError -> logger.log(
-                InternalLogger.Level.WARN,
-                InternalLogger.Target.USER,
-                { "$batchInfo failed because of a DNS error; we will retry later." }
-            )
-
-            is InvalidTokenError -> logger.log(
-                InternalLogger.Level.ERROR,
-                InternalLogger.Target.USER,
-                {
-                    "$batchInfo failed because your token is invalid; the batch was dropped. " +
-                        "Make sure that the provided token still exists " +
-                        "and you're targeting the relevant Datadog site."
-                }
-            )
-
-            is HttpRedirection -> logger.log(
-                InternalLogger.Level.WARN,
-                InternalLogger.Target.USER,
-                { "$batchInfo failed because of a network redirection; the batch was dropped." }
-            )
-
-            is HttpClientError -> {
-                logger.log(
-                    InternalLogger.Level.ERROR,
-                    listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
-                    {
-                        "$batchInfo failed because of a processing error or invalid data; " +
-                            "the batch was dropped."
-                    }
-                )
+
+        val targets = when (this) {
+            is HttpClientError,
+            is HttpClientRateLimiting -> listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY)
+
+            is DNSError,
+            is HttpRedirection,
+            is HttpServerError,
+            is InvalidTokenError,
+            is NetworkError,
+            is RequestCreationError,
+            is Success,
+            is UnknownException,
+            is UnknownHttpError -> listOf(InternalLogger.Target.USER)
+
+            else -> emptyList()
+        }
+
+        logger.log(
+            level,
+            targets,
+            {
+                buildStatusMessage(requestId, byteSize, context, throwable)
             }
+        )
+    }
 
-            is HttpClientRateLimiting -> {
-                logger.log(
-                    InternalLogger.Level.WARN,
-                    listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
-                    { "$batchInfo not uploaded due to rate limitation; we will retry later." }
-                )
+    private fun buildStatusMessage(
+        requestId: String?,
+        byteSize: Int,
+        context: String,
+        throwable: Throwable?
+    ): String {
+        return buildString {
+            if (requestId == null) {
+                append("Batch [$byteSize bytes] ($context)")
+            } else {
+                append("Batch $requestId [$byteSize bytes] ($context)")
             }
 
-            is HttpServerError -> logger.log(
-                InternalLogger.Level.ERROR,
-                InternalLogger.Target.USER,
-                { "$batchInfo failed because of a server processing error; we will retry later." }
-            )
-
-            is UnknownError -> logger.log(
-                InternalLogger.Level.ERROR,
-                InternalLogger.Target.USER,
-                { "$batchInfo failed because of an unknown error (status code = $code); the batch was dropped." }
-            )
-
-            is RequestCreationError -> logger.log(
-                InternalLogger.Level.ERROR,
-                InternalLogger.Target.USER,
-                {
-                    "$batchInfo failed because of an error when creating the request; " +
-                        "the batch was dropped."
+            if (this@UploadStatus is Success) {
+                append(" sent successfully.")
+            } else if (this@UploadStatus is UnknownStatus) {
+                append(" status is unknown")
+            } else {
+                append(" failed because ")
+                when (this@UploadStatus) {
+                    is DNSError -> append("of a DNS error")
+                    is HttpClientError -> append("of a processing error or invalid data")
+                    is HttpClientRateLimiting -> append("of an intake rate limitation")
+                    is HttpRedirection -> append("of a network redirection")
+                    is HttpServerError -> append("of a server processing error")
+                    is InvalidTokenError -> append("your token is invalid")
+                    is NetworkError -> append("of a network error")
+                    is RequestCreationError -> append("of an error when creating the request")
+                    is UnknownException -> append("of an unknown error")
+                    is UnknownHttpError -> append("of an unexpected HTTP error (status code = $code)")
+                    else -> {}
                 }
-            )
 
-            is Success -> logger.log(
-                InternalLogger.Level.INFO,
-                InternalLogger.Target.USER,
-                { "$batchInfo sent successfully." }
-            )
+                if (throwable != null) {
+                    append(" (")
+                    append(throwable.message)
+                    append(")")
+                }
 
-            else -> {
-                // no-op
+                if (shouldRetry) {
+                    append("; we will retry later.")
+                } else {
+                    append("; the batch was dropped.")
+                }
+            }
+
+            if (this@UploadStatus is InvalidTokenError) {
+                append(
+                    " Make sure that the provided token still exists " +
+                        "and you're targeting the relevant Datadog site."
+                )
             }
         }
     }
+
     companion object {
         internal const val UNKNOWN_RESPONSE_CODE = 0
     }

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/data/upload/DataOkHttpUploaderTest.kt

@@ -514,7 +514,7 @@ internal class DataOkHttpUploaderTest {
         val result = testedUploader.upload(fakeContext, batch, batchMetadata)
 
         // Then
-        assertThat(result).isInstanceOf(UploadStatus.UnknownError::class.java)
+        assertThat(result).isInstanceOf(UploadStatus.UnknownHttpError::class.java)
         assertThat(result.code).isEqualTo(statusCode)
         verifyRequest(fakeDatadogRequest)
         verifyResponseIsClosed()
@@ -572,7 +572,7 @@ internal class DataOkHttpUploaderTest {
         val result = testedUploader.upload(fakeContext, batch, batchMetadata)
 
         // Then
-        assertThat(result).isInstanceOf(UploadStatus.NetworkError::class.java)
+        assertThat(result).isInstanceOf(UploadStatus.UnknownException::class.java)
         assertThat(result.code).isEqualTo(UploadStatus.UNKNOWN_RESPONSE_CODE)
         verifyRequest(fakeDatadogRequest)
     }

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/data/upload/DataUploadRunnableTest.kt

@@ -19,6 +19,7 @@ import com.datadog.android.core.internal.persistence.Storage
 import com.datadog.android.core.internal.system.SystemInfo
 import com.datadog.android.core.internal.system.SystemInfoProvider
 import com.datadog.android.utils.forge.Configurator
+import com.datadog.tools.unit.forge.anException
 import fr.xgouchet.elmyr.Forge
 import fr.xgouchet.elmyr.annotation.Forgery
 import fr.xgouchet.elmyr.annotation.IntForgery
@@ -850,7 +851,7 @@ internal class DataUploadRunnableTest {
                     batch,
                     batchMetadata[index]
                 )
-            ) doReturn UploadStatus.DNSError
+            ) doReturn UploadStatus.DNSError(forge.anException())
         }
 
         // When
@@ -1025,9 +1026,10 @@ internal class DataUploadRunnableTest {
             }
 
             return listOf(
-                forge.getForgery(UploadStatus.HttpServerError::class.java),
                 forge.getForgery(UploadStatus.HttpClientRateLimiting::class.java),
-                forge.getForgery(UploadStatus.NetworkError::class.java)
+                forge.getForgery(UploadStatus.HttpServerError::class.java),
+                forge.getForgery(UploadStatus.NetworkError::class.java),
+                forge.getForgery(UploadStatus.UnknownException::class.java)
             )
         }
 
@@ -1040,7 +1042,7 @@ internal class DataUploadRunnableTest {
             return listOf(
                 forge.getForgery(UploadStatus.HttpClientError::class.java),
                 forge.getForgery(UploadStatus.HttpRedirection::class.java),
-                forge.getForgery(UploadStatus.UnknownError::class.java),
+                forge.getForgery(UploadStatus.UnknownHttpError::class.java),
                 forge.getForgery(UploadStatus.UnknownStatus::class.java)
             )
         }