Avoid throwing exceptions up the stack

DataDog · Feb 8, 2024 · 67c70a6 · 67c70a6
1 parent a65435f
commit 67c70a6
Show file tree

Hide file tree

Showing 17 changed files with 250 additions and 154 deletions.
diff --git a/dd-sdk-android-core/api/apiSurface b/dd-sdk-android-core/api/apiSurface
@@ -116,7 +116,7 @@ interface com.datadog.android.api.feature.StorageBackedFeature : Feature
 data class com.datadog.android.api.net.Request
   constructor(String, String, String, Map<String, String>, ByteArray, String? = null)
 interface com.datadog.android.api.net.RequestFactory
-  fun create(com.datadog.android.api.context.DatadogContext, List<com.datadog.android.api.storage.RawBatchEvent>, ByteArray?): Request
+  fun create(com.datadog.android.api.context.DatadogContext, List<com.datadog.android.api.storage.RawBatchEvent>, ByteArray?): Request?
   companion object 
     const val CONTENT_TYPE_JSON: String
     const val CONTENT_TYPE_TEXT_UTF8: String

diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/net/RequestFactory.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/net/RequestFactory.kt
@@ -25,7 +25,7 @@ fun interface RequestFactory {
         context: DatadogContext,
         batchData: List<RawBatchEvent>,
         batchMetadata: ByteArray?
-    ): Request
+    ): Request?
 
     companion object {
         /**

diff --git a/...-core/src/main/kotlin/com/datadog/android/core/internal/data/upload/DataOkHttpUploader.kt b/...-core/src/main/kotlin/com/datadog/android/core/internal/data/upload/DataOkHttpUploader.kt
@@ -29,14 +29,15 @@ internal class DataOkHttpUploader(
 
     // region DataUploader
 
-    @Suppress("TooGenericExceptionCaught")
+    @Suppress("TooGenericExceptionCaught", "ReturnCount")
     override fun upload(
         context: DatadogContext,
         batch: List<RawBatchEvent>,
         batchMeta: ByteArray?
     ): UploadStatus {
         val request = try {
             requestFactory.create(context, batch, batchMeta)
+                ?: return UploadStatus.RequestCreationError
         } catch (e: Exception) {
             internalLogger.log(
                 InternalLogger.Level.ERROR,

diff --git a/detekt_custom.yml b/detekt_custom.yml
@@ -230,6 +230,8 @@ datadog:
       # endregion
       # region Kotlin Collections
       - "kotlin.Array.first(kotlin.Function1):java.util.NoSuchElementException"
+      - "kotlin.ByteArray.toRequestBody(okhttp3.MediaType?, kotlin.Int, kotlin.Int):java.lang.ArrayIndexOutOfBoundsException,java.io.IOException"
+      - "kotlin.String.toRequestBody(okhttp3.MediaType?):java.lang.ArrayIndexOutOfBoundsException,java.io.IOException"
       - "kotlin.collections.List.last():java.util.NoSuchElementException"
       - "kotlin.collections.List.first():java.util.NoSuchElementException"
       - "kotlin.collections.List.get(kotlin.Int):java.util.NoSuchElementException"
@@ -247,6 +249,8 @@ datadog:
       - "okhttp3.Call.execute():java.io.IOException"
       - "okhttp3.Dns.lookup(kotlin.String):java.net.UnknownHostException"
       - "okhttp3.HttpUrl.toUrl():java.lang.RuntimeException"
+      - "okhttp3.MultipartBody.Builder.build():java.lang.IllegalStateException"
+      - "okhttp3.MultipartBody.Builder.setType(okhttp3.MediaType):java.lang.IllegalArgumentException"
       - "okhttp3.Response.peekBody(kotlin.Long):java.io.IOException,java.lang.IllegalArgumentException,java.lang.IllegalStateException"
       - "okhttp3.RequestBody.writeTo(okio.BufferedSink):java.io.IOException"
       - "okhttp3.Response.close():java.lang.IllegalStateException"
@@ -265,6 +269,7 @@ datadog:
       - "okio.BufferedSink.close():java.io.IOException"
       - "okio.Okio.buffer(okio.Sink):java.lang.NullPointerException"
       - "okio.Buffer.readString(java.nio.charset.Charset):java.lang.IllegalArgumentException,java.io.IOException"
+      - "okio.Buffer.readByteArray():java.io.EOFException"
       # endregion
       # region org.json
       - "org.json.JSONArray.get(kotlin.Int):org.json.JSONException"
@@ -1095,6 +1100,7 @@ datadog:
       - "kotlin.String.toHttpUrlOrNull()"
       - "kotlin.String.toIntOrNull()"
       - "kotlin.String.toLongOrNull()"
+      - "kotlin.String.toMediaTypeOrNull()"
       - "kotlin.String.toMethod()"
       - "kotlin.String.toOperationType(com.datadog.android.api.InternalLogger)"
       - "kotlin.String.uppercase(java.util.Locale)"
@@ -1156,6 +1162,8 @@ datadog:
       - "okhttp3.HttpUrl.parse(kotlin.String)"
       - "okhttp3.HttpUrl.url()"
       - "okhttp3.Interceptor.Chain.request()"
+      - "okhttp3.MultipartBody.Builder.constructor(kotlin.String)"
+      - "okhttp3.MultipartBody.Builder.addFormDataPart(kotlin.String, kotlin.String?, okhttp3.RequestBody)"
       - "okhttp3.MultipartBody.Part.body()"
       - "okhttp3.MultipartBody.Part.headers()"
       - "okhttp3.MultipartBody.parts()"

diff --git a/...k-android-logs/src/main/kotlin/com/datadog/android/log/internal/net/LogsRequestFactory.kt b/...k-android-logs/src/main/kotlin/com/datadog/android/log/internal/net/LogsRequestFactory.kt
@@ -30,7 +30,7 @@ internal class LogsRequestFactory(
         context: DatadogContext,
         batchData: List<RawBatchEvent>,
         batchMetadata: ByteArray?
-    ): Request {
+    ): Request? {
         val requestId = UUID.randomUUID().toString()
 
         return Request(

diff --git a/...droid-logs/src/test/kotlin/com/datadog/android/log/internal/net/LogsRequestFactoryTest.kt b/...droid-logs/src/test/kotlin/com/datadog/android/log/internal/net/LogsRequestFactoryTest.kt
@@ -61,22 +61,22 @@ internal class LogsRequestFactoryTest {
         val request = testedFactory.create(fakeDatadogContext, batchData, batchMetadata)
 
         // Then
-        assertThat(request.url).isEqualTo(
+        assertThat(request?.url).isEqualTo(
             "${fakeDatadogContext.site.intakeEndpoint}/api/v2/logs?" +
                 "ddsource=${fakeDatadogContext.source}"
         )
-        assertThat(request.contentType).isEqualTo(RequestFactory.CONTENT_TYPE_JSON)
-        assertThat(request.headers.minus(RequestFactory.HEADER_REQUEST_ID)).isEqualTo(
+        assertThat(request?.contentType).isEqualTo(RequestFactory.CONTENT_TYPE_JSON)
+        assertThat(request?.headers?.minus(RequestFactory.HEADER_REQUEST_ID)).isEqualTo(
             mapOf(
                 RequestFactory.HEADER_API_KEY to fakeDatadogContext.clientToken,
                 RequestFactory.HEADER_EVP_ORIGIN to fakeDatadogContext.source,
                 RequestFactory.HEADER_EVP_ORIGIN_VERSION to fakeDatadogContext.sdkVersion
             )
         )
-        assertThat(request.headers[RequestFactory.HEADER_REQUEST_ID]).isNotEmpty()
-        assertThat(request.id).isEqualTo(request.headers[RequestFactory.HEADER_REQUEST_ID])
-        assertThat(request.description).isEqualTo("Logs Request")
-        assertThat(request.body).isEqualTo(
+        assertThat(request?.headers?.get(RequestFactory.HEADER_REQUEST_ID)).isNotEmpty()
+        assertThat(request?.id).isEqualTo(request?.headers?.get(RequestFactory.HEADER_REQUEST_ID))
+        assertThat(request?.description).isEqualTo("Logs Request")
+        assertThat(request?.body).isEqualTo(
             batchData.map { it.data }
                 .join(
                     separator = ",".toByteArray(),
@@ -106,22 +106,22 @@ internal class LogsRequestFactoryTest {
         val request = testedFactory.create(fakeDatadogContext, batchData, batchMetadata)
 
         // Then
-        assertThat(request.url).isEqualTo(
+        assertThat(request?.url).isEqualTo(
             "$fakeEndpoint/api/v2/logs?" +
                 "ddsource=${fakeDatadogContext.source}"
         )
-        assertThat(request.contentType).isEqualTo(RequestFactory.CONTENT_TYPE_JSON)
-        assertThat(request.headers.minus(RequestFactory.HEADER_REQUEST_ID)).isEqualTo(
+        assertThat(request?.contentType).isEqualTo(RequestFactory.CONTENT_TYPE_JSON)
+        assertThat(request?.headers?.minus(RequestFactory.HEADER_REQUEST_ID)).isEqualTo(
             mapOf(
                 RequestFactory.HEADER_API_KEY to fakeDatadogContext.clientToken,
                 RequestFactory.HEADER_EVP_ORIGIN to fakeDatadogContext.source,
                 RequestFactory.HEADER_EVP_ORIGIN_VERSION to fakeDatadogContext.sdkVersion
             )
         )
-        assertThat(request.headers[RequestFactory.HEADER_REQUEST_ID]).isNotEmpty()
-        assertThat(request.id).isEqualTo(request.headers[RequestFactory.HEADER_REQUEST_ID])
-        assertThat(request.description).isEqualTo("Logs Request")
-        assertThat(request.body).isEqualTo(
+        assertThat(request?.headers?.get(RequestFactory.HEADER_REQUEST_ID)).isNotEmpty()
+        assertThat(request?.id).isEqualTo(request?.headers?.get(RequestFactory.HEADER_REQUEST_ID))
+        assertThat(request?.description).isEqualTo("Logs Request")
+        assertThat(request?.body).isEqualTo(
             batchData.map { it.data }
                 .join(
                     separator = ",".toByteArray(),

diff --git a/...sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/net/RumRequestFactory.kt b/...sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/net/RumRequestFactory.kt
@@ -27,7 +27,7 @@ internal class RumRequestFactory(
         context: DatadogContext,
         batchData: List<RawBatchEvent>,
         batchMetadata: ByteArray?
-    ): Request {
+    ): Request? {
         val requestId = UUID.randomUUID().toString()
 
         return Request(

diff --git a/...android-rum/src/test/kotlin/com/datadog/android/rum/internal/net/RumRequestFactoryTest.kt b/...android-rum/src/test/kotlin/com/datadog/android/rum/internal/net/RumRequestFactoryTest.kt
@@ -75,19 +75,19 @@ internal class RumRequestFactoryTest {
         val request = testedFactory.create(fakeDatadogContext, batchData, batchMetadata)
 
         // Then
-        assertThat(request.url).isEqualTo(expectedUrl(fakeDatadogContext.site.intakeEndpoint))
-        assertThat(request.contentType).isEqualTo(RequestFactory.CONTENT_TYPE_TEXT_UTF8)
-        assertThat(request.headers.minus(RequestFactory.HEADER_REQUEST_ID)).isEqualTo(
+        assertThat(request?.url).isEqualTo(expectedUrl(fakeDatadogContext.site.intakeEndpoint))
+        assertThat(request?.contentType).isEqualTo(RequestFactory.CONTENT_TYPE_TEXT_UTF8)
+        assertThat(request?.headers?.minus(RequestFactory.HEADER_REQUEST_ID)).isEqualTo(
             mapOf(
                 RequestFactory.HEADER_API_KEY to fakeDatadogContext.clientToken,
                 RequestFactory.HEADER_EVP_ORIGIN to fakeDatadogContext.source,
                 RequestFactory.HEADER_EVP_ORIGIN_VERSION to fakeDatadogContext.sdkVersion
             )
         )
-        assertThat(request.headers[RequestFactory.HEADER_REQUEST_ID]).isNotEmpty()
-        assertThat(request.id).isEqualTo(request.headers[RequestFactory.HEADER_REQUEST_ID])
-        assertThat(request.description).isEqualTo("RUM Request")
-        assertThat(request.body).isEqualTo(
+        assertThat(request?.headers?.get(RequestFactory.HEADER_REQUEST_ID)).isNotEmpty()
+        assertThat(request?.id).isEqualTo(request?.headers?.get(RequestFactory.HEADER_REQUEST_ID))
+        assertThat(request?.description).isEqualTo("RUM Request")
+        assertThat(request?.body).isEqualTo(
             batchData.map { it.data }.join(
                 separator = "\n".toByteArray(),
                 internalLogger = InternalLogger.UNBOUND
@@ -115,19 +115,19 @@ internal class RumRequestFactoryTest {
         val request = testedFactory.create(fakeDatadogContext, batchData, batchMetadata)
 
         // Then
-        assertThat(request.url).isEqualTo(expectedUrl(fakeEndpoint))
-        assertThat(request.contentType).isEqualTo(RequestFactory.CONTENT_TYPE_TEXT_UTF8)
-        assertThat(request.headers.minus(RequestFactory.HEADER_REQUEST_ID)).isEqualTo(
+        assertThat(request?.url).isEqualTo(expectedUrl(fakeEndpoint))
+        assertThat(request?.contentType).isEqualTo(RequestFactory.CONTENT_TYPE_TEXT_UTF8)
+        assertThat(request?.headers?.minus(RequestFactory.HEADER_REQUEST_ID)).isEqualTo(
             mapOf(
                 RequestFactory.HEADER_API_KEY to fakeDatadogContext.clientToken,
                 RequestFactory.HEADER_EVP_ORIGIN to fakeDatadogContext.source,
                 RequestFactory.HEADER_EVP_ORIGIN_VERSION to fakeDatadogContext.sdkVersion
             )
         )
-        assertThat(request.headers[RequestFactory.HEADER_REQUEST_ID]).isNotEmpty()
-        assertThat(request.id).isEqualTo(request.headers[RequestFactory.HEADER_REQUEST_ID])
-        assertThat(request.description).isEqualTo("RUM Request")
-        assertThat(request.body).isEqualTo(
+        assertThat(request?.headers?.get(RequestFactory.HEADER_REQUEST_ID)).isNotEmpty()
+        assertThat(request?.id).isEqualTo(request?.headers?.get(RequestFactory.HEADER_REQUEST_ID))
+        assertThat(request?.description).isEqualTo("RUM Request")
+        assertThat(request?.body).isEqualTo(
             batchData.map { it.data }.join(
                 separator = "\n".toByteArray(),
                 internalLogger = InternalLogger.UNBOUND