Fix Ui tests receiving empty requests (#3094)

* cleaned up MockRelay, removing requests that are not envelopes (we don't have such)
* added a check to discard already received envelopes, sent twice for any reason)

Author: stefanosiano

sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt

@@ -42,7 +42,6 @@ class EnvelopeTests : BaseUiTest() {
                 assertTrue(event.message?.formatted == "Message captured during test")
             }
             assertNoOtherEnvelopes()
-            assertNoOtherRequests()
         }
     }
 
@@ -170,7 +169,6 @@ class EnvelopeTests : BaseUiTest() {
                 assertNotNull(transactionData)
             }
             assertNoOtherEnvelopes()
-            assertNoOtherRequests()
         }
     }
 
@@ -205,7 +203,6 @@ class EnvelopeTests : BaseUiTest() {
                 assertEquals(ProfilingTraceData.TRUNCATION_REASON_TIMEOUT, profilingTraceData.truncationReason)
             }
             assertNoOtherEnvelopes()
-            assertNoOtherRequests()
         }
     }
 

sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt

@@ -72,7 +72,6 @@ class SdkInitTests : BaseUiTest() {
                 assertEquals("e2etests2", profilingTraceData.transactionName)
             }
             assertNoOtherEnvelopes()
-            assertNoOtherRequests()
         }
     }
 }

sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/MockRelay.kt

@@ -6,6 +6,7 @@ import okhttp3.mockwebserver.Dispatcher
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
 import okhttp3.mockwebserver.RecordedRequest
