RUM-11821: Fix crash in KronosTimeProvider

Author: aleksandr-gringauz

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt

@@ -154,7 +154,10 @@ internal class CoreFeature(
         DefaultFirstPartyHostHeaderTypeResolver(emptyMap())
     internal var networkInfoProvider: NetworkInfoProvider = NoOpNetworkInfoProvider()
     internal var systemInfoProvider: SystemInfoProvider = NoOpSystemInfoProvider()
+
+    @Volatile
     internal var timeProvider: TimeProvider = DefaultTimeProvider()
+
     internal var trackingConsentProvider: ConsentProvider = NoOpConsentProvider()
     internal var userInfoProvider: MutableUserInfoProvider = NoOpMutableUserInfoProvider()
     internal var accountInfoProvider: MutableAccountInfoProvider = NoOpMutableAccountInfoProvider()
@@ -487,7 +490,7 @@ internal class CoreFeature(
                 }
             }
 
-            timeProvider = KronosTimeProvider(this)
+            timeProvider = KronosTimeProvider(this, internalLogger)
         }
     }
 

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/time/KronosTimeProvider.kt

@@ -6,30 +6,57 @@
 
 package com.datadog.android.core.internal.time
 
+import com.datadog.android.api.InternalLogger
 import com.datadog.android.internal.time.TimeProvider
 import com.lyft.kronos.Clock
 import java.util.concurrent.TimeUnit
 
 internal class KronosTimeProvider(
-    private val clock: Clock
+    private val clock: Clock,
+    private val internalLogger: InternalLogger
 ) : TimeProvider {
 
     override fun getDeviceTimestamp(): Long {
         return System.currentTimeMillis()
     }
 
+    @Suppress("TooGenericExceptionCaught")
     override fun getServerTimestamp(): Long {
-        return clock.getCurrentTimeMs()
+        return clock.safeGetCurrentTimeMs()
+            .getOrElse { System.currentTimeMillis() }
     }
 
+    @Suppress("TooGenericExceptionCaught")
     override fun getServerOffsetMillis(): Long {
-        val server = clock.getCurrentTimeMs()
-        val device = System.currentTimeMillis()
-        val delta = server - device
-        return delta
+        return clock.safeGetCurrentTimeMs()
+            .map { server ->
+                val device = System.currentTimeMillis()
+                val delta = server - device
+                delta
+            }
+            .getOrElse { 0 }
     }
 
     override fun getServerOffsetNanos(): Long {
         return TimeUnit.MILLISECONDS.toNanos(getServerOffsetMillis())
     }
+
+    private fun Clock.safeGetCurrentTimeMs(): Result<Long> {
+        return runCatching {
+            getCurrentTimeMs()
+        }.onFailure { ex ->
+            internalLogger.log(
+                level = InternalLogger.Level.ERROR,
+                targets = listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY),
+                messageBuilder = { FAIL_MESSAGE },
+                throwable = ex,
+                onlyOnce = true,
+                additionalProperties = emptyMap()
+            )
+        }
+    }
+
+    companion object {
+        const val FAIL_MESSAGE = "KronosClock.getCurrentTimeMs failed with an exception"
+    }
 }

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/time/KronosTimeProviderTest.kt

@@ -6,8 +6,13 @@
 
 package com.datadog.android.core.internal.time
 
+import com.datadog.android.api.InternalLogger
+import com.datadog.android.core.internal.time.KronosTimeProvider.Companion.FAIL_MESSAGE
 import com.datadog.android.utils.forge.Configurator
+import com.datadog.android.utils.verifyLog
+import com.datadog.tools.unit.forge.anException
 import com.lyft.kronos.Clock
+import fr.xgouchet.elmyr.Forge
 import fr.xgouchet.elmyr.annotation.Forgery
 import fr.xgouchet.elmyr.junit5.ForgeConfiguration
 import fr.xgouchet.elmyr.junit5.ForgeExtension
@@ -21,6 +26,7 @@ import org.mockito.Mock
 import org.mockito.junit.jupiter.MockitoExtension
 import org.mockito.junit.jupiter.MockitoSettings
 import org.mockito.kotlin.doReturn
+import org.mockito.kotlin.doThrow
 import org.mockito.kotlin.whenever
 import org.mockito.quality.Strictness
 import java.util.Date
@@ -42,10 +48,13 @@ internal class KronosTimeProviderTest {
     @Forgery
     lateinit var fakeDate: Date
 
+    @Mock
+    lateinit var internalLogger: InternalLogger
+
     @BeforeEach
     fun `set up`() {
         whenever(mockClock.getCurrentTimeMs()) doReturn fakeDate.time
-        testedTimeProvider = KronosTimeProvider(mockClock)
+        testedTimeProvider = KronosTimeProvider(mockClock, internalLogger)
     }
 
     @Test
@@ -87,6 +96,49 @@ internal class KronosTimeProviderTest {
         assertThat(result).isCloseTo(now, Offset.offset(TEST_OFFSET))
     }
 
+    @Test
+    fun `M log and return 0 W getServerOffsetMillis { getCurrentTimeMs throws }`(forge: Forge) {
+        // Given
+        val exception = forge.anException()
+        whenever(mockClock.getCurrentTimeMs()) doThrow exception
+
+        // When
+        val result = testedTimeProvider.getServerOffsetMillis()
+
+        // Then
+        internalLogger.verifyLog(
+            level = InternalLogger.Level.ERROR,
+            targets = listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY),
+            message = FAIL_MESSAGE,
+            throwable = exception,
+            onlyOnce = true,
+            additionalProperties = emptyMap()
+        )
+        assertThat(result).isZero()
+    }
+
+    @Test
+    fun `M log and return System currentTimeMillis W getServerTimestamp { getCurrentTimeMs throws }`(forge: Forge) {
+        // Given
+        val exception = forge.anException()
+        whenever(mockClock.getCurrentTimeMs()) doThrow exception
+
+        // When
+        val now = System.currentTimeMillis()
+        val result = testedTimeProvider.getServerTimestamp()
+
+        // Then
+        internalLogger.verifyLog(
+            level = InternalLogger.Level.ERROR,
+            targets = listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY),
+            message = FAIL_MESSAGE,
+            throwable = exception,
+            onlyOnce = true,
+            additionalProperties = emptyMap()
+        )
+        assertThat(result).isCloseTo(now, Offset.offset(TEST_OFFSET))
+    }
+
     companion object {
         const val TEST_OFFSET = 10L
     }

detekt_custom_safe_calls.yml

@@ -577,6 +577,9 @@ datadog:
       - "kotlin.ReplaceWith.constructor(kotlin.String, kotlin.Array)"
       - "kotlin.Result.failure(kotlin.Throwable)"
       - "kotlin.Result.fold(kotlin.Function1, kotlin.Function1)"
+      - "kotlin.Result.getOrElse(kotlin.Function1)"
+      - "kotlin.Result.map(kotlin.Function1)"
+      - "kotlin.Result.onFailure(kotlin.Function1)"
       # endregion
       # region Kotlin Collections
       - "kotlin.Array.all(kotlin.Function1)"
@@ -1187,5 +1190,6 @@ datadog:
       - "com.squareup.sqldelight.android.AndroidSqliteDriver.Callback.onCorruption(androidx.sqlite.db.SupportSQLiteDatabase)"
       # endregion
       # region Kronos
+      - "com.lyft.kronos.Clock.runCatching(kotlin.Function1)"
       - "com.lyft.kronos.KronosClock.shutdown()"
       # endregion