mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix(desktop): surface macOS notification-permission OS errors + timeout the request
The "Enable OS notifications" button still fails on macOS even after
e9475dd0 + the auto-enable follow-up, and the failure mode gives the
user nothing to act on:
1. Nucleus's requestAuthorization callback carries the OS error string
(UNErrorDomain), but the dispatcher discarded it ({ granted, _ -> })
and mapped every non-grant to PermissionState.Denied. The settings UI
then showed "Enable in System Settings → Notifications → Amethyst" —
a dead end when macOS refused the request outright ("Notifications
are not allowed for this application"), because a refused app never
gets a System Settings entry.
2. On recent macOS the permission prompt is an auto-dismissing banner.
If the user misses it, UNUserNotificationCenter may never invoke the
completion handler, leaving requestPermission()'s
suspendCancellableCoroutine parked forever and the UI stuck on
"Requesting…".
Fixes:
- requestPermission() now captures the OS error string and exposes it
via NotificationDispatcher.lastRequestError (new interface property,
null-defaulted so other implementations are unaffected).
- The request is wrapped in withTimeoutOrNull(90s); on timeout the
coroutine returns, the spinner clears, and lastRequestError tells the
user to watch for the banner and retry.
- The Denied branch of NotificationSettingsScreen gains an "Ask again"
button (re-request re-surfaces the banner) and both branches render
the raw OS error when one is present.
- sendMac() now uses Nucleus's add(request, callback) overload and
reports SendResult.Failed with the OS error instead of unconditionally
returning Delivered for a request the notification center may have
rejected. Timeout without an ack still counts as delivered (the
request was queued).
Reproduced the hang + the silent-error path on macOS 26.4 with a
minimal Nucleus harness: first requestAuthorization call from a
freshly-installed bundle never fired its callback (30s timeout),
subsequent calls returned granted=false with "Notifications are not
allowed for this application" — neither observable from the Amethyst
UI before this change.
This commit is contained in:
+12
@@ -39,6 +39,18 @@ interface NotificationDispatcher {
|
||||
*/
|
||||
val nativeAvailable: StateFlow<Boolean>
|
||||
|
||||
/**
|
||||
* Human-readable error from the most recent [requestPermission] call,
|
||||
* or null if the last request succeeded / none has run yet. macOS can
|
||||
* refuse an authorization request outright (UNErrorDomain "Notifications
|
||||
* are not allowed for this application") without ever showing a prompt —
|
||||
* a different failure from the user clicking "Don't Allow", with a
|
||||
* different recovery. The settings UI surfaces this so the user gets
|
||||
* the real OS message instead of a generic "denied".
|
||||
*/
|
||||
val lastRequestError: StateFlow<String?>
|
||||
get() = kotlinx.coroutines.flow.MutableStateFlow(null)
|
||||
|
||||
/**
|
||||
* Trigger the OS-level permission prompt (macOS only — no-op elsewhere).
|
||||
* Suspends until the user answers. Updates [permission] as a side effect.
|
||||
|
||||
+83
-19
@@ -28,6 +28,7 @@ import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.suspendCancellableCoroutine
|
||||
import kotlinx.coroutines.withContext
|
||||
import kotlinx.coroutines.withTimeoutOrNull
|
||||
import java.util.UUID
|
||||
import kotlin.coroutines.resume
|
||||
|
||||
@@ -53,6 +54,15 @@ class NucleusNotificationDispatcher(
|
||||
) : NotificationDispatcher {
|
||||
private val host: HostOs = detectHostOs()
|
||||
|
||||
private companion object {
|
||||
const val PERMISSION_REQUEST_TIMEOUT_MS = 90_000L
|
||||
const val SEND_ACK_TIMEOUT_MS = 10_000L
|
||||
const val TIMEOUT_ERROR_MESSAGE =
|
||||
"macOS never answered the permission request. The permission banner may have " +
|
||||
"auto-dismissed — click again and watch the top-right of the screen for the " +
|
||||
"Amethyst banner, then click Allow."
|
||||
}
|
||||
|
||||
// Nucleus module availability probed once at construction. Native lib load
|
||||
// is triggered by the first class access — wrap in Throwable catch so a
|
||||
// missing JAR or unsatisfied link doesn't crash the app.
|
||||
@@ -62,6 +72,22 @@ class NucleusNotificationDispatcher(
|
||||
private val _permission = MutableStateFlow<PermissionState>(initialPermissionState())
|
||||
override val permission: StateFlow<PermissionState> = _permission.asStateFlow()
|
||||
|
||||
/**
|
||||
* Human-readable error from the most recent [requestPermission] call, or
|
||||
* null if the last request succeeded / none has run yet. macOS can refuse
|
||||
* the authorization request outright (e.g. UNErrorDomain
|
||||
* "Notifications are not allowed for this application" when the system
|
||||
* blocks the app before any prompt), which is NOT the same as the user
|
||||
* clicking "Don't Allow" — the recovery for each is different, and the
|
||||
* settings UI needs the raw OS message to show actionable guidance.
|
||||
* A timeout marker is also written here when the OS never invokes the
|
||||
* completion handler (the permission banner can be auto-dismissed on
|
||||
* recent macOS versions without firing the callback, which previously
|
||||
* left the request coroutine suspended forever).
|
||||
*/
|
||||
private val _lastRequestError = MutableStateFlow<String?>(null)
|
||||
override val lastRequestError: StateFlow<String?> = _lastRequestError.asStateFlow()
|
||||
|
||||
@Volatile
|
||||
private var winInitialized: Boolean = false
|
||||
|
||||
@@ -98,25 +124,42 @@ class NucleusNotificationDispatcher(
|
||||
_permission.value = PermissionState.BundleRequired
|
||||
return _permission.value
|
||||
}
|
||||
val granted =
|
||||
val outcome =
|
||||
withContext(Dispatchers.IO) {
|
||||
suspendCancellableCoroutine<Boolean> { cont ->
|
||||
try {
|
||||
io.github.kdroidfilter.nucleus.notification.NotificationCenter
|
||||
.requestAuthorization(
|
||||
options =
|
||||
setOf(
|
||||
io.github.kdroidfilter.nucleus.notification.AuthorizationOption.ALERT,
|
||||
io.github.kdroidfilter.nucleus.notification.AuthorizationOption.SOUND,
|
||||
io.github.kdroidfilter.nucleus.notification.AuthorizationOption.BADGE,
|
||||
),
|
||||
callback = { granted, _ -> cont.resume(granted) },
|
||||
)
|
||||
} catch (t: Throwable) {
|
||||
cont.resume(false)
|
||||
// Hard timeout: recent macOS versions present the permission
|
||||
// request as an auto-dismissing banner; if the user misses it
|
||||
// the completion handler may never fire, which would
|
||||
// otherwise leave this coroutine (and the settings UI's
|
||||
// "Requesting…" spinner) suspended forever.
|
||||
withTimeoutOrNull(PERMISSION_REQUEST_TIMEOUT_MS) {
|
||||
suspendCancellableCoroutine<Pair<Boolean, String?>> { cont ->
|
||||
try {
|
||||
io.github.kdroidfilter.nucleus.notification.NotificationCenter
|
||||
.requestAuthorization(
|
||||
options =
|
||||
setOf(
|
||||
io.github.kdroidfilter.nucleus.notification.AuthorizationOption.ALERT,
|
||||
io.github.kdroidfilter.nucleus.notification.AuthorizationOption.SOUND,
|
||||
io.github.kdroidfilter.nucleus.notification.AuthorizationOption.BADGE,
|
||||
),
|
||||
callback = { granted, error -> cont.resume(granted to error) },
|
||||
)
|
||||
} catch (t: Throwable) {
|
||||
cont.resume(false to (t.message ?: t::class.simpleName))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (outcome == null) {
|
||||
_lastRequestError.value = TIMEOUT_ERROR_MESSAGE
|
||||
// Leave _permission at its current value: the OS never answered,
|
||||
// so we genuinely don't know the state. A follow-up
|
||||
// refreshPermission() (window focus / settings re-entry) will
|
||||
// reconcile it if the user did grant via the banner.
|
||||
return _permission.value
|
||||
}
|
||||
val (granted, osError) = outcome
|
||||
_lastRequestError.value = if (granted) null else osError
|
||||
_permission.value =
|
||||
if (granted) PermissionState.Granted else PermissionState.Denied
|
||||
return _permission.value
|
||||
@@ -202,7 +245,7 @@ class NucleusNotificationDispatcher(
|
||||
}
|
||||
}
|
||||
|
||||
private fun sendMac(spec: NotificationSpec): SendResult {
|
||||
private suspend fun sendMac(spec: NotificationSpec): SendResult {
|
||||
val content =
|
||||
io.github.kdroidfilter.nucleus.notification.NotificationContent(
|
||||
title = spec.title,
|
||||
@@ -220,9 +263,30 @@ class NucleusNotificationDispatcher(
|
||||
identifier = UUID.randomUUID().toString(),
|
||||
content = content,
|
||||
)
|
||||
io.github.kdroidfilter.nucleus.notification.NotificationCenter
|
||||
.add(request)
|
||||
return SendResult.Delivered
|
||||
// Nucleus delivers add() asynchronously; the optional callback carries
|
||||
// the OS error string (null on success). Without it, a rejected
|
||||
// notification (e.g. unauthorized at send time) is silently swallowed
|
||||
// and the caller logs a phantom "Delivered". The Result wrapper
|
||||
// disambiguates "callback fired with null error" from "callback never
|
||||
// fired" (timeout — treat as delivered, since UNUserNotificationCenter
|
||||
// still posts the request in that case on some macOS versions).
|
||||
val addOutcome =
|
||||
withTimeoutOrNull(SEND_ACK_TIMEOUT_MS) {
|
||||
suspendCancellableCoroutine<Result<String?>> { cont ->
|
||||
try {
|
||||
io.github.kdroidfilter.nucleus.notification.NotificationCenter
|
||||
.add(request) { error -> cont.resume(Result.success(error)) }
|
||||
} catch (t: Throwable) {
|
||||
cont.resume(Result.success(t.message ?: t::class.simpleName))
|
||||
}
|
||||
}
|
||||
}
|
||||
val addError = addOutcome?.getOrNull()
|
||||
return when {
|
||||
addOutcome == null -> SendResult.Delivered // no ack from OS; request was queued
|
||||
addError.isNullOrBlank() -> SendResult.Delivered
|
||||
else -> SendResult.Failed(IllegalStateException("UNUserNotificationCenter.add failed: $addError"))
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- Windows ----------
|
||||
|
||||
+52
@@ -0,0 +1,52 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.commons.moderation.notifications
|
||||
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertNull
|
||||
|
||||
/**
|
||||
* Pins the [NotificationDispatcher.lastRequestError] contract: implementations
|
||||
* that never surface OS-level request errors must default to a null-valued
|
||||
* flow so the settings UI can rely on the property existing on every
|
||||
* dispatcher (avoids `is` checks against NucleusNotificationDispatcher).
|
||||
*/
|
||||
class NotificationDispatcherLastRequestErrorTest {
|
||||
private class MinimalDispatcher : NotificationDispatcher {
|
||||
override val permission: StateFlow<PermissionState> = MutableStateFlow(PermissionState.NotApplicable)
|
||||
override val nativeAvailable: StateFlow<Boolean> = MutableStateFlow(false)
|
||||
|
||||
override suspend fun requestPermission(): PermissionState = PermissionState.NotApplicable
|
||||
|
||||
override suspend fun refreshPermission(): PermissionState = PermissionState.NotApplicable
|
||||
|
||||
override suspend fun send(spec: NotificationSpec): SendResult = SendResult.Suppressed("test")
|
||||
|
||||
override fun release() {}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `default lastRequestError is null-valued`() {
|
||||
assertNull(MinimalDispatcher().lastRequestError.value)
|
||||
}
|
||||
}
|
||||
+36
-2
@@ -120,6 +120,8 @@ fun NotificationSettingsScreen(onBack: (() -> Unit)? = null) {
|
||||
var testStatus by remember { mutableStateOf<String?>(null) }
|
||||
var requestingPermission by remember { mutableStateOf(false) }
|
||||
var sendingTest by remember { mutableStateOf(false) }
|
||||
val lastRequestError by dispatcher?.lastRequestError?.collectAsState()
|
||||
?: remember { mutableStateOf<String?>(null) }
|
||||
|
||||
// Re-sync permission state whenever this screen enters composition
|
||||
// and whenever the window regains focus — user may have toggled
|
||||
@@ -219,9 +221,11 @@ fun NotificationSettingsScreen(onBack: (() -> Unit)? = null) {
|
||||
testStatus =
|
||||
when (newState) {
|
||||
PermissionState.Granted -> "Permission granted. Notifications are on — try the test toast below."
|
||||
PermissionState.Denied -> "Permission denied. Enable in System Settings if you change your mind."
|
||||
PermissionState.Denied ->
|
||||
dispatcher?.lastRequestError?.value
|
||||
?: "Permission denied. Enable in System Settings if you change your mind."
|
||||
PermissionState.BundleRequired -> "Notifications need a bundled app — run `./gradlew :desktopApp:runDistributable`."
|
||||
else -> null
|
||||
else -> dispatcher?.lastRequestError?.value
|
||||
}
|
||||
}
|
||||
},
|
||||
@@ -310,6 +314,36 @@ fun NotificationSettingsScreen(onBack: (() -> Unit)? = null) {
|
||||
}
|
||||
},
|
||||
) { Text("Open System Settings") }
|
||||
// When the OS refused the request outright (no prompt
|
||||
// ever shown — e.g. UNErrorDomain "Notifications are
|
||||
// not allowed for this application"), Amethyst never
|
||||
// gets a System Settings entry, so the deep link above
|
||||
// is a dead end. Offer a retry alongside it: on recent
|
||||
// macOS the permission prompt is an auto-dismissing
|
||||
// banner, and a fresh request re-surfaces it.
|
||||
OutlinedButton(
|
||||
onClick = {
|
||||
if (requestingPermission) return@OutlinedButton
|
||||
coroutineScope.launch {
|
||||
requestingPermission = true
|
||||
testStatus = "Waiting for OS prompt…"
|
||||
val newState =
|
||||
try {
|
||||
dispatcher?.requestPermission() ?: PermissionState.Denied
|
||||
} finally {
|
||||
requestingPermission = false
|
||||
}
|
||||
testStatus =
|
||||
when (newState) {
|
||||
PermissionState.Granted -> "Permission granted. Try the test toast below."
|
||||
else ->
|
||||
dispatcher?.lastRequestError?.value
|
||||
?: "Permission denied. Enable in System Settings if you change your mind."
|
||||
}
|
||||
}
|
||||
},
|
||||
enabled = dispatcher != null && !requestingPermission,
|
||||
) { Text(if (requestingPermission) "Requesting…" else "Ask again") }
|
||||
Text(
|
||||
"Enable in System Settings → Notifications → Amethyst",
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
|
||||
Reference in New Issue
Block a user