Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Merge #6738
Browse files Browse the repository at this point in the history 
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: Sebastian Kaspari <[email protected]>
  • Loading branch information
@MozLando @pocmo
MozLando and pocmo committed Apr 27, 2020
2 parents 80d9859 + 593c29e commit bcd7245
Show file tree
Hide file tree
Showing 11 changed files with 608 additions and 402 deletions.
15 changes: 12 additions & 3 deletions ...ents/lib/crash/src/main/java/mozilla/components/lib/crash/service/CrashReporterService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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?
}
9 changes: 6 additions & 3 deletions ...lib/crash/src/main/java/mozilla/components/lib/crash/service/GleanCrashReporterService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
70 changes: 54 additions & 16 deletions ...nts/lib/crash/src/main/java/mozilla/components/lib/crash/service/MozillaSocorroService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}"
Expand All @@ -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.
*
Expand Down Expand Up @@ -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)
Expand All @@ -110,7 +130,7 @@ class MozillaSocorroService(
extrasFilePath: String?,
isNativeCodeCrash: Boolean,
isFatalCrash: Boolean
) {
): String? {
val url = URL(serverUrl)
val boundary = generateBoundary()
var conn: HttpURLConnection? = null
Expand All @@ -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,
Expand Down
22 changes: 18 additions & 4 deletions components/lib/crash/src/main/java/mozilla/components/lib/crash/service/SentryService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand All @@ -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)
Expand Down
36 changes: 16 additions & 20 deletions components/lib/crash/src/test/java/mozilla/components/lib/crash/CrashReporterTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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
}
}

Expand All @@ -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(
Expand Down
9 changes: 3 additions & 6 deletions ...ents/lib/crash/src/test/java/mozilla/components/lib/crash/handler/ExceptionHandlerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading

0 comments on commit bcd7245

Please sign in to comment.