+import kotlin.test.assertNotNull
 
 /** Mocks a relay server. */
 class MockRelay(
@@ -22,31 +23,40 @@ class MockRelay(
     /** List of unasserted requests sent to the [envelopePath]. */
     private val unassertedEnvelopes = mutableListOf<RelayAsserter.RelayResponse>()
 
-    /** List of unasserted requests not contained in [unassertedEnvelopes]. */
-    private val unassertedRequests = mutableListOf<RelayAsserter.RelayResponse>()
-
     /** List of responses to return when a request is sent. */
     private val responses = mutableListOf<(RecordedRequest) -> MockResponse?>()
 
+    /** Set to check already received envelopes, to avoid duplicates due to e.g. retrying. */
+    private val receivedEnvelopes: MutableSet<String> = HashSet()
+
     init {
         relay.dispatcher = object : Dispatcher() {
             override fun dispatch(request: RecordedRequest): MockResponse {
+                // If a request with a body size of 0 is received, we drop it.
+                // This shouldn't happen in reality, but it rarely happens in tests.
+                if (request.bodySize == 0L || request.failure != null) {
+                    return MockResponse()
+                }
                 // We check if there is any custom response previously set to return to this request,
                 // otherwise we return a successful MockResponse.
                 val response = responses.asSequence()
                     .mapNotNull { it(request) }
                     .firstOrNull()
                     ?: MockResponse()
 
-                // Based on the path of the request, we populate the right list.
-                val relayResponse = RelayAsserter.RelayResponse(request, response)
-                when (request.path) {
-                    envelopePath -> {
-                        unassertedEnvelopes.add(relayResponse)
-                    }
-                    else -> {
-                        unassertedRequests.add(relayResponse)
+                // We should receive only envelopes on this path.
+                if (request.path == envelopePath) {
+                    val relayResponse = RelayAsserter.RelayResponse(request, response)
+                    assertNotNull(relayResponse.envelope)
+                    val envelopeId: String = relayResponse.envelope!!.header.eventId!!.toString()
+                    // If we already received the envelope (e.g. retrying mechanism) we drop it
+                    if (receivedEnvelopes.contains(envelopeId)) {
+                        return MockResponse()
                     }
+                    receivedEnvelopes.add(envelopeId)
+                    unassertedEnvelopes.add(relayResponse)
+                } else {
+                    throw AssertionError("Expected $envelopePath, but the request path was ${request.path}")
                 }
 
                 // If we are waiting for requests to be received, we decrement the associated counter.
@@ -62,7 +72,10 @@ class MockRelay(
     fun createMockDsn() = "http://key@${relay.hostName}:${relay.port}/$dsnProject"
 
     /** Starts the mock relay server. */
-    fun start() = relay.start()
+    fun start() {
+        receivedEnvelopes.clear()
+        relay.start()
+    }
 
     /** Shutdown the mock relay server and clear everything. */
     fun shutdown() {
@@ -98,6 +111,6 @@ class MockRelay(
         if (waitForRequests) {
             waitUntilIdle()
         }
-        assertion(RelayAsserter(unassertedEnvelopes, unassertedRequests))
+        assertion(RelayAsserter(unassertedEnvelopes))
     }
 }

sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/RelayAsserter.kt

@@ -1,8 +1,14 @@
 package io.sentry.uitest.android.mockservers
 
 import io.sentry.EnvelopeReader
+import io.sentry.JsonSerializer
+import io.sentry.ProfilingTraceData
 import io.sentry.Sentry
 import io.sentry.SentryEnvelope
+import io.sentry.SentryEvent
+import io.sentry.SentryItemType
+import io.sentry.SentryOptions
+import io.sentry.protocol.SentryTransaction
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.RecordedRequest
 import okio.GzipSource
@@ -12,40 +18,18 @@ import java.util.zip.GZIPInputStream
 
 /** Class used to assert requests sent to [MockRelay]. */
 class RelayAsserter(
-    private val unassertedEnvelopes: MutableList<RelayResponse>,
-    private val unassertedRequests: MutableList<RelayResponse>
+    private val unassertedEnvelopes: MutableList<RelayResponse>
 ) {
-
-    /**
-     * Asserts an envelope request exists and allows to make other assertions on the first one and on its response.
-     * The asserted envelope request is then removed from internal list of unasserted envelopes.
-     */
-    fun assertFirstRawEnvelope(assertion: ((relayResponse: RelayResponse) -> Unit)? = null) {
-        val relayResponse = unassertedEnvelopes.removeFirstOrNull()
-            ?: throw AssertionError("No envelope request found")
-        assertion?.let { it(relayResponse) }
-    }
-
-    /**
-     * Asserts a request exists and makes other assertions on the first one and on its response.
-     * The asserted request is then removed from internal list of unasserted requests.
-     */
-    fun assertRawRequest(assertion: ((request: RecordedRequest, response: MockResponse) -> Unit)? = null) {
-        val relayResponse = unassertedRequests.removeFirstOrNull()
-            ?: throw AssertionError("No raw request found")
-        assertion?.let {
-            it(relayResponse.request, relayResponse.response)
-        }
-    }
+    private val originalUnassertedEnvelopes: MutableList<RelayResponse> = ArrayList(unassertedEnvelopes)
 
     /**
      * Parses the first request as an envelope and makes other assertions through a [EnvelopeAsserter].
      * The asserted envelope is then removed from internal list of unasserted envelopes.
      */
     fun assertFirstEnvelope(assertion: (asserter: EnvelopeAsserter) -> Unit) {
-        assertFirstRawEnvelope { relayResponse ->
-            relayResponse.assert(assertion)
-        }
+        val relayResponse = unassertedEnvelopes.removeFirstOrNull()
+            ?: throw AssertionError("No envelope request found")
+        relayResponse.assert(assertion)
     }
 
     /**
@@ -63,22 +47,39 @@ class RelayAsserter(
     /** Asserts no other envelopes were sent. */
     fun assertNoOtherEnvelopes() {
         if (unassertedEnvelopes.isNotEmpty()) {
-            throw AssertionError("There were other ${unassertedEnvelopes.size} envelope requests: $unassertedEnvelopes")
+            throw AssertionError(
+                "There was a total of ${originalUnassertedEnvelopes.size} envelopes: " +
+                    originalUnassertedEnvelopes.joinToString { describeEnvelope(it.envelope!!) }
+            )
         }
     }
 
-    /** Asserts no other raw requests were sent. */
-    fun assertNoOtherRawRequests() {
-        assertNoOtherEnvelopes()
-        if (unassertedRequests.isNotEmpty()) {
-            throw AssertionError("There were other ${unassertedRequests.size} requests: $unassertedRequests")
+    /** Function used to describe the content of the envelope to print in the logs. For debugging purposes only. */
+    private fun describeEnvelope(envelope: SentryEnvelope): String {
+        var descr = ""
+        envelope.items.forEach { item ->
+            when (item.header.type) {
+                SentryItemType.Event -> {
+                    val deserialized = JsonSerializer(SentryOptions())
+                        .deserialize(item.data.inputStream().reader(), SentryEvent::class.java)!!
+                    descr += "Event (${deserialized.eventId}) - message: ${deserialized.message!!.formatted} -- "
+                }
+                SentryItemType.Transaction -> {
+                    val deserialized = JsonSerializer(SentryOptions())
+                        .deserialize(item.data.inputStream().reader(), SentryTransaction::class.java)!!
+                    descr += "Transaction (${deserialized.eventId}) - transaction: ${deserialized.transaction} - spans: ${deserialized.spans.joinToString { "${it.op} ${it.description}" }} -- "
+                }
+                SentryItemType.Profile -> {
+                    val deserialized = JsonSerializer(SentryOptions())
+                        .deserialize(item.data.inputStream().reader(), ProfilingTraceData::class.java)!!
+                    descr += "Profile (${deserialized.profileId}) - transactionName: ${deserialized.transactionName} -- "
+                }
+                else -> {
+                    descr += "${item.header.type} -- "
+                }
+            }
         }
-    }
-
-    /** Asserts no other requests or envelopes were sent. */
-    fun assertNoOtherRequests() {
-        assertNoOtherEnvelopes()
-        assertNoOtherRawRequests()
+        return "*** Envelope: $descr ***"
     }
 
     data class RelayResponse(val request: RecordedRequest, val response: MockResponse) {