From 7e78846c0e566163db054288d08a0979e8eacc69 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 20:11:19 +0000 Subject: [PATCH] Record crashes where they can be read, and stop the HA sync causing them The app is reportedly crashing repeatedly. That matters beyond the crash itself: repeated crashes get an app force-stopped, and a force-stop cancels every AlarmManager entry it owned - which is exactly the failure being chased, an alarm that silently stops existing overnight. Makes crashes visible. This build is sideloaded, so nothing collects crashes and reading logcat needs a computer and a cable, leaving a background crash completely invisible from the phone. CrashRecorder stores the last stack trace and Settings shows it with a copy button. The handler chains to the previous one, so the system still behaves exactly as before. Fixes a path that could produce one. HaApiClient.pushSync built its Request outside the try, and Request.Builder.url throws IllegalArgumentException on a malformed base URL - which the user types by hand - while the catch covered only IOException. Anything else escaped into HaSyncService's lifecycleScope, which has no exception handler, killing the process. That loop re-runs on every state change and every service start, so it would crash again and again. The request is now built inside the try, non-IO exceptions are caught and logged, the collect body is guarded, and HaSyncPayload no longer calls nextTriggerAtMillis on rows that would throw. Whether this is the crash being seen is unconfirmed - that is what the recorder is for. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01C7x8v5c5Lu29PBWy1LdfCe --- .../haalarmclock/HaAlarmClockApp.kt | 5 ++ .../haalarmclock/ha/HaApiClient.kt | 26 +++++-- .../haalarmclock/ha/HaSyncPayload.kt | 9 ++- .../haalarmclock/ha/HaSyncService.kt | 13 +++- .../ui/settings/SettingsScreen.kt | 45 ++++++++++++ .../haalarmclock/util/CrashRecorder.kt | 72 +++++++++++++++++++ 6 files changed, 160 insertions(+), 10 deletions(-) create mode 100644 app/src/main/java/com/targetcrafter/haalarmclock/util/CrashRecorder.kt diff --git a/app/src/main/java/com/targetcrafter/haalarmclock/HaAlarmClockApp.kt b/app/src/main/java/com/targetcrafter/haalarmclock/HaAlarmClockApp.kt index 9f75c74..f4aa749 100644 --- a/app/src/main/java/com/targetcrafter/haalarmclock/HaAlarmClockApp.kt +++ b/app/src/main/java/com/targetcrafter/haalarmclock/HaAlarmClockApp.kt @@ -25,6 +25,7 @@ import com.targetcrafter.haalarmclock.ha.HaSettingsStore import com.targetcrafter.haalarmclock.ha.HaWebSocketClient import com.targetcrafter.haalarmclock.timer.TimerNotifications import com.targetcrafter.haalarmclock.timer.TimerScheduler +import com.targetcrafter.haalarmclock.util.CrashRecorder import com.targetcrafter.haalarmclock.widget.ClockWidgetUpdater import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope @@ -53,6 +54,7 @@ class HaAlarmClockApp : Application() { val worldClockStore: WorldClockStore by lazy { WorldClockStore(this) } val tabPreferencesStore: TabPreferencesStore by lazy { TabPreferencesStore(this) } val alarmScheduleAudit: AlarmScheduleAudit by lazy { AlarmScheduleAudit(this) } + val crashRecorder: CrashRecorder by lazy { CrashRecorder(this) } val timerScheduler: TimerScheduler by lazy { TimerScheduler(this) } private val timerNotifications: TimerNotifications by lazy { TimerNotifications(this) } @@ -88,6 +90,9 @@ class HaAlarmClockApp : Application() { override fun onCreate() { super.onCreate() + // First, before anything that could itself crash — a crash during startup is precisely the + // kind this exists to catch, and it can only record what happens after it's installed. + crashRecorder.install() createNotificationChannels() // Re-arm everything on every process start, not just after a reboot (BootReceiver). An // AlarmManager entry is dropped when an app is force-stopped — which is exactly what OEM diff --git a/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaApiClient.kt b/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaApiClient.kt index d5d03ec..5f48866 100644 --- a/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaApiClient.kt +++ b/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaApiClient.kt @@ -1,5 +1,6 @@ package com.targetcrafter.haalarmclock.ha +import android.util.Log import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import kotlinx.serialization.json.JsonObject @@ -9,21 +10,34 @@ import okhttp3.Request import okhttp3.RequestBody.Companion.toRequestBody import java.io.IOException +private const val TAG = "HaApiClient" private val JSON_MEDIA_TYPE = "application/json; charset=utf-8".toMediaType() /** Pushes alarm/ringing state to the HA Alarm Clock custom integration's REST endpoint. */ class HaApiClient(private val httpClient: OkHttpClient) { + /** Returns whether the push succeeded. Never throws: a failed sync is not worth taking the app + * down for, and it used to be able to. Building the request was outside the try — and + * `Request.Builder.url` throws [IllegalArgumentException] on a malformed base URL, which the + * user types by hand — while the catch only covered [IOException], so anything else OkHttp + * raised escaped into the caller's coroutine, which has no exception handler. That crashes the + * process, and this runs again on every state change and every service start, so it would + * crash repeatedly. Repeated crashes get an app force-stopped, and a force-stop cancels every + * alarm it had scheduled. */ suspend fun pushSync(baseUrl: String, accessToken: String, payload: JsonObject): Boolean = withContext(Dispatchers.IO) { - val request = Request.Builder() - .url("${normalizeBaseUrl(baseUrl)}/api/ha_alarmclock/sync") - .addHeader("Authorization", "Bearer $accessToken") - .post(payload.toString().toRequestBody(JSON_MEDIA_TYPE)) - .build() try { + val request = Request.Builder() + .url("${normalizeBaseUrl(baseUrl)}/api/ha_alarmclock/sync") + .addHeader("Authorization", "Bearer $accessToken") + .post(payload.toString().toRequestBody(JSON_MEDIA_TYPE)) + .build() httpClient.newCall(request).execute().use { it.isSuccessful } - } catch (_: IOException) { + } catch (e: IOException) { + Log.w(TAG, "Sync push failed", e) + false + } catch (e: Exception) { + Log.e(TAG, "Sync push failed unexpectedly — check the Home Assistant URL", e) false } } diff --git a/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaSyncPayload.kt b/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaSyncPayload.kt index cf0575a..446610d 100644 --- a/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaSyncPayload.kt +++ b/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaSyncPayload.kt @@ -38,13 +38,18 @@ object HaSyncPayload { put("time", "%02d:%02d".format(alarm.hour, alarm.minute)) put("enabled", alarm.enabled) put("repeat", daysMaskLabel(alarm.repeatDaysMask)) - if (alarm.enabled) put("next_trigger", isoInstant(alarm.nextTriggerAtMillis())) + // hasValidTime guards nextTriggerAtMillis, which throws on an out-of-range row. + // This runs inside the HA sync loop, so one bad row must not be able to take + // the push — or the process — down with it. + if (alarm.enabled && alarm.hasValidTime) { + put("next_trigger", isoInstant(alarm.nextTriggerAtMillis())) + } alarm.snoozedUntilMillis?.let { put("snoozed_until", isoInstant(it)) } } } } - val next = alarms.filter { it.enabled }.minByOrNull { it.nextTriggerAtMillis() } + val next = alarms.filter { it.enabled && it.hasValidTime }.minByOrNull { it.nextTriggerAtMillis() } putJsonObject("next_alarm") { if (next != null) { put("alarm_id", next.id) diff --git a/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaSyncService.kt b/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaSyncService.kt index f3d40ab..e04139f 100644 --- a/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaSyncService.kt +++ b/app/src/main/java/com/targetcrafter/haalarmclock/ha/HaSyncService.kt @@ -74,8 +74,17 @@ class HaSyncService : LifecycleService() { ) { settings, alarms, ringing, timers -> SyncInputs(settings, alarms, ringing, timers) } .collect { inputs -> if (inputs.settings.isConfigured) { - val payload = HaSyncPayload.build(app.deviceId, app.deviceName, inputs.alarms, inputs.ringing, inputs.timers) - app.haApiClient.pushSync(inputs.settings.baseUrl, inputs.settings.accessToken, payload) + // Nothing about pushing state to Home Assistant is worth crashing over. + // This scope has no exception handler, so anything thrown here would kill + // the process — and since this re-runs on every state change and every + // service start, it would do so repeatedly. Repeated crashes get the app + // force-stopped, which cancels every alarm it had scheduled. + try { + val payload = HaSyncPayload.build(app.deviceId, app.deviceName, inputs.alarms, inputs.ringing, inputs.timers) + app.haApiClient.pushSync(inputs.settings.baseUrl, inputs.settings.accessToken, payload) + } catch (e: Exception) { + Log.e(TAG, "Failed to build or push the sync payload", e) + } } } } diff --git a/app/src/main/java/com/targetcrafter/haalarmclock/ui/settings/SettingsScreen.kt b/app/src/main/java/com/targetcrafter/haalarmclock/ui/settings/SettingsScreen.kt index 1ed7119..b3ad58c 100644 --- a/app/src/main/java/com/targetcrafter/haalarmclock/ui/settings/SettingsScreen.kt +++ b/app/src/main/java/com/targetcrafter/haalarmclock/ui/settings/SettingsScreen.kt @@ -1,5 +1,7 @@ package com.targetcrafter.haalarmclock.ui.settings +import android.content.ClipData +import android.content.ClipboardManager import android.content.Intent import android.media.MediaPlayer import android.net.Uri @@ -67,6 +69,7 @@ import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.input.PasswordVisualTransformation import androidx.compose.ui.text.input.VisualTransformation import androidx.compose.ui.unit.dp +import androidx.core.content.getSystemService import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleEventObserver import androidx.lifecycle.viewmodel.compose.viewModel @@ -422,9 +425,51 @@ private fun ReliabilitySection() { } ScheduleHealthRow() ScheduleDropHistoryRow() + LastCrashRow() AlarmSoundTestRow() } +/** + * Shows the last crash, if there was one. Sideloaded builds have no Play Console collecting these + * and reading logcat needs a computer, so without this a background crash is invisible from the + * phone — while still being able to get the app force-stopped, which cancels its alarms. + */ +@Composable +private fun LastCrashRow() { + val context = LocalContext.current + val app = HaAlarmClockApp.from(context) + val crash by app.crashRecorder.lastCrash.collectAsState() + var expanded by remember { mutableStateOf(false) } + + val recorded = crash ?: return + val formatter = remember { DateTimeFormatter.ofPattern("d MMM, HH:mm:ss") } + val at = remember(recorded.atMillis) { + Instant.ofEpochMilli(recorded.atMillis).atZone(ZoneId.systemDefault()).format(formatter) + } + + ReliabilityWarningRow( + title = "The app crashed on $at", + description = if (expanded) { + "On thread \"${recorded.threadName}\".\n\n${recorded.stackTrace}" + } else { + "Repeated crashes get an app force-stopped, which cancels its alarms. Tap to see the " + + "stack trace, then copy it into a bug report." + }, + buttonLabel = if (expanded) "Copy" else "Show", + onClick = { + if (expanded) { + context.getSystemService()?.setPrimaryClip( + ClipData.newPlainText("HA Alarm Clock crash", recorded.stackTrace), + ) + app.crashRecorder.clear() + expanded = false + } else { + expanded = true + } + }, + ) +} + /** * Shows how often Android has been caught having dropped the alarm schedule. Without this the * self-repair on app start would hide its own reason for existing: the schedule is put back before diff --git a/app/src/main/java/com/targetcrafter/haalarmclock/util/CrashRecorder.kt b/app/src/main/java/com/targetcrafter/haalarmclock/util/CrashRecorder.kt new file mode 100644 index 0000000..50c2a86 --- /dev/null +++ b/app/src/main/java/com/targetcrafter/haalarmclock/util/CrashRecorder.kt @@ -0,0 +1,72 @@ +package com.targetcrafter.haalarmclock.util + +import android.content.Context +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import java.io.PrintWriter +import java.io.StringWriter + +/** The last crash the app recorded, or null if it hasn't crashed since this was last cleared. */ +data class RecordedCrash(val atMillis: Long, val threadName: String, val stackTrace: String) + +/** + * Keeps the stack trace of the last crash where the user can actually read it. + * + * This app is sideloaded, so there is no Play Console collecting crashes, and reading logcat needs + * a computer and a cable — which means a background crash is completely invisible from the phone. + * That matters more here than in most apps: repeated crashes get an app force-stopped, and a + * force-stop cancels every alarm it had scheduled. A crash nobody can see is therefore a plausible + * cause of an alarm that silently never rings, and "the app crashed" alone doesn't say where. + * + * The handler chains to whatever was installed before it, so the system still shows its dialog and + * kills the process exactly as it would have. + */ +class CrashRecorder(context: Context) { + + private val prefs = context.getSharedPreferences("crash_recorder", Context.MODE_PRIVATE) + + private val _lastCrash = MutableStateFlow(read()) + val lastCrash: StateFlow = _lastCrash.asStateFlow() + + fun install() { + val previous = Thread.getDefaultUncaughtExceptionHandler() + Thread.setDefaultUncaughtExceptionHandler { thread, throwable -> + // Never let recording a crash become a second crash that hides the first. + runCatching { record(thread, throwable) } + previous?.uncaughtException(thread, throwable) + } + } + + private fun record(thread: Thread, throwable: Throwable) { + val trace = StringWriter().also { throwable.printStackTrace(PrintWriter(it)) }.toString() + // commit(), not apply(): the process is about to be killed, and an async write would be + // lost — which would leave exactly the invisible crash this exists to prevent. + prefs.edit() + .putLong(KEY_AT, System.currentTimeMillis()) + .putString(KEY_THREAD, thread.name) + .putString(KEY_TRACE, trace.take(MAX_TRACE_CHARS)) + .commit() + } + + fun clear() { + prefs.edit().clear().apply() + _lastCrash.value = null + } + + private fun read(): RecordedCrash? { + val at = prefs.getLong(KEY_AT, 0L) + val trace = prefs.getString(KEY_TRACE, null) ?: return null + if (at <= 0L) return null + return RecordedCrash(at, prefs.getString(KEY_THREAD, "?").orEmpty(), trace) + } + + companion object { + /** Enough for the frames that matter; a full trace with dozens of "caused by" chains would + * be unreadable on a phone and is not worth the preference bloat. */ + private const val MAX_TRACE_CHARS = 8_000 + private const val KEY_AT = "crashed_at" + private const val KEY_THREAD = "thread" + private const val KEY_TRACE = "stack_trace" + } +}