From ebf3b8272cbed3825c86c6d5147da5943bb9c4c9 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Thu, 9 Apr 2026 22:24:03 -0500 Subject: [PATCH] fix(service): resolve MeshService crash from eager notification channel init (#5034) --- app/proguard-rules.pro | 4 ++ .../service/AndroidNotificationManagerTest.kt | 5 ++- .../service/AndroidNotificationManager.kt | 19 ++++++-- .../meshtastic/core/service/MeshService.kt | 44 ++++++++++++------- 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/app/proguard-rules.pro b/app/proguard-rules.pro index 4d6c3924e..3db98de86 100644 --- a/app/proguard-rules.pro +++ b/app/proguard-rules.pro @@ -40,6 +40,10 @@ -dontobfuscate -optimizations !code/simplification/arithmetic,!field/*,!class/merging/*,!code/allocation/variable +# Koin DI: prevent R8 from merging exception classes (observed as io.ktor.http.URLDecodeException +# replacing Koin's InstanceCreationException in stack traces, making crashes undiagnosable). +-keep class org.koin.core.error.** { *; } + # R8 optimization for Kotlin null checks (AGP 9.0+) -processkotlinnullchecks remove diff --git a/core/service/src/androidHostTest/kotlin/org/meshtastic/core/service/AndroidNotificationManagerTest.kt b/core/service/src/androidHostTest/kotlin/org/meshtastic/core/service/AndroidNotificationManagerTest.kt index 4791c99bf..d385c5a16 100644 --- a/core/service/src/androidHostTest/kotlin/org/meshtastic/core/service/AndroidNotificationManagerTest.kt +++ b/core/service/src/androidHostTest/kotlin/org/meshtastic/core/service/AndroidNotificationManagerTest.kt @@ -65,10 +65,11 @@ class AndroidNotificationManagerTest { } @Test - fun `init removes legacy node channel and creates canonical node channel`() { + fun `dispatch removes legacy node channel and creates canonical node channel`() { createChannel("NodeEvent") - AndroidNotificationManager(context) + val manager = AndroidNotificationManager(context) + manager.dispatch(Notification(title = "Node", message = "Seen", category = Notification.Category.NodeEvent)) assertNull(systemNotificationManager.getNotificationChannel("NodeEvent")) assertNotNull(systemNotificationManager.getNotificationChannel(NotificationChannels.NEW_NODES)) diff --git a/core/service/src/androidMain/kotlin/org/meshtastic/core/service/AndroidNotificationManager.kt b/core/service/src/androidMain/kotlin/org/meshtastic/core/service/AndroidNotificationManager.kt index f15190c8a..17735e28c 100644 --- a/core/service/src/androidMain/kotlin/org/meshtastic/core/service/AndroidNotificationManager.kt +++ b/core/service/src/androidMain/kotlin/org/meshtastic/core/service/AndroidNotificationManager.kt @@ -40,11 +40,21 @@ class AndroidNotificationManager(private val context: Context) : NotificationMan private data class ChannelConfig(val id: String, val importance: Int) - init { - initChannels() - } + /** + * Tracks whether notification channels have been created. + * + * Channels are **not** created in the constructor because this singleton is instantiated by Koin during + * [org.meshtastic.core.service.MeshService.onCreate] on the main thread. The CMP [getString] helper uses + * [kotlinx.coroutines.runBlocking] which can fail in that context, crashing the entire service startup chain. + * Instead, channels are lazily ensured before the first [dispatch] call. Note that + * [MeshServiceNotificationsImpl.initChannels] already creates a superset of these channels when the orchestrator + * starts, so this lazy path is only a safety net for notifications dispatched before orchestrator initialization. + */ + private var channelsInitialized = false - private fun initChannels() { + private fun ensureChannelsInitialized() { + if (channelsInitialized) return + channelsInitialized = true if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { val channels = listOf( @@ -91,6 +101,7 @@ class AndroidNotificationManager(private val context: Context) : NotificationMan } override fun dispatch(notification: Notification) { + ensureChannelsInitialized() val builder = NotificationCompat.Builder(context, notification.category.channelConfig().id) .setContentTitle(notification.title) diff --git a/core/service/src/androidMain/kotlin/org/meshtastic/core/service/MeshService.kt b/core/service/src/androidMain/kotlin/org/meshtastic/core/service/MeshService.kt index c8b7fdfab..7bcc8c815 100644 --- a/core/service/src/androidMain/kotlin/org/meshtastic/core/service/MeshService.kt +++ b/core/service/src/androidMain/kotlin/org/meshtastic/core/service/MeshService.kt @@ -73,6 +73,8 @@ class MeshService : Service() { private val serviceJob = Job() private val serviceScope = CoroutineScope(Dispatchers.IO + serviceJob) + private var isServiceInitialized = false + private val myNodeNum: Int get() = nodeManager.myNodeNum.value ?: throw RadioNotConnectedException() @@ -96,25 +98,35 @@ class MeshService : Service() { } override fun onCreate() { - try { - super.onCreate() - } catch (e: IllegalStateException) { - // Koin can throw IllegalStateException in tests if the component is not created. - // This can happen if the service is started by the system (e.g. after a crash or on boot) - // before the test rule has a chance to create the component. - if (e.message?.contains("HiltAndroidRule") == true || e.message?.contains("Koin") == true) { - Logger.w(e) { "MeshService created before DI component was ready in test, stopping service" } - stopSelf() - return - } - throw e - } + super.onCreate() Logger.i { "Creating mesh service" } - orchestrator.start() + try { + orchestrator.start() + isServiceInitialized = true + } catch (e: IllegalStateException) { + // Koin throws IllegalStateException when the DI graph is not yet initialized. + // This can happen if the system restarts the service (e.g. after a crash or on boot) + // before Application.onCreate() has finished setting up Koin. + // In release builds, R8 may merge Koin's InstanceCreationException with unrelated + // exception classes (observed as io.ktor.http.URLDecodeException), so we cannot rely + // on the exception type alone. We catch IllegalStateException narrowly around the + // orchestrator/DI access — not around super.onCreate() — so framework exceptions + // still propagate normally. + Logger.e(e) { "MeshService: DI not ready, stopping service" } + stopSelf() + return + } } + @Suppress("ReturnCount") override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { + if (!isServiceInitialized) { + Logger.w { "onStartCommand called but service is not initialized (likely DI failure). Stopping." } + stopSelf() + return START_NOT_STICKY + } + val a = radioInterfaceService.getDeviceAddress() val wantForeground = a != null && a != "n" @@ -180,7 +192,9 @@ class MeshService : Service() { override fun onDestroy() { Logger.i { "Destroying mesh service" } ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE) - orchestrator.stop() + if (isServiceInitialized) { + orchestrator.stop() + } serviceJob.cancel() super.onDestroy() }