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

…l and non-fatal crashes
rocketsroger · Jan 8, 2020 · 426a546 · 426a546
1 parent 1121929
commit 426a546
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 22 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/...ents/lib/crash/src/main/java/mozilla/components/lib/crash/service/CrashReporterService.kt b/...ents/lib/crash/src/main/java/mozilla/components/lib/crash/service/CrashReporterService.kt
@@ -7,6 +7,8 @@ package mozilla.components.lib.crash.service
 import mozilla.components.lib.crash.Crash
 
 internal const val INFO_PREFIX = "[INFO]"
+internal const val FATAL_PREFIX = "[FATAL]"
+internal const val NONFATAL_PREFIX = "[NONFATAL]"
 
 /**
  * Interface to be implemented by external services that accept crash reports.

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
@@ -61,22 +61,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 +92,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()
@@ -117,6 +118,7 @@ class MozillaSocorroService(
         throwable: Throwable?,
         miniDumpFilePath: String?,
         extrasFilePath: String?,
+        isFatal: Boolean,
         isCaughtException: Boolean
     ) {
         val nameSet = mutableSetOf<String>()
@@ -137,7 +139,7 @@ class MozillaSocorroService(
         }
 
         if (throwable?.stackTrace?.isEmpty() == false) {
-            sendPart(gzipOs, boundary, "JavaStackTrace", getExceptionStackTrace(throwable,
+            sendPart(gzipOs, boundary, "JavaStackTrace", getExceptionStackTrace(throwable, isFatal,
                 isCaughtException), nameSet)
         }
 
@@ -328,15 +330,20 @@ class MozillaSocorroService(
         return resultMap
     }
 
-    private fun getExceptionStackTrace(throwable: Throwable, isCaughtException: Boolean): String {
+    private fun getExceptionStackTrace(
+        throwable: Throwable,
+        isFatal: Boolean,
+        isCaughtException: Boolean
+    ): String {
         val stringWriter = StringWriter()
         val printWriter = PrintWriter(stringWriter)
         throwable.printStackTrace(printWriter)
         printWriter.flush()
 
-        return when (isCaughtException) {
-            true -> "$INFO_PREFIX $stringWriter"
-            false -> stringWriter.toString()
+        return when {
+            isFatal -> "$FATAL_PREFIX $stringWriter"
+            isCaughtException -> "$INFO_PREFIX $stringWriter"
+            else -> "$NONFATAL_PREFIX $stringWriter"
         }
     }
 }
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())
         service.report(crash)
 
         verify(service).report(crash)
-        verify(service).sendReport(null, crash.minidumpPath, crash.extrasPath, false)
+        verify(service).sendReport(null, crash.minidumpPath, crash.extrasPath, false, 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
@@ -96,7 +96,7 @@ class MozillaSocorroServiceTest {
         val bufferedReader = BufferedReader(reader)
         var request = bufferedReader.readText()
 
-        assert(request.contains("name=JavaStackTrace\r\n\r\njava.lang.RuntimeException: Test"))
+        assert(request.contains("name=JavaStackTrace\r\n\r\n$FATAL_PREFIX java.lang.RuntimeException: Test"))
         assert(request.contains("name=Android_ProcessName\r\n\r\nmozilla.components.lib.crash.test"))
         assert(request.contains("name=ProductID\r\n\r\n{aa3c5121-dab2-40e2-81ca-7ea25febc110}"))
         assert(request.contains("name=Vendor\r\n\r\nMozilla"))
@@ -106,7 +106,7 @@ class MozillaSocorroServiceTest {
         assertFalse(request.contains("name=Notes\r\n\r\n$CAUGHT_EXCEPTION_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
@@ -265,7 +265,7 @@ class MozillaSocorroServiceTest {
         mockWebServer.shutdown()
 
         verify(service).report(crash)
-        verify(service).sendReport(null, crash.minidumpPath, crash.extrasPath, false)
+        verify(service).sendReport(null, crash.minidumpPath, crash.extrasPath, false, false)
     }
 
     @Test