mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-12 01:07:46 +00:00
fix(notif): call startForeground on every start command
The ForegroundServiceDidNotStartInTimeException came from MainActivity.onResume calling NotificationRelayService.start() on every resume. Each startForegroundService() re-arms Android's "must call startForeground() within the timeout" requirement, even when the service is already running and already foregrounded. The old initializeForeground() early-returned via an `if (foregroundStarted) return` guard once it had been started once, so later startForegroundService() calls were never matched by a startForeground() — the re-armed requirement went unsatisfied and the OS crashed the whole app. ensureForeground() now runs on every onStartCommand (startForeground() is idempotent — it just refreshes the existing notification) and rebuilds the notification with the current relay count so repeated calls don't flicker back to "connecting". It also stopSelf()s on every promotion-failure path (not only ForegroundServiceStartNotAllowedException), which clears the OS fgRequired flag and cancels the pending timeout when promotion genuinely can't happen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012hCJDhJmCNkzqb7SaMWQgB
This commit is contained in:
+30
-17
@@ -132,7 +132,6 @@ class NotificationRelayService : Service() {
|
||||
private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
|
||||
private var relayServiceCollectorJob: Job? = null
|
||||
private var connectedRelayCount = 0
|
||||
private var foregroundStarted = false
|
||||
|
||||
override fun onBind(intent: Intent?): IBinder? = null
|
||||
|
||||
@@ -140,7 +139,7 @@ class NotificationRelayService : Service() {
|
||||
super.onCreate()
|
||||
Log.d(TAG, "Service created")
|
||||
createNotificationChannel()
|
||||
initializeForeground()
|
||||
ensureForeground()
|
||||
}
|
||||
|
||||
override fun onStartCommand(
|
||||
@@ -149,13 +148,16 @@ class NotificationRelayService : Service() {
|
||||
startId: Int,
|
||||
): Int {
|
||||
Log.d(TAG, "Starting service")
|
||||
// Safety: also call startForeground from onStartCommand in case
|
||||
// onCreate didn't complete before onStartCommand fired (ntfy #1520)
|
||||
initializeForeground()
|
||||
// If we couldn't promote to the foreground, initializeForeground() already
|
||||
// called stopSelf(); don't spin up relay coroutines on a service that's
|
||||
// tearing itself down.
|
||||
if (!foregroundStarted) return START_NOT_STICKY
|
||||
// Every startForegroundService() call re-arms Android's "must call
|
||||
// startForeground() within the timeout" requirement — including the repeated
|
||||
// calls MainActivity.onResume fires on each resume, even when the service is
|
||||
// already running and already foregrounded. We therefore call startForeground()
|
||||
// on EVERY start command, not just the first. Skipping it on later starts (the
|
||||
// old `if (foregroundStarted) return` guard) left the re-armed requirement
|
||||
// unsatisfied and triggered ForegroundServiceDidNotStartInTimeException, which
|
||||
// crashes the whole app. startForeground() is idempotent — repeated calls just
|
||||
// refresh the existing notification.
|
||||
if (!ensureForeground()) return START_NOT_STICKY
|
||||
startRelayConnection()
|
||||
return START_STICKY
|
||||
}
|
||||
@@ -210,10 +212,20 @@ class NotificationRelayService : Service() {
|
||||
super.onDestroy()
|
||||
}
|
||||
|
||||
private fun initializeForeground() {
|
||||
if (foregroundStarted) return
|
||||
/**
|
||||
* Promotes the service to the foreground. Called on creation AND on every
|
||||
* [onStartCommand], because each [Context.startForegroundService] re-arms the
|
||||
* "must call startForeground() within the timeout" requirement and the old code's
|
||||
* early-return-when-already-started skipped it, crashing the app with
|
||||
* ForegroundServiceDidNotStartInTimeException. Calling startForeground() repeatedly
|
||||
* is safe — it just refreshes the existing notification.
|
||||
*
|
||||
* Returns true if the service is foregrounded, false if promotion failed (in which
|
||||
* case the service has already been told to stop).
|
||||
*/
|
||||
private fun ensureForeground(): Boolean {
|
||||
try {
|
||||
val notification = buildNotification(0)
|
||||
val notification = buildNotification(connectedRelayCount)
|
||||
ServiceCompat.startForeground(
|
||||
this,
|
||||
NOTIFICATION_ID,
|
||||
@@ -224,14 +236,14 @@ class NotificationRelayService : Service() {
|
||||
0
|
||||
},
|
||||
)
|
||||
foregroundStarted = true
|
||||
return true
|
||||
} catch (e: Exception) {
|
||||
// Any failure to promote to the foreground leaves the service in a
|
||||
// "started but not foregrounded" zombie state. Once startForegroundService()
|
||||
// has been called, Android expects startForeground() within ~10s; if it never
|
||||
// lands it fires ForegroundServiceDidNotStartInTimeException and crashes the
|
||||
// whole app. Tearing the service down clears the OS's fgRequired flag, which
|
||||
// cancels that pending timeout, so we must stopSelf() on EVERY failure path —
|
||||
// has been called, Android expects startForeground() within the timeout; if it
|
||||
// never lands it fires ForegroundServiceDidNotStartInTimeException and crashes
|
||||
// the whole app. Tearing the service down clears the OS's fgRequired flag,
|
||||
// which cancels that pending timeout, so we stopSelf() on EVERY failure path —
|
||||
// not just ForegroundServiceStartNotAllowedException. Other causes include
|
||||
// OEM-specific RemoteException/IllegalStateException and resource lookup
|
||||
// failures while building the notification.
|
||||
@@ -243,6 +255,7 @@ class NotificationRelayService : Service() {
|
||||
Log.e(TAG, "Failed to start foreground, stopping self", e)
|
||||
}
|
||||
stopSelf()
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user