Skip to content

Commit

Permalink
refactor: remove unnecessary sendEventAndWait function
Browse files Browse the repository at this point in the history
  • Loading branch information
fractalwrench committed Dec 13, 2023
1 parent 7a0bf7a commit b4ffbf8
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ internal class EmbraceCrashService(
// and the request is made on a background executor, but data analysis shows that
// a surprising % of crashes make it through based on the receive time. Therefore we
// attempt to send the crash and if it fails, we will send it again on the next launch.
deliveryService.sendCrash(crashEvent)
deliveryService.sendCrash(crashEvent, true)

// End, cache and send the session
sessionService.handleCrash(crash.crashId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ internal interface ApiService {
fun sendLog(eventMessage: EventMessage)
fun sendNetworkCall(networkEvent: NetworkEvent)
fun sendEvent(eventMessage: EventMessage)
fun sendEventAndWait(eventMessage: EventMessage)
fun sendCrash(crash: EventMessage)
fun sendCrash(crash: EventMessage): Future<*>
fun sendAEIBlob(blobMessage: BlobMessage)
fun sendSession(sessionPayload: ByteArray, onFinish: (() -> Unit)?): Future<*>?
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import io.embrace.android.embracesdk.payload.NetworkEvent
import java.io.ByteArrayInputStream
import java.util.concurrent.Future
import java.util.concurrent.ScheduledExecutorService
import java.util.concurrent.TimeUnit

internal class EmbraceApiService(
private val apiClient: ApiClient,
Expand Down Expand Up @@ -145,29 +144,13 @@ internal class EmbraceApiService(
post(eventMessage, mapper::eventMessageRequest)
}

/**
* Sends an event to the API and waits for the request to be completed
*
* @param eventMessage the event message containing the event
*/
override fun sendEventAndWait(eventMessage: EventMessage) {
post(eventMessage, mapper::eventMessageRequest).get()
}

/**
* Sends a crash event to the API and reschedules it if the request times out
*
* @param crash the event message containing the crash
*/
override fun sendCrash(crash: EventMessage) {
try {
post(crash, mapper::eventMessageRequest) { cacheManager.deleteCrash() }.get(
CRASH_TIMEOUT,
TimeUnit.SECONDS
)
} catch (e: Exception) {
logger.logError("The crash report request has timed out.")
}
override fun sendCrash(crash: EventMessage): Future<*> {
return post(crash, mapper::eventMessageRequest) { cacheManager.deleteCrash() }
}

override fun sendSession(sessionPayload: ByteArray, onFinish: (() -> Unit)?): Future<*> {
Expand Down Expand Up @@ -230,4 +213,3 @@ internal class EmbraceApiService(
}

private const val TAG = "EmbraceApiService"
private const val CRASH_TIMEOUT = 1L // Seconds to wait before timing out when sending a crash
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ internal interface DeliveryService {
fun sendBackgroundActivities()
fun sendLog(eventMessage: EventMessage)
fun sendNetworkCall(networkEvent: NetworkEvent)
fun sendCrash(crash: EventMessage)
fun sendCrash(crash: EventMessage, processTerminating: Boolean)
fun sendAEIBlob(blobMessage: BlobMessage)
fun sendMoment(eventMessage: EventMessage)
fun sendEventAndWait(eventMessage: EventMessage)
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ internal class EmbraceDeliveryService(

companion object {
private const val TAG = "EmbraceDeliveryService"

private const val SEND_SESSION_TIMEOUT = 1L
private const val CRASH_TIMEOUT = 1L // Seconds to wait before timing out when sending a crash
}

private val backgroundActivities by lazy { mutableSetOf<String>() }
Expand Down Expand Up @@ -135,9 +135,15 @@ internal class EmbraceDeliveryService(
apiService.sendNetworkCall(networkEvent)
}

override fun sendCrash(crash: EventMessage) {
cacheManager.saveCrash(crash)
apiService.sendCrash(crash)
override fun sendCrash(crash: EventMessage, processTerminating: Boolean) {
runCatching {
cacheManager.saveCrash(crash)
val future = apiService.sendCrash(crash)

if (processTerminating) {
future.get(CRASH_TIMEOUT, TimeUnit.SECONDS)
}
}
}

override fun sendAEIBlob(blobMessage: BlobMessage) {
Expand Down Expand Up @@ -234,8 +240,4 @@ internal class EmbraceDeliveryService(
apiService.sendEvent(eventMessage)
}
}

override fun sendEventAndWait(eventMessage: EventMessage) {
apiService.sendEventAndWait(eventMessage)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ internal class EmbraceNdkService(
"EmbraceNDKService",
"About to send EventMessage from native crash."
)
deliveryService.sendEventAndWait(nativeCrashMessageEvent)
deliveryService.sendCrash(nativeCrashMessageEvent, false)

Check warning on line 566 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/ndk/EmbraceNdkService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/ndk/EmbraceNdkService.kt#L566

Added line #L566 was not covered by tests
logger.logDeveloper(
"EmbraceNDKService",
"Finished send attempt for EventMessage from native crash."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ internal class EmbraceCrashServiceTest {
verify { CrashFactory.ofThrowable(testException, localJsException, any()) }
verify { anrService.forceAnrTrackingStopOnCrash() }

verify { deliveryService.sendCrash(any()) }
verify { deliveryService.sendCrash(any(), true) }
verify { sessionService.handleCrash(crash.crashId) }

/*
Expand All @@ -132,7 +132,7 @@ internal class EmbraceCrashServiceTest {
*/
embraceCrashService.handleCrash(Thread.currentThread(), testException)
verify(exactly = 1) { anrService.forceAnrTrackingStopOnCrash() }
verify(exactly = 1) { deliveryService.sendCrash(any()) }
verify(exactly = 1) { deliveryService.sendCrash(any(), true) }
verify(exactly = 1) { sessionService.handleCrash(crash.crashId) }
}

Expand All @@ -146,7 +146,7 @@ internal class EmbraceCrashServiceTest {

verify { CrashFactory.ofThrowable(testException, localJsException, "Unity123") }
verify { anrService.forceAnrTrackingStopOnCrash() }
verify { deliveryService.sendCrash(any()) }
verify { deliveryService.sendCrash(any(), true) }
verify { sessionService.handleCrash(crash.crashId) }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,7 @@ internal class FakeDeliveryService : DeliveryService {
lastSentNetworkCall = networkEvent
}

override fun sendEventAndWait(eventMessage: EventMessage) {
lastSentEvent = eventMessage
}

override fun sendCrash(crash: EventMessage) {
override fun sendCrash(crash: EventMessage, processTerminating: Boolean) {
lastSavedCrash = crash
lastSentCrash = crash
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ internal class EmbraceDeliveryServiceTest {
fun testSaveCrash() {
initializeDeliveryService()
val obj = EventMessage(Event(eventId = "abc", type = EmbraceEvent.Type.CRASH))
deliveryService.sendCrash(obj)
deliveryService.sendCrash(obj, true)
verify(exactly = 1) { mockDeliveryCacheManager.saveCrash(obj) }
verify(exactly = 1) { apiService.sendCrash(obj) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@ internal class FakeApiService : ApiService {
TODO("Not yet implemented")
}

override fun sendEventAndWait(eventMessage: EventMessage) {
TODO("Not yet implemented")
}

override fun sendCrash(crash: EventMessage) {
override fun sendCrash(crash: EventMessage): Future<*> {
TODO("Not yet implemented")
}

Expand Down

0 comments on commit b4ffbf8

Please sign in to comment.