Merge pull request #5 from kkostov/bugfix/catch-ktor-exceptions

Bugfix/catch ktor exceptions
TelemetryDeck · Mar 21, 2022 · 4b501da · 4b501da
2 parents a9bf6fe + c80c966
commit 4b501da
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 26 deletions.
diff --git a/lib/build.gradle b/lib/build.gradle
@@ -96,7 +96,7 @@ afterEvaluate {
                 // You can then customize attributes of the publication as shown below.
                 groupId = 'com.telemetrydeck.sdk'
                 artifactId = 'final'
-                version = '1.0'
+                version = '1.1'
             }
         }
     }

diff --git a/lib/src/main/java/com/telemetrydeck/sdk/Signal.kt b/lib/src/main/java/com/telemetrydeck/sdk/Signal.kt
@@ -9,7 +9,7 @@ data class Signal(
      * When was this signal generated
      */
     @Serializable(with = DateSerializer::class)
-    var receivedAt: Date? = null,
+    var receivedAt: Date = Date(),
 
     /**
      * The App ID of this signal

diff --git a/lib/src/main/java/com/telemetrydeck/sdk/TelemetryBroadcastTimer.kt b/lib/src/main/java/com/telemetrydeck/sdk/TelemetryBroadcastTimer.kt
@@ -3,6 +3,8 @@ package com.telemetrydeck.sdk
 import kotlinx.coroutines.*
 import kotlinx.coroutines.channels.ticker
 import java.lang.ref.WeakReference
+import java.util.*
+import kotlin.math.abs
 
 internal class TelemetryBroadcastTimer(private val manager: WeakReference<TelemetryManager>, debugLogger: WeakReference<DebugLogger>) {
 
@@ -15,6 +17,16 @@ internal class TelemetryBroadcastTimer(private val manager: WeakReference<Teleme
     }
     private var job: Job? = null
 
+    companion object {
+        fun filterOldSignals(signals: List<Signal>): List<Signal> {
+            val now = Date().time
+            return signals.filter {
+                // ignore signals older than 24h
+                (abs(now - it.receivedAt.time) / 1000) <= 24 * 60 * 60
+            }
+        }
+    }
+
     fun start() {
         CoroutineScope(Dispatchers.IO).launch {
             job?.cancel()
@@ -24,15 +36,21 @@ internal class TelemetryBroadcastTimer(private val manager: WeakReference<Teleme
                     val managerInstance = manager.get()
                         ?: // can't broadcast without a manager to provide signals
                         continue
-                    val signals =  managerInstance.cache?.empty() ?: emptyList()
+
+                    val signals =  filterOldSignals(managerInstance.cache?.empty() ?: emptyList())
                     if (signals.isEmpty()) {
                         // no signals to broadcast
                         logger?.debug("No signals to broadcast")
                         continue
                     }
 
                     logger?.debug("Broadcasting ${signals.count()} queued signals")
-                    managerInstance.send(signals)
+                    if (managerInstance.send(signals).isFailure) {
+                        logger?.debug("Re-enqueueing ${signals.count()} signals")
+                        for (failedSignal in signals) {
+                            managerInstance.cache?.add(failedSignal)
+                        }
+                    }
                 }
             }
         }

diff --git a/lib/src/main/java/com/telemetrydeck/sdk/TelemetryManager.kt b/lib/src/main/java/com/telemetrydeck/sdk/TelemetryManager.kt
@@ -7,6 +7,8 @@ import java.lang.ref.WeakReference
 import java.net.URL
 import java.security.MessageDigest
 import java.util.*
+import kotlin.Result.Companion.failure
+import kotlin.Result.Companion.success
 
 
 class TelemetryManager(
@@ -47,34 +49,40 @@ class TelemetryManager(
         signalType: String,
         clientUser: String?,
         additionalPayload: Map<String, String>
-    ) {
-        send(createSignal(signalType, clientUser, additionalPayload))
+    ): Result<Unit> {
+        return send(createSignal(signalType, clientUser, additionalPayload))
     }
 
     override suspend fun send(
         signalType: SignalType,
         clientUser: String?,
         additionalPayload: Map<String, String>
-    ) {
-        send(signalType.type, clientUser, additionalPayload)
+    ): Result<Unit> {
+        return send(signalType.type, clientUser, additionalPayload)
     }
 
     suspend fun send(
         signal: Signal
-    ) {
-        send(listOf(signal))
+    ): Result<Unit> {
+        return send(listOf(signal))
     }
 
     suspend fun send(
         signals: List<Signal>
-    ) {
-        val client = TelemetryClient(
-            configuration.telemetryAppID,
-            configuration.apiBaseURL,
-            configuration.showDebugLogs,
-            logger
-        )
-        client.send(signals)
+    ): Result<Unit> {
+        return try {
+            val client = TelemetryClient(
+                configuration.telemetryAppID,
+                configuration.apiBaseURL,
+                configuration.showDebugLogs,
+                logger
+            )
+            client.send(signals)
+            success(Unit)
+        } catch(e: Exception) {
+            logger?.error("Failed to send signals due to an error ${e} ${e.stackTraceToString()}")
+            failure(e)
+        }
     }
 
     internal var broadcastTimer: TelemetryBroadcastTimer? = null
@@ -202,16 +210,24 @@ class TelemetryManager(
             signalType: String,
             clientUser: String?,
             additionalPayload: Map<String, String>
-        ) {
-            getInstance()?.send(signalType, clientUser, additionalPayload)
+        ): Result<Unit> {
+            val result = getInstance()?.send(signalType, clientUser, additionalPayload)
+            if (result != null) {
+                return result
+            }
+            return failure(NullPointerException())
         }
 
         override suspend fun send(
             signalType: SignalType,
             clientUser: String?,
             additionalPayload: Map<String, String>
-        ) {
-            getInstance()?.send(signalType, clientUser, additionalPayload)
+        ): Result<Unit> {
+            val result = getInstance()?.send(signalType, clientUser, additionalPayload)
+            if (result != null) {
+                return result
+            }
+            return failure(NullPointerException())
         }
     }
 

diff --git a/lib/src/main/java/com/telemetrydeck/sdk/TelemetryManagerSignals.kt b/lib/src/main/java/com/telemetrydeck/sdk/TelemetryManagerSignals.kt
@@ -46,7 +46,7 @@ interface TelemetryManagerSignals {
         signalType: String,
         clientUser: String? = null,
         additionalPayload: Map<String, String> = emptyMap()
-    )
+    ): Result<Unit>
 
 
     /**
@@ -56,5 +56,5 @@ interface TelemetryManagerSignals {
         signalType: SignalType,
         clientUser: String? = null,
         additionalPayload: Map<String, String> = emptyMap()
-    )
+    ): Result<Unit>
 }
diff --git a/lib/src/test/java/com/telemetrydeck/sdk/TelemetryManagerTest.kt b/lib/src/test/java/com/telemetrydeck/sdk/TelemetryManagerTest.kt
@@ -5,8 +5,6 @@ import androidx.arch.core.executor.testing.InstantTaskExecutorRule
 import org.junit.Assert
 import org.junit.Rule
 import org.junit.Test
-import org.mockito.Mockito.*
-import org.mockito.kotlin.verify
 import java.net.URL
 import java.util.*
 
@@ -221,6 +219,21 @@ class TelemetryManagerTest {
 
         Assert.assertTrue(provider.registered)
     }
+
+    @Test
+    fun telemetryBroadcastTimer_can_filter_older_signals() {
+        // an old signal is received longer than 24h ago
+        val okSignal = Signal(appID = UUID.randomUUID(), "okSignal", "user", SignalPayload())
+        val oldSignal = Signal(appID = UUID.randomUUID(), "oldSignal", "user", SignalPayload())
+        val calendar = Calendar.getInstance()
+        calendar.add(Calendar.DAY_OF_YEAR, -2)
+        oldSignal.receivedAt = calendar.time
+
+        val filteredSignals = TelemetryBroadcastTimer.filterOldSignals(listOf(okSignal, oldSignal))
+
+        Assert.assertEquals(1, filteredSignals.count())
+        Assert.assertEquals("okSignal", filteredSignals[0].type)
+    }
 }
 
 open class TestProvider: TelemetryProvider {