1869545 - Unable to report parent or GPU process crashes if notifications permission not enabled

Status: REOPENED

(Firefox for Android :: Crash Reporting, defect, P2)

Description

[Jamie Nicol [:jnicol]]

If the Android notifications permission has not been granted, then users are unable to report gecko parent process or GPU process crashes. These should show a notification which the user can click on to report it. (Content process crashes are shown inside the Fenix app, so are unaffected)

I see this in logcat:

    ActivityManager: Start proc 19040:org.mozilla.fenix:crashReportingProcess/u0a340 for service {org.mozilla.fenix/mozilla.components.lib.crash.service.SendCrashTelemetryService}
    ExceptionHandler: Uncaught exception handled:
    ExceptionHandler: mozilla.components.support.base.android.UnboundHandlerException: You must bind the NotificationPermissionHandler to an activity
    ExceptionHandler: 	at mozilla.components.support.base.android.NotificationsDelegate.requestNotificationPermission(NotificationsDelegate.kt:107)
    ExceptionHandler: 	at mozilla.components.support.base.android.NotificationsDelegate.notify$default(NotificationsDelegate.kt:73)
    ExceptionHandler: 	at mozilla.components.lib.crash.CrashReporter.onCrash$lib_crash_release(CrashReporter.kt:438)
    ExceptionHandler: 	at mozilla.components.lib.crash.handler.CrashHandlerService$handleCrashIntent$1.invokeSuspend(CrashHandlerService.kt:23)
    ExceptionHandler: 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:9)
    ExceptionHandler: 	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:112)
    ExceptionHandler: 	at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:4)
    ExceptionHandler: 	at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:3)
    ExceptionHandler: 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:96)
    ExceptionHandler: 	Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@2f27363, Dispatchers.IO]
    mozac/CrashReporter: Received crash: UncaughtExceptionCrash(timestamp=1702391692727, throwable=mozilla.components.support.base.android.UnboundHandlerException: You must bind the NotificationPermissionHandler to an activity, breadcrumbs=[], uuid=a4dbc082-ab18-4b0a-b2e2-943800ebc3ae)
    mozac/CrashReporter: Showing notification
    ExceptionHandler: Crash reporter has crashed.
    ExceptionHandler: mozilla.components.support.base.android.UnboundHandlerException: You must bind the NotificationPermissionHandler to an activity
    ExceptionHandler: 	at mozilla.components.support.base.android.NotificationsDelegate.requestNotificationPermission(NotificationsDelegate.kt:107)
    ExceptionHandler: 	at mozilla.components.support.base.android.NotificationsDelegate.notify$default(NotificationsDelegate.kt:73)
    ExceptionHandler: 	at mozilla.components.lib.crash.CrashReporter.onCrash$lib_crash_release(CrashReporter.kt:438)
    ExceptionHandler: 	at mozilla.components.lib.crash.handler.ExceptionHandler.uncaughtException(ExceptionHandler.kt:65)
    ExceptionHandler: 	at java.lang.ThreadGroup.uncaughtException(ThreadGroup.java:1071)
    ExceptionHandler: 	at java.lang.ThreadGroup.uncaughtException(ThreadGroup.java:1066)
    ExceptionHandler: 	at com.google.android.gms.common.wrappers.InstantApps.handleUncaughtCoroutineException(com.google.android.gms:play-services-basement@@18.1.0:66)
    ExceptionHandler: 	at kotlinx.coroutines.CoroutineExceptionHandlerKt.handleCoroutineException(CoroutineExceptionHandler.kt:15)
    ExceptionHandler: 	at kotlinx.coroutines.StandaloneCoroutine.handleJobException(Builders.common.kt:3)
    ExceptionHandler: 	at kotlinx.coroutines.JobSupport.finalizeFinishingState(JobSupport.kt:105)
    ExceptionHandler: 	at kotlinx.coroutines.JobSupport.tryMakeCompleting(JobSupport.kt:233)
    ExceptionHandler: 	at kotlinx.coroutines.JobSupport.makeCompletingOnce$kotlinx_coroutines_core(JobSupport.kt:5)
    ExceptionHandler: 	at kotlinx.coroutines.AbstractCoroutine.resumeWith(AbstractCoroutine.kt:14)
    ExceptionHandler: 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:32)
    ExceptionHandler: 	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:112)
    ExceptionHandler: 	at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:4)
    ExceptionHandler: 	at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:3)
    ExceptionHandler: 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:96)

Comment 1

[Jamie Nicol [:jnicol]]

A crash can be reproduced for testing by navigating to about:crashparent or about:crashgpu respectively

Comment 2

[Mihai Adrian Carare [:mcarare]]

Unfortunately, if at the moment of the crash, the notification permission is not already granted, we cannot show a new permission request because the activity has been destroyed ( that is the reason for the UnboundHandlerException.)
I have proposed a different mechanism in the past, keeping the latest crashes and prompting the user to send the reports, AFTER the app has been reopened after a crash. That would provide a workaround for the notification permission not being granted ( Only needed for Android 13+)

Comment 3

[Jamie Nicol [:jnicol]]

That is unfortunate. We should really prioritise a solution, as missing ~30% of our crashes isn't good. (The percentage of people who haven't granted the permission)

I would have thought for GPU process crashes at least the Activity should still exist? (It's certainly gone for parent process ones though)

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Would be good to make sure this doesn't stop the Crash Ping Telemetry for working.

Comment 5

[Alex Franchuk [:afranchuk]]

I think keeping the reports and sending when the app restarts is a good idea; prompting the user for a permission when the app has crashed doesn't come over well and may be confusing. If these aren't crashes which take down the entire app that might be a bit better, but it still may be confusing to suddenly be prompted for a permission.

(In reply to Gian-Carlo Pascutto [:gcp] from comment #4)

Would be good to make sure this doesn't stop the Crash Ping Telemetry for working.

It looks like it does. There are a few notifications in lib-crash. One (involving a prompt/notification for crashes) is avoided, but the SendCrashTelemetryService itself also shows a notification (and this is what is running and failing in comment #0, I think). The notification is shown (it says "Gathering crash data") and then I think it might be immediately cancelled, or it ends up cancelling some other notification (the code is an odd combination of requesting a notification ID to be generated and using a fixed value). Either way, it interacts with system notifications so I imagine that will produce an exception without the correct permissions.

What version of the app is running? The line numbers in the backtrace don't make sense to me.

Comment 6

[Jamie Nicol [:jnicol]]

Last night's nightly (122.0a1 2023-12-12T03:23:07)

Comment 7

[Alex Franchuk [:afranchuk]]

Am I missing something? CrashReporter.kt has 355 lines, but the backtrace references line 438. Does nightly not use the latest android-components?

Comment 8

[William Durand [:willdurand]]

Since Bug 1868778, this bug also affects the extensions process (which is a child process similar to the GPU process in Fenix).

Comment 9

[Alex Franchuk [:afranchuk]]

Following up to comment #5, the crash telemetry (ping) should be sent. I reviewed the android docs, which mention that notifications that are part of launching foreground services are not subject to permission requirements. And indeed, I've run the application with an android 13 emulator and have confirmed that the telemetry services are invoked and don't crash; the crash only occurs later in the crash report code. It was difficult to get android studio to attach to the crash handler as it's a separate process from the main app that only exists for a short time (though I'm guessing there's some way to do this and I just didn't bother looking), but I confirmed with logging.