Skip to content

Commit

Permalink
Issue mozilla-mobile#1705: Allow CrashReporterService to return a uni…
Browse files Browse the repository at this point in the history 
…que identifier for reported crashes.
  • Loading branch information
@pocmo
pocmo committed Apr 22, 2020
1 parent ab2ccd1 commit 593c29e
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 593c29e

Please sign in to comment.