Closes mozilla-mobile#5175: Allow Socorro to distinguish between fata…

…l and non-fatal crashes
rocketsroger · Jan 9, 2020 · 769e079 · 769e079
1 parent 1121929
commit 769e079
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 23 deletions.
diff --git a/components/lib/crash/src/main/java/mozilla/components/lib/crash/Crash.kt b/components/lib/crash/src/main/java/mozilla/components/lib/crash/Crash.kt
@@ -31,6 +31,7 @@ sealed class Crash {
      * A crash caused by an uncaught exception.
      *
      * @property throwable The [Throwable] that caused the crash.
+     * @property breadcrumbs List of breadcrumbs to send with the crash report.
      */
     data class UncaughtExceptionCrash(
         val throwable: Throwable,
@@ -61,6 +62,7 @@ sealed class Crash {
      * @property isFatal Whether or not the crash was fatal or not: If true, the main application process was affected
      *                   by the crash. If false, only an internal process used by Gecko has crashed and the application
      *                   may be able to recover.
+     * @property breadcrumbs List of breadcrumbs to send with the crash report.
      */
     data class NativeCodeCrash(
         val minidumpPath: String?,

diff --git a/...nts/lib/crash/src/main/java/mozilla/components/lib/crash/service/MozillaSocorroService.kt b/...nts/lib/crash/src/main/java/mozilla/components/lib/crash/service/MozillaSocorroService.kt
@@ -35,6 +35,12 @@ import kotlin.random.Random
 /* 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}"
 
+@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+internal const val FATAL_CRASH_NOTE = "This is a fatal crash"
+
+@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+internal const val NONFATAL_CRASH_NOTE = "This is a non-fatal crash"
+
 @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
 internal const val CAUGHT_EXCEPTION_NOTE = "This is a caught exception, not a real crash"
 
@@ -61,22 +67,23 @@ class MozillaSocorroService(
     private val startTime = System.currentTimeMillis()
 
     override fun report(crash: Crash.UncaughtExceptionCrash) {
-        sendReport(crash.throwable, null, null, false)
+        sendReport(crash.throwable, null, null, isFatal = true, isCaughtException = false)
     }
 
     override fun report(crash: Crash.NativeCodeCrash) {
-        sendReport(null, crash.minidumpPath, crash.extrasPath, false)
+        sendReport(null, crash.minidumpPath, crash.extrasPath, isFatal = crash.isFatal, isCaughtException = false)
     }
 
     override fun report(throwable: Throwable) {
-        sendReport(throwable, null, null, true)
+        sendReport(throwable, null, null, isFatal = false, isCaughtException = true)
     }
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal fun sendReport(
         throwable: Throwable?,
         miniDumpFilePath: String?,
         extrasFilePath: String?,
+        isFatal: Boolean,
         isCaughtException: Boolean
     ) {
         val url = URL(serverUrl)
@@ -91,7 +98,7 @@ class MozillaSocorroService(
             conn.setRequestProperty("Content-Encoding", "gzip")
 
             sendCrashData(conn.outputStream, boundary, throwable, miniDumpFilePath, extrasFilePath,
-                    isCaughtException)
+                    isFatal, isCaughtException)
 
             BufferedReader(InputStreamReader(conn.inputStream)).use {
                 val response = StringBuffer()
@@ -110,13 +117,14 @@ class MozillaSocorroService(
         }
     }
 
-    @Suppress("LongParameterList")
+    @Suppress("LongParameterList", "LongMethod")
     private fun sendCrashData(
         os: OutputStream,
         boundary: String,
         throwable: Throwable?,
         miniDumpFilePath: String?,
         extrasFilePath: String?,
+        isFatal: Boolean,
         isCaughtException: Boolean
     ) {
         val nameSet = mutableSetOf<String>()
@@ -147,8 +155,10 @@ class MozillaSocorroService(
             minidumpFile.delete()
         }
 
-        if (isCaughtException) {
-            sendPart(gzipOs, boundary, "Notes", CAUGHT_EXCEPTION_NOTE, nameSet)
+        when {
+            isFatal -> sendPart(gzipOs, boundary, "Notes", FATAL_CRASH_NOTE, nameSet)
+            isCaughtException -> sendPart(gzipOs, boundary, "Notes", CAUGHT_EXCEPTION_NOTE, nameSet)
+            else -> sendPart(gzipOs, boundary, "Notes", NONFATAL_CRASH_NOTE, nameSet)
         }
 
         sendPackageInstallTime(gzipOs, boundary, nameSet)
@@ -328,7 +338,10 @@ class MozillaSocorroService(
         return resultMap
     }
 
-    private fun getExceptionStackTrace(throwable: Throwable, isCaughtException: Boolean): String {
+    private fun getExceptionStackTrace(
+        throwable: Throwable,
+        isCaughtException: Boolean
+    ): String {
         val stringWriter = StringWriter()
         val printWriter = PrintWriter(stringWriter)
         throwable.printStackTrace(printWriter)

diff --git a/...lib/crash/src/test/java/mozilla/components/lib/crash/service/MozillaSocorroServiceTest.kt b/...lib/crash/src/test/java/mozilla/components/lib/crash/service/MozillaSocorroServiceTest.kt
@@ -34,13 +34,13 @@ class MozillaSocorroServiceTest {
             testContext,
             "Test App"
         ))
-        doNothing().`when`(service).sendReport(any(), any(), any(), anyBoolean())
+        doNothing().`when`(service).sendReport(any(), any(), any(), anyBoolean(), anyBoolean())
 
-        val crash = Crash.NativeCodeCrash("", true, "", false, arrayListOf())
+        val crash = Crash.NativeCodeCrash("", true, "", true, arrayListOf())
         service.report(crash)
 
         verify(service).report(crash)
-        verify(service).sendReport(null, crash.minidumpPath, crash.extrasPath, false)
+        verify(service).sendReport(null, crash.minidumpPath, crash.extrasPath, true, false)
     }
 
     @Test
@@ -49,13 +49,13 @@ class MozillaSocorroServiceTest {
             testContext,
             "Test App"
         ))
-        doNothing().`when`(service).sendReport(any(), any(), any(), anyBoolean())
+        doNothing().`when`(service).sendReport(any(), any(), any(), anyBoolean(), anyBoolean())
 
         val crash = Crash.UncaughtExceptionCrash(RuntimeException("Test"), arrayListOf())
         service.report(crash)
 
         verify(service).report(crash)
-        verify(service).sendReport(crash.throwable, null, null, false)
+        verify(service).sendReport(crash.throwable, null, null, true, false)
     }
 
     @Test
@@ -64,13 +64,13 @@ class MozillaSocorroServiceTest {
                 testContext,
                 "Test App"
         ))
-        doNothing().`when`(service).sendReport(any(), any(), any(), anyBoolean())
+        doNothing().`when`(service).sendReport(any(), any(), any(), anyBoolean(), anyBoolean())
 
         val throwable = RuntimeException("Test")
         service.report(throwable)
 
         verify(service).report(throwable)
-        verify(service).sendReport(throwable, null, null, true)
+        verify(service).sendReport(throwable, null, null, false, true)
     }
 
     @Test
@@ -103,10 +103,10 @@ class MozillaSocorroServiceTest {
         assert(request.contains("name=ReleaseChannel\r\n\r\nnightly"))
         assert(request.contains("name=Android_PackageName\r\n\r\nmozilla.components.lib.crash.test"))
         assert(request.contains("name=Android_Device\r\n\r\nrobolectric"))
-        assertFalse(request.contains("name=Notes\r\n\r\n$CAUGHT_EXCEPTION_NOTE"))
+        assert(request.contains("name=Notes\r\n\r\n$FATAL_CRASH_NOTE"))
 
         verify(service).report(crash)
-        verify(service).sendReport(crash.throwable, null, null, false)
+        verify(service).sendReport(crash.throwable, null, null, true, false)
     }
 
     @Test
@@ -141,7 +141,7 @@ class MozillaSocorroServiceTest {
         assert(request.contains("name=Notes\r\n\r\n$CAUGHT_EXCEPTION_NOTE"))
 
         verify(service).report(throwable)
-        verify(service).sendReport(throwable, null, null, true)
+        verify(service).sendReport(throwable, null, null, false, true)
     }
 
     @Test
@@ -182,7 +182,7 @@ class MozillaSocorroServiceTest {
         assert(request.contains("name=Notes\r\n\r\n$CAUGHT_EXCEPTION_NOTE"))
 
         verify(service).report(throwable)
-        verify(service).sendReport(throwable, null, null, true)
+        verify(service).sendReport(throwable, null, null, false, true)
     }
 
     @Test
@@ -224,7 +224,7 @@ class MozillaSocorroServiceTest {
         assert(request.contains("name=Notes\r\n\r\n$CAUGHT_EXCEPTION_NOTE"))
 
         verify(service).report(throwable)
-        verify(service).sendReport(throwable, null, null, true)
+        verify(service).sendReport(throwable, null, null, false, true)
     }
 
     @Test
@@ -245,7 +245,7 @@ class MozillaSocorroServiceTest {
 
         mockWebServer.shutdown()
         verify(service).report(crash)
-        verify(service).sendReport(crash.throwable, null, null, false)
+        verify(service).sendReport(crash.throwable, null, null, true, false)
     }
 
     @Test
@@ -260,12 +260,12 @@ class MozillaSocorroServiceTest {
                 serverUrl = serverUrl.toString()
         ))
 
-        val crash = Crash.NativeCodeCrash(null, true, null, false, arrayListOf())
+        val crash = Crash.NativeCodeCrash(null, true, null, true, arrayListOf())
         service.report(crash)
         mockWebServer.shutdown()
 
         verify(service).report(crash)
-        verify(service).sendReport(null, crash.minidumpPath, crash.extrasPath, false)
+        verify(service).sendReport(null, crash.minidumpPath, crash.extrasPath, true, false)
     }
 
     @Test