Skip to content
This repository has been archived by the owner on Jun 17, 2024. It is now read-only.

Commit

Permalink
[components] For mozilla-mobile/android-components#11358 - Show crash…
Browse files Browse the repository at this point in the history
… notification for BACKGROUND_CHILD process crashes.

This adds a processType field to NativeCodeCrash which is read from
the GeckoView crash intent. This can have one of the following values:

* MAIN: indicating a fatal crash in the main process.
* FOREGROUND_CHILD: indicating a crash in a foreground child process (such as a
  Gecko content process). The application may be able to recover from this.
* BACKGROUND_CHILD: indicating a crash in a background child process (such as
  Gecko's GPU process). The engine will automatically recover from this, and the
  crash will have barely been noticeable to the user, if at all.

The existing isFatal property is now calculated from the processType. It is true
for MAIN, and false for others.

We treat MAIN crashes as we did fatal crashes before: we do not send the intent
to the application, but do show the prompt or notification to the user, if
required.

We treat FOREGROUND_CHILD crashes as we did non-fatal crashes before: sending
the intent to the application if requested.

For BACKGROUND_CHILD crashes, we do _not_ send the intent to the application. As
the engine automatically recovers there is no need for the application to handle
the crash. Additionally, we choose to display the crash notification rather than
prompt, as we do not wish to interrupt the user's flow.
  • Loading branch information
jamienicol authored and mergify[bot] committed Mar 16, 2022
1 parent 9569578 commit 53d8bcb
Show file tree
Hide file tree
Showing 17 changed files with 331 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,10 @@ class SentryService(

private fun createMessage(crash: Crash.NativeCodeCrash): String {
val fatal = crash.isFatal.toString()
val processType = crash.processType
val minidumpSuccess = crash.minidumpSuccess

return "NativeCodeCrash(fatal=$fatal, minidumpSuccess=$minidumpSuccess)"
return "NativeCodeCrash(fatal=$fatal, processType=$processType, minidumpSuccess=$minidumpSuccess)"
}

private fun createMessage(crash: Crash.UncaughtExceptionCrash): String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ class SentryServiceTest {
sendEventForNativeCrashes = true
)

service.report(Crash.NativeCodeCrash(0, "", true, "", false, arrayListOf()))
service.report(Crash.NativeCodeCrash(0, "", true, "", Crash.NativeCodeCrash.PROCESS_TYPE_FOREGROUND_CHILD, arrayListOf()))

verify(client).sendEvent(any<EventBuilder>())
verify(clientContext).clearBreadcrumbs()
}

@Test
fun `Fatal native code crashes are reported as level FATAL`() {
fun `Main process native code crashes are reported as level FATAL`() {
val client: SentryClient = mock()
val clientContext: Context = mock()
doReturn(clientContext).`when`(client).context
Expand All @@ -164,7 +164,7 @@ class SentryServiceTest {
sendEventForNativeCrashes = true
)

service.report(Crash.NativeCodeCrash(0, "", true, "", true, arrayListOf()))
service.report(Crash.NativeCodeCrash(0, "", true, "", Crash.NativeCodeCrash.PROCESS_TYPE_MAIN, arrayListOf()))

val eventBuilderCaptor: ArgumentCaptor<EventBuilder> = ArgumentCaptor.forClass(EventBuilder::class.java)
verify(client, times(1)).sendEvent(eventBuilderCaptor.capture())
Expand All @@ -175,7 +175,7 @@ class SentryServiceTest {
}

@Test
fun `Non-fatal native code crashes are reported as level ERROR`() {
fun `Foreground child process native code crashes are reported as level ERROR`() {
val client: SentryClient = mock()
val clientContext: Context = mock()
doReturn(clientContext).`when`(client).context
Expand All @@ -190,7 +190,33 @@ class SentryServiceTest {
sendEventForNativeCrashes = true
)

service.report(Crash.NativeCodeCrash(0, "", true, "", false, arrayListOf()))
service.report(Crash.NativeCodeCrash(0, "", true, "", Crash.NativeCodeCrash.PROCESS_TYPE_FOREGROUND_CHILD, arrayListOf()))

val eventBuilderCaptor: ArgumentCaptor<EventBuilder> = ArgumentCaptor.forClass(EventBuilder::class.java)
verify(client, times(1)).sendEvent(eventBuilderCaptor.capture())

val capturedEvent: List<EventBuilder> = eventBuilderCaptor.allValues
assertEquals(Event.Level.ERROR, capturedEvent[0].event.level)
verify(clientContext).clearBreadcrumbs()
}

@Test
fun `Background child process native code crashes are reported as level ERROR`() {
val client: SentryClient = mock()
val clientContext: Context = mock()
doReturn(clientContext).`when`(client).context
doNothing().`when`(clientContext).clearBreadcrumbs()

val service = SentryService(
testContext,
"https://not:real6@sentry.prod.example.net/405",
clientFactory = object : SentryClientFactory() {
override fun createSentryClient(dsn: Dsn?): SentryClient = client
},
sendEventForNativeCrashes = true
)

service.report(Crash.NativeCodeCrash(0, "", true, "", Crash.NativeCodeCrash.PROCESS_TYPE_BACKGROUND_CHILD, arrayListOf()))

val eventBuilderCaptor: ArgumentCaptor<EventBuilder> = ArgumentCaptor.forClass(EventBuilder::class.java)
verify(client, times(1)).sendEvent(eventBuilderCaptor.capture())
Expand Down Expand Up @@ -264,7 +290,7 @@ class SentryServiceTest {
}
)

service.report(Crash.NativeCodeCrash(0, "", true, "", false, arrayListOf()))
service.report(Crash.NativeCodeCrash(0, "", true, "", Crash.NativeCodeCrash.PROCESS_TYPE_FOREGROUND_CHILD, arrayListOf()))

verify(client, never()).sendEvent(any<EventBuilder>())
}
Expand Down Expand Up @@ -373,7 +399,7 @@ class SentryServiceTest {
"dump.path",
true,
"extras.path",
isFatal = false,
processType = Crash.NativeCodeCrash.PROCESS_TYPE_FOREGROUND_CHILD,
breadcrumbs = crashBreadCrumbs
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,10 @@ class SentryService(
@VisibleForTesting
internal fun createMessage(crash: Crash.NativeCodeCrash): String {
val fatal = crash.isFatal.toString()
val processType = crash.processType
val minidumpSuccess = crash.minidumpSuccess

return "NativeCodeCrash(fatal=$fatal, minidumpSuccess=$minidumpSuccess)"
return "NativeCodeCrash(fatal=$fatal, processType=$processType, minidumpSuccess=$minidumpSuccess)"
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class SentryServiceTest {
}

@Test
fun `GIVEN a fatal native crash WHEN reporting THEN forward to a fatal crash the Sentry sdk`() {
fun `GIVEN a main process native crash WHEN reporting THEN forward to a fatal crash the Sentry sdk`() {
val service = spy(
SentryService(
applicationContext = testContext,
Expand All @@ -80,7 +80,7 @@ class SentryServiceTest {
minidumpPath = "",
minidumpSuccess = true,
extrasPath = "",
isFatal = true,
processType = Crash.NativeCodeCrash.PROCESS_TYPE_MAIN,
breadcrumbs = breadcrumbs
)

Expand All @@ -91,7 +91,7 @@ class SentryServiceTest {
}

@Test
fun `GIVEN a no fatal native crash WHEN reporting THEN forward an error to the Sentry sdk`() {
fun `GIVEN a foreground child process native crash WHEN reporting THEN forward an error to the Sentry sdk`() {
val service = spy(
SentryService(
applicationContext = testContext,
Expand All @@ -106,7 +106,33 @@ class SentryServiceTest {
minidumpPath = "",
minidumpSuccess = true,
extrasPath = "",
isFatal = false,
processType = Crash.NativeCodeCrash.PROCESS_TYPE_FOREGROUND_CHILD,
breadcrumbs = breadcrumbs
)

service.report(nativeCrash)

verify(service).prepareReport(breadcrumbs, SentryLevel.ERROR)
verify(service).reportToSentry(nativeCrash)
}

@Test
fun `GIVEN a background child process native crash WHEN reporting THEN forward an error to the Sentry sdk`() {
val service = spy(
SentryService(
applicationContext = testContext,
dsn = "https://not:real6@sentry.prod.example.net/405",
sendEventForNativeCrashes = true
)
)

val breadcrumbs = arrayListOf<Breadcrumb>()
val nativeCrash = Crash.NativeCodeCrash(
timestamp = 0,
minidumpPath = "",
minidumpSuccess = true,
extrasPath = "",
processType = Crash.NativeCodeCrash.PROCESS_TYPE_BACKGROUND_CHILD,
breadcrumbs = breadcrumbs
)

Expand All @@ -132,7 +158,7 @@ class SentryServiceTest {
minidumpPath = "",
minidumpSuccess = true,
extrasPath = "",
isFatal = false,
processType = Crash.NativeCodeCrash.PROCESS_TYPE_FOREGROUND_CHILD,
breadcrumbs = breadcrumbs
)

Expand All @@ -157,13 +183,13 @@ class SentryServiceTest {
minidumpPath = "",
minidumpSuccess = true,
extrasPath = "",
isFatal = true,
processType = Crash.NativeCodeCrash.PROCESS_TYPE_MAIN,
breadcrumbs = breadcrumbs
)

val result = service.createMessage(nativeCrash)
val expected =
"NativeCodeCrash(fatal=${nativeCrash.isFatal}, minidumpSuccess=${nativeCrash.minidumpSuccess})"
"NativeCodeCrash(fatal=${nativeCrash.isFatal}, processType=${nativeCrash.processType}, minidumpSuccess=${nativeCrash.minidumpSuccess})"

assertEquals(expected, result)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package mozilla.components.lib.crash

import android.content.Intent
import android.os.Bundle
import androidx.annotation.StringDef
import mozilla.components.concept.base.crash.Breadcrumb
import java.io.Serializable
import java.util.UUID
Expand All @@ -27,7 +28,7 @@ private const val INTENT_UUID = "uuid"
private const val INTENT_MINIDUMP_PATH = "minidumpPath"
private const val INTENT_EXTRAS_PATH = "extrasPath"
private const val INTENT_MINIDUMP_SUCCESS = "minidumpSuccess"
private const val INTENT_FATAL = "fatal"
private const val INTENT_PROCESS_TYPE = "processType"

/**
* Crash types that are handled by this library.
Expand Down Expand Up @@ -78,17 +79,16 @@ sealed class Crash {
* @property extrasPath Path to a file containing extra metadata about the crash. The file contains key-value pairs
* in the form `Key=Value`. Be aware, it may contain sensitive data such as the URI that was
* loaded at the time of the 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 processType The type of process the crash occurred in. Affects whether or not the crash is fatal
* or whether the application can recover from it.
* @property breadcrumbs List of breadcrumbs to send with the crash report.
*/
data class NativeCodeCrash(
val timestamp: Long,
val minidumpPath: String?,
val minidumpSuccess: Boolean,
val extrasPath: String?,
val isFatal: Boolean,
@ProcessType val processType: String?,
val breadcrumbs: ArrayList<Breadcrumb>,
override val uuid: String = UUID.randomUUID().toString()
) : Crash() {
Expand All @@ -97,18 +97,45 @@ sealed class Crash {
putString(INTENT_MINIDUMP_PATH, minidumpPath)
putBoolean(INTENT_MINIDUMP_SUCCESS, minidumpSuccess)
putString(INTENT_EXTRAS_PATH, extrasPath)
putBoolean(INTENT_FATAL, isFatal)
putString(INTENT_PROCESS_TYPE, processType)
putLong(INTENT_CRASH_TIMESTAMP, timestamp)
putParcelableArrayList(INTENT_BREADCRUMBS, breadcrumbs)
}

/**
* Whether 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.
*/
val isFatal: Boolean
get() = processType == PROCESS_TYPE_MAIN

companion object {
/**
* Indicates a crash occurred in the main process and is therefore fatal.
*/
const val PROCESS_TYPE_MAIN = "MAIN"
/**
* Indicates a crash occurred in a foreground child process. The application may be
* able to recover from this crash, but it was likely noticable to the user.
*/
const val PROCESS_TYPE_FOREGROUND_CHILD = "FOREGROUND_CHILD"
/**
* Indicates a crash occurred in a background child process. This should have been
* recovered from automatically, and will have had minimal impact to the user, if any.
*/
const val PROCESS_TYPE_BACKGROUND_CHILD = "BACKGROUND_CHILD"

@StringDef(PROCESS_TYPE_MAIN, PROCESS_TYPE_FOREGROUND_CHILD, PROCESS_TYPE_BACKGROUND_CHILD)
@Retention(AnnotationRetention.SOURCE)
annotation class ProcessType

internal fun fromBundle(bundle: Bundle) = NativeCodeCrash(
uuid = bundle.getString(INTENT_UUID) ?: UUID.randomUUID().toString(),
minidumpPath = bundle.getString(INTENT_MINIDUMP_PATH, null),
minidumpSuccess = bundle.getBoolean(INTENT_MINIDUMP_SUCCESS, false),
extrasPath = bundle.getString(INTENT_EXTRAS_PATH, null),
isFatal = bundle.getBoolean(INTENT_FATAL, false),
processType = bundle.getString(INTENT_PROCESS_TYPE, PROCESS_TYPE_MAIN),
breadcrumbs = bundle.getParcelableArrayList(INTENT_BREADCRUMBS) ?: arrayListOf(),
timestamp = bundle.getLong(INTENT_CRASH_TIMESTAMP, System.currentTimeMillis())
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,11 @@ class CrashReporter(
// If the app has not registered any intent then we can't send one.
false
} else {
// If this is a non-fatal native code crash then we can recover and can notify the app
crash is Crash.NativeCodeCrash && !crash.isFatal
// If this is a native code crash in a foreground child process then we can recover
// and can notify the app. Background child process crashes will be recovered from
// automatically, and main process crashes cannot be recovered from, so we do not
// send the intent for those.
crash is Crash.NativeCodeCrash && crash.processType == Crash.NativeCodeCrash.PROCESS_TYPE_FOREGROUND_CHILD
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,19 @@ internal class CrashNotification(

val channel = ensureChannelExists(context)

val title = if (crash is Crash.NativeCodeCrash &&
crash.processType == Crash.NativeCodeCrash.PROCESS_TYPE_BACKGROUND_CHILD
) {
context.getString(
R.string.mozac_lib_crash_background_process_notification_title,
configuration.appName
)
} else {
context.getString(R.string.mozac_lib_crash_dialog_title, configuration.appName)
}

val notification = NotificationCompat.Builder(context, channel)
.setContentTitle(context.getString(R.string.mozac_lib_crash_dialog_title, configuration.appName))
.setContentTitle(title)
.setSmallIcon(R.drawable.mozac_lib_crash_notification)
.setPriority(NotificationCompat.PRIORITY_DEFAULT)
.setCategory(NotificationCompat.CATEGORY_ERROR)
Expand All @@ -73,8 +84,15 @@ internal class CrashNotification(
* services launching activities in Q+. On those system we may need to show a notification for the given [crash]
* and launch the reporter from the notification.
*/
fun shouldShowNotificationInsteadOfPrompt(crash: Crash, sdkLevel: Int = Build.VERSION.SDK_INT): Boolean {
fun shouldShowNotificationInsteadOfPrompt(
crash: Crash,
sdkLevel: Int = Build.VERSION.SDK_INT
): Boolean {
return when {
// We always want to show the notification for background child process crashes.
crash is Crash.NativeCodeCrash && crash.processType ==
Crash.NativeCodeCrash.PROCESS_TYPE_BACKGROUND_CHILD -> true

// We can always launch an activity from a background service pre Android Q.
sdkLevel < NOTIFICATION_SDK_LEVEL -> false

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
<!-- Name of the "notification channel" used for displaying a notification when the app crashed and we can't show a prompt. See https://developer.android.com/training/notify-user/channels -->
<string name="mozac_lib_crash_channel">Crashes</string>

<!-- Title of the crash reporter notification for background process crashes. %1$s will be replaced with the name of the app (e.g. Firefox Focus). -->
<string name="mozac_lib_crash_background_process_notification_title">Sorry. A problem occurred in %1$s.</string>

<!-- Label of a notification action/button that will send the crash report to Mozilla. -->
<string name="mozac_lib_crash_notification_action_report">Report</string>

Expand Down
Loading

0 comments on commit 53d8bcb

Please sign in to comment.