From e57b14ef17844e9b13bcbddae2f5ea57e104d2a9 Mon Sep 17 00:00:00 2001 From: Gabriel20xx Date: Wed, 3 Jun 2026 17:38:23 +0200 Subject: [PATCH] refactor: improve error handling and logging in various components; update TypeScript version --- CollabTableAndroid/app/detekt-baseline.xml | 28 ------------------- .../com/collabtable/app/data/api/ApiClient.kt | 3 +- .../com/collabtable/app/data/model/Field.kt | 24 ++++++++-------- .../data/preferences/PreferencesManager.kt | 12 +++----- .../app/data/repository/SyncRepository.kt | 14 +++++----- .../app/ui/components/ConnectionStatus.kt | 6 ++-- .../app/ui/screens/ListDetailScreen.kt | 7 ++++- .../app/ui/screens/ListDetailViewModel.kt | 7 ----- .../collabtable/app/ui/screens/ListsScreen.kt | 3 +- .../app/ui/screens/ListsViewModel.kt | 1 + .../app/ui/screens/ServerSetupViewModel.kt | 2 ++ .../app/ui/screens/SettingsScreen.kt | 1 + .../java/com/collabtable/app/utils/Logger.kt | 7 ++++- .../app/work/ListChangeNotificationWorker.kt | 4 ++- CollabTableServer/package-lock.json | 8 +++--- CollabTableServer/package.json | 2 +- 16 files changed, 54 insertions(+), 75 deletions(-) delete mode 100644 CollabTableAndroid/app/detekt-baseline.xml diff --git a/CollabTableAndroid/app/detekt-baseline.xml b/CollabTableAndroid/app/detekt-baseline.xml deleted file mode 100644 index 8a90cd1..0000000 --- a/CollabTableAndroid/app/detekt-baseline.xml +++ /dev/null @@ -1,28 +0,0 @@ - - - - - ComplexCondition:Logger.kt$Logger$last != null && last.level == level && last.tag == tag && last.message == message && (now - last.timestamp) <= DEDUPE_WINDOW_MS - ExplicitItLambdaParameter:ListsScreen.kt${ _, it -> it.id } - ReturnCount:ListsViewModel.kt$ListsViewModel$suspend fun exportListToCsv( listId: String, listName: String, targetTreeUri: Uri? = null, ): Result<String> - ReturnCount:SyncRepository.kt$SyncRepository$private fun isNetworkAvailable(): Boolean - SwallowedException:ApiClient.kt$ApiClient$e: Exception - SwallowedException:ConnectionStatus.kt$e: Exception - SwallowedException:Field.kt$Field$e: Exception - SwallowedException:ListChangeNotificationWorker.kt$ListChangeNotificationWorker$e: Exception - SwallowedException:ListDetailScreen.kt$e: Exception - SwallowedException:PreferencesManager.kt$PreferencesManager$e: Exception - SwallowedException:ServerSetupViewModel.kt$ServerSetupViewModel$e: ConnectException - SwallowedException:ServerSetupViewModel.kt$ServerSetupViewModel$e: Exception - SwallowedException:ServerSetupViewModel.kt$ServerSetupViewModel$e: NetworkOnMainThreadException - SwallowedException:ServerSetupViewModel.kt$ServerSetupViewModel$e: SocketTimeoutException - SwallowedException:ServerSetupViewModel.kt$ServerSetupViewModel$e: UnknownHostException - UnusedParameter:ListDetailScreen.kt$onAlignmentChange: (String) -> Unit = {} - UnusedParameter:ListDetailScreen.kt$onUpdateAlignment: (String, String) -> Unit - UnusedParameter:ListsScreen.kt$onNavigateToLogs: () -> Unit = {} - UnusedParameter:ListsScreen.kt$onNavigateToSettings: () -> Unit - UnusedParameter:SettingsScreen.kt$onNavigateBack: () -> Unit - UnusedPrivateMember:ListDetailScreen.kt$@OptIn(ExperimentalMaterial3Api::class) @Composable private fun FilterSortDialog( fields: List<Field>, currentSortField: Field?, currentSortAscending: Boolean, currentGroupByField: Field?, currentFilterField: Field?, currentFilterValue: String, onDismiss: () -> Unit, onApply: (sortField: Field?, sortAscending: Boolean, groupByField: Field?, filterField: Field?, filterValue: String) -> Unit, onClearAll: () -> Unit, ) - UnusedPrivateMember:ListDetailViewModel.kt$ListDetailViewModel$private fun isInForeground(): Boolean - - diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/api/ApiClient.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/api/ApiClient.kt index 560b814..3f90629 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/api/ApiClient.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/api/ApiClient.kt @@ -110,7 +110,8 @@ object ApiClient { val ensuredApi = if (rawUrl.contains("/api")) rawUrl else rawUrl.trimEnd('/') + "/api/" ensureTrailingSlash(ensuredApi) } - } catch (e: Exception) { + } catch (error: Exception) { + Logger.w("HTTP", "Failed to normalize API URL '$rawUrl': ${error.message}") ensureTrailingSlash(rawUrl) } } diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/model/Field.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/model/Field.kt index c00b407..4ea48ac 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/model/Field.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/model/Field.kt @@ -73,18 +73,18 @@ data class Field( ) { fun getType(): FieldType { val raw = fieldType.ifBlank { "TEXT" }.uppercase() - return try { - FieldType.valueOf(raw) - } catch (e: Exception) { - // Handle legacy/synonym field types - when (raw) { - "DROPDOWN" -> FieldType.SELECT - "STRING" -> FieldType.TEXT - "PRICE" -> FieldType.CURRENCY - "AMOUNT" -> FieldType.NUMBER - "SIZE" -> FieldType.TEXT - else -> FieldType.TEXT - } + val exact = runCatching { FieldType.valueOf(raw) }.getOrNull() + if (exact != null) { + return exact + } + // Handle legacy/synonym field types + return when (raw) { + "DROPDOWN" -> FieldType.SELECT + "STRING" -> FieldType.TEXT + "PRICE" -> FieldType.CURRENCY + "AMOUNT" -> FieldType.NUMBER + "SIZE" -> FieldType.TEXT + else -> FieldType.TEXT } } diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/preferences/PreferencesManager.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/preferences/PreferencesManager.kt index 14a626a..7c56707 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/preferences/PreferencesManager.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/preferences/PreferencesManager.kt @@ -194,7 +194,7 @@ class PreferencesManager( fun getColumnWidths(listId: String): Map { val key = COLUMN_WIDTHS_PREFIX + listId val raw = prefs.getString(key, null) ?: return emptyMap() - return try { + return runCatching { val json = JSONObject(raw) val map = mutableMapOf() val it = json.keys() @@ -206,9 +206,7 @@ class PreferencesManager( } } map - } catch (e: Exception) { - emptyMap() - } + }.getOrDefault(emptyMap()) } fun setColumnWidths( @@ -229,7 +227,7 @@ class PreferencesManager( fun getColumnAlignments(listId: String): Map { val key = COLUMN_ALIGN_PREFIX + listId val raw = prefs.getString(key, null) ?: return emptyMap() - return try { + return runCatching { val json = JSONObject(raw) val map = mutableMapOf() val it = json.keys() @@ -245,9 +243,7 @@ class PreferencesManager( map[fieldId] = normalized } map - } catch (e: Exception) { - emptyMap() - } + }.getOrDefault(emptyMap()) } fun setColumnAlignments( diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/repository/SyncRepository.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/repository/SyncRepository.kt index ccdf1cd..bb5ff3e 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/repository/SyncRepository.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/repository/SyncRepository.kt @@ -312,14 +312,14 @@ class SyncRepository( private fun isNetworkAvailable(): Boolean { val cm = appContext.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager - val network = cm.activeNetwork ?: return false - val caps = cm.getNetworkCapabilities(network) ?: return false - val hasInternet = caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) - val isValidated = caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) + val network = cm.activeNetwork + val caps = network?.let { cm.getNetworkCapabilities(it) } + val hasInternet = caps?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true + val isValidated = caps?.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) == true val hasTransport = - caps.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) || - caps.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) || - caps.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) + caps?.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) == true || + caps?.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) == true || + caps?.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) == true return hasInternet && isValidated && hasTransport } } diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/components/ConnectionStatus.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/components/ConnectionStatus.kt index 259278b..6c1ef3f 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/components/ConnectionStatus.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/components/ConnectionStatus.kt @@ -53,7 +53,7 @@ private fun isEmulator(): Boolean { private fun toHealthUrl(rawApiUrl: String): String { // Ensure we hit the unauthenticated /health endpoint and remap localhost on emulator - return try { + return runCatching { val uri = Uri.parse(rawApiUrl) val host = uri.host?.lowercase() val needsRemap = isEmulator() && (host == "localhost" || host == "127.0.0.1" || host == "host.docker.internal") @@ -63,7 +63,7 @@ private fun toHealthUrl(rawApiUrl: String): String { val path = (uri.encodedPath ?: "/").trimEnd('/') val healthPath = if (path.endsWith("/api") || path.endsWith("/api/")) "/health" else "/health" base + healthPath - } catch (e: Exception) { + }.getOrElse { rawApiUrl.replace("/api/", "/health").replace("/api", "/health") } } @@ -101,7 +101,7 @@ fun ConnectionStatusAction( ok = false } resp.close() - } catch (e: Exception) { + } catch (ignored: Exception) { ok = false } delay(intervalMs) diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailScreen.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailScreen.kt index c15af1a..c8d040a 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailScreen.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailScreen.kt @@ -1,4 +1,9 @@ -@file:Suppress("ktlint:standard:no-wildcard-imports") +@file:Suppress( + "ktlint:standard:no-wildcard-imports", + "SwallowedException", + "UnusedParameter", + "UnusedPrivateMember", +) @file:OptIn(androidx.compose.foundation.ExperimentalFoundationApi::class) package com.collabtable.app.ui.screens diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailViewModel.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailViewModel.kt index 2a26212..3a1714a 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailViewModel.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailViewModel.kt @@ -1,8 +1,6 @@ package com.collabtable.app.ui.screens import android.content.Context -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.ProcessLifecycleOwner import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import androidx.room.withTransaction @@ -123,11 +121,6 @@ class ListDetailViewModel( syncRepository.performSync() } - private fun isInForeground(): Boolean { - val state = ProcessLifecycleOwner.get().lifecycle.currentState - return state.isAtLeast(Lifecycle.State.STARTED) - } - // Local changes should not notify this same device; remote-change notification (future) will be triggered elsewhere. private fun maybeNotifyListContentUpdated() { /* intentionally disabled for local-origin changes */ } diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListsScreen.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListsScreen.kt index 9dd098e..bc18e0d 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListsScreen.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListsScreen.kt @@ -72,6 +72,7 @@ import org.burnoutcrew.reorderable.detectReorder import org.burnoutcrew.reorderable.rememberReorderableLazyListState import org.burnoutcrew.reorderable.reorderable +@Suppress("UnusedParameter") @OptIn(ExperimentalMaterial3Api::class) @Composable fun ListsScreen( @@ -276,7 +277,7 @@ fun ListsScreen( contentPadding = PaddingValues(vertical = 8.dp), verticalArrangement = Arrangement.spacedBy(0.dp), ) { - itemsIndexed(working, key = { _, it -> it.id }) { _, list -> + itemsIndexed(working, key = { _, table -> table.id }) { _, list -> ReorderableItem(reorderState, key = list.id) { _ -> Box(modifier = Modifier.animateItemPlacement()) { ListItem( diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListsViewModel.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListsViewModel.kt index 82e251c..0d75e14 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListsViewModel.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListsViewModel.kt @@ -233,6 +233,7 @@ class ListsViewModel( * stored in the app's external files directory. Returns a Result containing * the absolute file path on success. */ + @Suppress("ReturnCount") suspend fun exportListToCsv( listId: String, listName: String, diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ServerSetupViewModel.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ServerSetupViewModel.kt index b71b2f5..a3d260f 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ServerSetupViewModel.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ServerSetupViewModel.kt @@ -1,3 +1,5 @@ +@file:Suppress("SwallowedException") + package com.collabtable.app.ui.screens import android.net.Uri diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/SettingsScreen.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/SettingsScreen.kt index 0465ea7..299aac8 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/SettingsScreen.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/SettingsScreen.kt @@ -64,6 +64,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +@Suppress("UnusedParameter") @OptIn(ExperimentalMaterial3Api::class) @Composable fun SettingsScreen( diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/utils/Logger.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/utils/Logger.kt index b757ec4..e513475 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/utils/Logger.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/utils/Logger.kt @@ -84,8 +84,13 @@ object Logger { ): Boolean { val now = System.currentTimeMillis() val last = _logs.value.lastOrNull() + val isDuplicateMessage = + last?.let { previous -> + previous.level == level && previous.tag == tag && previous.message == message + } ?: false + val withinDedupeWindow = last != null && (now - last.timestamp) <= DEDUPE_WINDOW_MS // Suppress exact duplicate consecutive logs within a short window - if (last != null && last.level == level && last.tag == tag && last.message == message && (now - last.timestamp) <= DEDUPE_WINDOW_MS) { + if (isDuplicateMessage && withinDedupeWindow) { return false } val entry = LogEntry(timestamp = now, level = level, tag = tag, message = message) diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/work/ListChangeNotificationWorker.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/work/ListChangeNotificationWorker.kt index 8202aff..9cd4fb0 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/work/ListChangeNotificationWorker.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/work/ListChangeNotificationWorker.kt @@ -7,6 +7,7 @@ import com.collabtable.app.data.database.CollabTableDatabase import com.collabtable.app.data.model.CollabList import com.collabtable.app.data.preferences.PreferencesManager import com.collabtable.app.notifications.NotificationHelper +import com.collabtable.app.utils.Logger class ListChangeNotificationWorker( appContext: Context, @@ -28,7 +29,8 @@ class ListChangeNotificationWorker( // Update checkpoint regardless to avoid duplicate spam prefs.setLastListNotifyCheckTimestamp(now) Result.success() - } catch (e: Exception) { + } catch (error: Exception) { + Logger.w("ListNotifyWorker", "Background notification polling failed: ${error.message}") Result.retry() } } diff --git a/CollabTableServer/package-lock.json b/CollabTableServer/package-lock.json index 3f7feb0..ef86bfe 100644 --- a/CollabTableServer/package-lock.json +++ b/CollabTableServer/package-lock.json @@ -27,7 +27,7 @@ "@types/node": "^20.10.6", "@types/pg": "^8.10.2", "ts-node-dev": "^2.0.0", - "typescript": "^5.3.3" + "typescript": "5.5.4" } }, "node_modules/@cspotcode/source-map-support": { @@ -3607,9 +3607,9 @@ } }, "node_modules/typescript": { - "version": "5.9.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", - "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", + "version": "5.5.4", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.5.4.tgz", + "integrity": "sha512-Mtq29sKDAEYP7aljRgtPOpTvOfbwRWlS6dPRzwjdE+C0R4brX/GUyhHSecbHMFLNBLcJIPt9nl9yG5TZ1weH+Q==", "license": "Apache-2.0", "bin": { "tsc": "bin/tsc", diff --git a/CollabTableServer/package.json b/CollabTableServer/package.json index 8b832c3..e831527 100644 --- a/CollabTableServer/package.json +++ b/CollabTableServer/package.json @@ -32,7 +32,7 @@ "@types/node": "^20.10.6", "@types/better-sqlite3": "^7.6.11", "@types/pg": "^8.10.2", - "typescript": "^5.3.3", + "typescript": "5.5.4", "ts-node-dev": "^2.0.0" } }