Commit bcd72454 authored by MozLando's avatar MozLando
Browse files

Merge #6738



6738: Issue #1705: Allow CrashReporterService to return a unique identifier for reported crashes. r=rocketsroger a=pocmo

This is one small piece of #1705. I am going to land some pieces independently to avoid ending up with a huge patch.

Here we are allowing a `CrashReporterService` to return an ID that we can later use to lookup that crash. For Socorro we return the crash ID that we get when uploading. We can later use that to create a URL pointing to this crash. For Sentry we return the UUID of the event. We can at least ask Sentry with this UUID for the actual crash URL.
Co-authored-by: default avatarSebastian Kaspari <s.kaspari@gmail.com>
parents 80d98598 593c29e0
......@@ -14,16 +14,25 @@ internal const val INFO_PREFIX = "[INFO]"
interface CrashReporterService {
/**
* Submits a crash report for this [Crash.UncaughtExceptionCrash].
*
* @return Unique identifier that can be used by/with this crash reporter service to find this
* crash - or null if no identifier can be provided.
*/
fun report(crash: Crash.UncaughtExceptionCrash)
fun report(crash: Crash.UncaughtExceptionCrash): String?
/**
* Submits a crash report for this [Crash.NativeCodeCrash].
*
* @return Unique identifier that can be used by/with this crash reporter service to find this
* crash - or null if no identifier can be provided.
*/
fun report(crash: Crash.NativeCodeCrash)
fun report(crash: Crash.NativeCodeCrash): String?
/**
* Submits a caught exception report for this [Throwable].
*
* @return Unique identifier that can be used by/with this crash reporter service to find this
* crash - or null if no identifier can be provided.
*/
fun report(throwable: Throwable)
fun report(throwable: Throwable): String?
}
......@@ -173,19 +173,22 @@ class GleanCrashReporterService(
}
}
override fun report(crash: Crash.UncaughtExceptionCrash) {
override fun report(crash: Crash.UncaughtExceptionCrash): String? {
reportCrash(UNCAUGHT_EXCEPTION_KEY)
return null
}
override fun report(crash: Crash.NativeCodeCrash) {
override fun report(crash: Crash.NativeCodeCrash): String? {
if (crash.isFatal) {
reportCrash(FATAL_NATIVE_CODE_CRASH_KEY)
} else {
reportCrash(NONFATAL_NATIVE_CODE_CRASH_KEY)
}
return null
}
override fun report(throwable: Throwable) {
override fun report(throwable: Throwable): String? {
reportCrash(CAUGHT_EXCEPTION_KEY)
return null
}
}
......@@ -9,7 +9,6 @@ import android.content.Context
import android.content.pm.PackageManager
import android.os.Build
import androidx.annotation.VisibleForTesting
import mozilla.components.Build as AcBuild
import mozilla.components.lib.crash.Crash
import mozilla.components.support.base.log.logger.Logger
import org.json.JSONException
......@@ -30,8 +29,8 @@ import java.net.URL
import java.nio.channels.Channels
import java.util.concurrent.TimeUnit
import java.util.zip.GZIPOutputStream
import kotlin.collections.HashMap
import kotlin.random.Random
import mozilla.components.Build as AcBuild
/* This ID is used for all Mozilla products. Setting as default if no ID is passed in */
private const val MOZILLA_PRODUCT_ID = "{eeb82917-e434-4870-8148-5c03d4caa81b}"
......@@ -43,6 +42,9 @@ internal const val FATAL_NATIVE_CRASH_TYPE = "fatal native crash"
internal const val NON_FATAL_NATIVE_CRASH_TYPE = "non-fatal native crash"
internal const val DEFAULT_VERSION_NAME = "N/A"
private const val KEY_CRASH_ID = "CrashID"
/**
* A [CrashReporterService] implementation uploading crash reports to crash-stats.mozilla.com.
*
......@@ -91,16 +93,34 @@ class MozillaSocorroService(
}
}
override fun report(crash: Crash.UncaughtExceptionCrash) {
sendReport(crash.throwable, null, null, isNativeCodeCrash = false, isFatalCrash = true)
override fun report(crash: Crash.UncaughtExceptionCrash): String? {
return sendReport(
crash.throwable,
miniDumpFilePath = null,
extrasFilePath = null,
isNativeCodeCrash = false,
isFatalCrash = true
)
}
override fun report(crash: Crash.NativeCodeCrash) {
sendReport(null, crash.minidumpPath, crash.extrasPath, isNativeCodeCrash = true, isFatalCrash = crash.isFatal)
override fun report(crash: Crash.NativeCodeCrash): String? {
return sendReport(
throwable = null,
miniDumpFilePath = crash.minidumpPath,
extrasFilePath = crash.extrasPath,
isNativeCodeCrash = true,
isFatalCrash = crash.isFatal
)
}
override fun report(throwable: Throwable) {
sendReport(throwable, null, null, isNativeCodeCrash = false, isFatalCrash = false)
override fun report(throwable: Throwable): String? {
return sendReport(
throwable,
miniDumpFilePath = null,
extrasFilePath = null,
isNativeCodeCrash = false,
isFatalCrash = false
)
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
......@@ -110,7 +130,7 @@ class MozillaSocorroService(
extrasFilePath: String?,
isNativeCodeCrash: Boolean,
isFatalCrash: Boolean
) {
): String? {
val url = URL(serverUrl)
val boundary = generateBoundary()
var conn: HttpURLConnection? = null
......@@ -125,23 +145,41 @@ class MozillaSocorroService(
sendCrashData(conn.outputStream, boundary, throwable, miniDumpFilePath, extrasFilePath,
isNativeCodeCrash, isFatalCrash)
BufferedReader(InputStreamReader(conn.inputStream)).use {
val response = StringBuffer()
var inputLine = it.readLine()
while (inputLine != null) {
response.append(inputLine)
inputLine = it.readLine()
BufferedReader(InputStreamReader(conn.inputStream)).use { reader ->
val map = parseResponse(reader)
val id = map?.get(KEY_CRASH_ID)
if (id != null) {
Logger.info("Crash reported to Socorro: $id")
} else {
Logger.info("Server rejected crash report")
}
Logger.info("Crash reported to Socorro: $response")
return id
}
} catch (e: IOException) {
Logger.error("failed to send report to Socorro", e)
return null
} finally {
conn?.disconnect()
}
}
private fun parseResponse(reader: BufferedReader): Map<String, String>? {
val map = mutableMapOf<String, String>()
reader.readLines().forEach { line ->
val position = line.indexOf("=")
if (position != -1) {
val key = line.substring(0, position)
val value = unescape(line.substring(position + 1))
map[key] = value
}
}
return map
}
@Suppress("LongParameterList", "LongMethod")
private fun sendCrashData(
os: OutputStream,
......
......@@ -64,7 +64,7 @@ class SentryService(
}
}
override fun report(crash: Crash.UncaughtExceptionCrash) {
override fun report(crash: Crash.UncaughtExceptionCrash): String? {
crash.breadcrumbs.forEach {
client.context.recordBreadcrumb(it.toSentryBreadcrumb())
}
......@@ -73,22 +73,36 @@ class SentryService(
.withLevel(Event.Level.FATAL)
.withSentryInterface(ExceptionInterface(crash.throwable))
client.sendEvent(eventBuilder)
return eventBuilder.event.id.toString()
}
override fun report(crash: Crash.NativeCodeCrash) {
override fun report(crash: Crash.NativeCodeCrash): String? {
if (sendEventForNativeCrashes) {
crash.breadcrumbs.forEach {
client.context.recordBreadcrumb(it.toSentryBreadcrumb())
}
client.sendMessage(createMessage(crash))
val eventBuilder = EventBuilder()
.withMessage(createMessage(crash))
.withLevel(Event.Level.INFO)
client.sendEvent(eventBuilder)
return eventBuilder.event.id.toString()
}
return null
}
override fun report(throwable: Throwable) {
override fun report(throwable: Throwable): String? {
val eventBuilder = EventBuilder().withMessage(createMessage(throwable))
.withLevel(Event.Level.INFO)
.withSentryInterface(ExceptionInterface(throwable))
client.sendEvent(eventBuilder)
return eventBuilder.event.id.toString()
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
......
......@@ -303,15 +303,14 @@ class CrashReporterTest {
var exceptionCrash = false
val service = object : CrashReporterService {
override fun report(crash: Crash.UncaughtExceptionCrash) {
override fun report(crash: Crash.UncaughtExceptionCrash): String? {
exceptionCrash = true
return null
}
override fun report(crash: Crash.NativeCodeCrash) {
}
override fun report(crash: Crash.NativeCodeCrash): String? = null
override fun report(throwable: Throwable) {
}
override fun report(throwable: Throwable): String? = null
}
val reporter = spy(CrashReporter(
......@@ -330,15 +329,14 @@ class CrashReporterTest {
var nativeCrash = false
val service = object : CrashReporterService {
override fun report(crash: Crash.UncaughtExceptionCrash) {
}
override fun report(crash: Crash.UncaughtExceptionCrash): String? = null
override fun report(crash: Crash.NativeCodeCrash) {
override fun report(crash: Crash.NativeCodeCrash): String? {
nativeCrash = true
return null
}
override fun report(throwable: Throwable) {
}
override fun report(throwable: Throwable): String? = null
}
val reporter = spy(CrashReporter(
......@@ -357,14 +355,13 @@ class CrashReporterTest {
var exceptionCrash = false
val service = object : CrashReporterService {
override fun report(crash: Crash.UncaughtExceptionCrash) {
}
override fun report(crash: Crash.UncaughtExceptionCrash): String? = null
override fun report(crash: Crash.NativeCodeCrash) {
}
override fun report(crash: Crash.NativeCodeCrash): String? = null
override fun report(throwable: Throwable) {
override fun report(throwable: Throwable): String? {
exceptionCrash = true
return null
}
}
......@@ -385,15 +382,14 @@ class CrashReporterTest {
var nativeCrash = false
val telemetryService = object : CrashReporterService {
override fun report(crash: Crash.UncaughtExceptionCrash) {
}
override fun report(crash: Crash.UncaughtExceptionCrash): String? = null
override fun report(crash: Crash.NativeCodeCrash) {
override fun report(crash: Crash.NativeCodeCrash): String? {
nativeCrash = true
return null
}
override fun report(throwable: Throwable) {
}
override fun report(throwable: Throwable): String? = null
}
val reporter = spy(CrashReporter(
......
......@@ -60,14 +60,11 @@ class ExceptionHandlerTest {
val crashReporter = CrashReporter(
shouldPrompt = CrashReporter.Prompt.NEVER,
services = listOf(object : CrashReporterService {
override fun report(crash: Crash.UncaughtExceptionCrash) {
}
override fun report(crash: Crash.UncaughtExceptionCrash): String? = null
override fun report(crash: Crash.NativeCodeCrash) {
}
override fun report(crash: Crash.NativeCodeCrash): String? = null
override fun report(throwable: Throwable) {
}
override fun report(throwable: Throwable): String? = null
}),
scope = scope
).install(testContext)
......
......@@ -57,16 +57,19 @@ class SendCrashReportServiceTest {
val crashReporter = spy(CrashReporter(
shouldPrompt = CrashReporter.Prompt.NEVER,
services = listOf(object : CrashReporterService {
override fun report(crash: Crash.UncaughtExceptionCrash) {
override fun report(crash: Crash.UncaughtExceptionCrash): String? {
fail("Didn't expect uncaught exception crash")
return null
}
override fun report(crash: Crash.NativeCodeCrash) {
override fun report(crash: Crash.NativeCodeCrash): String? {
caughtCrash = crash
return null
}
override fun report(throwable: Throwable) {
override fun report(throwable: Throwable): String? {
fail("Didn't expect caught exception")
return null
}
}),
scope = scope
......
......@@ -55,16 +55,19 @@ class SendCrashTelemetryServiceTest {
val crashReporter = spy(CrashReporter(
shouldPrompt = CrashReporter.Prompt.NEVER,
telemetryServices = listOf(object : CrashReporterService {
override fun report(crash: Crash.UncaughtExceptionCrash) {
override fun report(crash: Crash.UncaughtExceptionCrash): String? {
fail("Didn't expect uncaught exception crash")
return null
}
override fun report(crash: Crash.NativeCodeCrash) {
override fun report(crash: Crash.NativeCodeCrash): String? {
caughtCrash = crash
return null
}
override fun report(throwable: Throwable) {
override fun report(throwable: Throwable): String? {
fail("Didn't expect caught exception")
return null
}
}),
scope = scope
......
......@@ -106,7 +106,7 @@ class SentryServiceTest {
}
@Test
fun `SentryService sends message for native code crashes`() {
fun `SentryService sends event for native code crashes`() {
val client: SentryClient = mock()
val service = SentryService(
......@@ -119,11 +119,11 @@ class SentryServiceTest {
service.report(Crash.NativeCodeCrash("", true, "", false, arrayListOf()))
verify(client).sendMessage(any())
verify(client).sendEvent(any<EventBuilder>())
}
@Test
fun `SentryService does not send message for native code crashes by default`() {
fun `SentryService does not send event for native code crashes by default`() {
val client: SentryClient = mock()
val service = SentryService(
......@@ -135,7 +135,7 @@ class SentryServiceTest {
service.report(Crash.NativeCodeCrash("", true, "", false, arrayListOf()))
verify(client, never()).sendMessage(any())
verify(client, never()).sendEvent(any<EventBuilder>())
}
@Test
......
......@@ -55,22 +55,25 @@ private fun createDummyCrashService(context: Context): CrashReporterService {
// For this sample we create a dummy service. In a real application this would be an instance of SentryCrashService
// or SocorroCrashService.
return object : CrashReporterService {
override fun report(crash: Crash.UncaughtExceptionCrash) {
override fun report(crash: Crash.UncaughtExceptionCrash): String? {
GlobalScope.launch(Dispatchers.Main) {
Toast.makeText(context, "Uploading uncaught exception crash...", Toast.LENGTH_SHORT).show()
}
return null
}
override fun report(crash: Crash.NativeCodeCrash) {
override fun report(crash: Crash.NativeCodeCrash): String? {
GlobalScope.launch(Dispatchers.Main) {
Toast.makeText(context, "Uploading native crash...", Toast.LENGTH_SHORT).show()
}
return null
}
override fun report(throwable: Throwable) {
override fun report(throwable: Throwable): String? {
GlobalScope.launch(Dispatchers.Main) {
Toast.makeText(context, "Uploading caught exception...", Toast.LENGTH_SHORT).show()
}
return null
}
}
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment