Readable sync errors and a save reminder after Test Connection

README.md

@@ -49,7 +49,7 @@ Settings also lets you set default tags and choose whether bookmarks are marked
 
 ShareDing connects to linkding and bookmarked pages directly. It has no proxy setting; if your server is reachable only through another route, use a VPN on your device. Versions before 0.2.1 had an optional HTTP proxy. It was removed because proxy credentials were sent without TLS, and updating deletes any saved proxy settings.
 
-Settings shows an inline Test Connection result. The API token is encrypted using a key held in Android Keystore. The [privacy policy](docs/privacy.md) explains how the app handles bookmarks and the token.
+Settings shows an inline Test Connection result. The test uses the values typed in the fields and does not save them; tap **Save settings** to sync with a connection you have just tested. The API token is encrypted using a key held in Android Keystore. The [privacy policy](docs/privacy.md) explains how the app handles bookmarks and the token.
 
 ShareDing is written in Kotlin. Jetpack Compose provides the interface, Room stores the queue, and WorkManager handles background sync.
 

app/src/androidTest/java/org/evsyukov/shareding/QueueUiTest.kt

@@ -354,6 +354,42 @@ class QueueUiTest {
         compose.onNodeWithText("Sync problem alerts").assertDoesNotExist()
     }
 
+    @Test fun testedButUnsavedTokenAsksToSaveSettings() {
+        val tls = AndroidTestTls(androidx.test.platform.app.InstrumentationRegistry.getInstrumentation().context)
+        MockWebServer().apply { tls.start(this) }.use { server ->
+            server.dispatcher = object : Dispatcher() {
+                override fun dispatch(request: RecordedRequest) =
+                    if (request.getHeader("Authorization") == "Token restored") MockResponse().setBody("{}")
+                    else MockResponse().setResponseCode(401).setBody("""{"detail":"Invalid token."}""")
+            }
+            val app = compose.activity.application as ShareDingApplication
+            app.container.settings.save(tls.url(server), "revoked", "", true, false)
+            val reminder = "Save settings to sync with this connection."
+            compose.onNodeWithText("Settings").performClick()
+            compose.onNodeWithText("Test Connection").performClick()
+            compose.waitUntil(5_000) {
+                compose.onAllNodes(hasText("Connection failed: linkding rejected the API token (HTTP 401)\nInvalid token."))
+                    .fetchSemanticsNodes().isNotEmpty()
+            }
+            compose.onNodeWithText(reminder).assertDoesNotExist()
+
+            // Test Connection checks the typed token; sync would still use the saved one.
+            compose.onNodeWithText("API token (leave empty to keep current)").performTextInput("restored")
+            compose.onNodeWithText("Test Connection").performClick()
+            compose.waitUntil(5_000) {
+                compose.onAllNodes(hasText("Connection successful")).fetchSemanticsNodes().isNotEmpty()
+            }
+            compose.onNodeWithText(reminder).assertIsDisplayed()
+            assertEquals("revoked", app.container.settings.token())
+
+            compose.onNodeWithText("Save settings").performSemanticsAction(SemanticsActions.OnClick)
+            compose.waitUntil(5_000) { app.container.settings.token() == "restored" }
+            compose.waitUntil(5_000) {
+                compose.onAllNodes(hasText(reminder)).fetchSemanticsNodes().isEmpty()
+            }
+        }
+    }
+
     @Test fun connectionFailureRemainsVisibleInSettings() {
         compose.onNodeWithText("Settings").performClick()
         compose.onNodeWithText("Server URL").performTextClearance()

app/src/androidTest/java/org/evsyukov/shareding/SyncNotifierTest.kt

@@ -54,39 +54,39 @@ class SyncNotifierTest {
 
     @Test fun problemIsShownOncePerKindAndClearedOnSuccess() {
         val notifier = SyncNotifier(context) { now }
-        notifier.problem(SyncProblem.AUTH, "linkding returned HTTP 401")
+        notifier.problem(SyncProblem.AUTH)
         val first = active(SyncNotifier.PROBLEM_ID)
         assertNotNull(first)
         assertEquals(SyncProblem.AUTH.title, first!!.notification.extras.getString("android.title"))
         assertEquals(SyncNotifier.CHANNEL_ID, first.notification.channelId)
 
         manager.cancel(SyncNotifier.PROBLEM_ID)
         waitGone(SyncNotifier.PROBLEM_ID)
-        notifier.problem(SyncProblem.AUTH, "linkding returned HTTP 401")
+        notifier.problem(SyncProblem.AUTH)
         Thread.sleep(300)
         assertNull("Same problem must not be shown again", manager.activeNotifications
             .firstOrNull { it.id == SyncNotifier.PROBLEM_ID })
 
-        notifier.problem(SyncProblem.NOT_FOUND, "linkding returned HTTP 404")
+        notifier.problem(SyncProblem.NOT_FOUND)
         assertEquals(SyncProblem.NOT_FOUND.title,
             active(SyncNotifier.PROBLEM_ID)!!.notification.extras.getString("android.title"))
 
         notifier.resolved()
         waitGone(SyncNotifier.PROBLEM_ID)
         assertNull(manager.activeNotifications.firstOrNull { it.id == SyncNotifier.PROBLEM_ID })
-        notifier.problem(SyncProblem.NOT_FOUND, "again")
+        notifier.problem(SyncProblem.NOT_FOUND)
         assertNotNull("A resolved problem may be shown again", active(SyncNotifier.PROBLEM_ID))
     }
 
     @Test fun reminderReturnsAfterTheUserDismissedIt() {
         val notifier = SyncNotifier(context) { now }
-        notifier.remind(SyncProblem.NOT_CONFIGURED, SyncProblem.NOT_CONFIGURED_DETAIL)
+        notifier.remind(SyncProblem.NOT_CONFIGURED)
         assertEquals(SyncProblem.NOT_CONFIGURED.title,
             active(SyncNotifier.PROBLEM_ID)?.notification?.extras?.getString("android.title"))
         manager.cancel(SyncNotifier.PROBLEM_ID)
         waitGone(SyncNotifier.PROBLEM_ID)
 
-        notifier.remind(SyncProblem.NOT_CONFIGURED, SyncProblem.NOT_CONFIGURED_DETAIL)
+        notifier.remind(SyncProblem.NOT_CONFIGURED)
         assertNotNull("Every save while sync cannot run reminds again", active(SyncNotifier.PROBLEM_ID))
     }
 
@@ -122,10 +122,15 @@ class SyncNotifierTest {
             try {
                 app.container.settings.save(tls.url(server), "revoked", "", true, false)
                 dao.insert(Bookmark(url = "https://example.com/notify"))
-                server.enqueue(MockResponse().setResponseCode(401).setBody("Invalid token."))
+                server.enqueue(MockResponse().setResponseCode(401).setBody("""{"detail":"Invalid token."}"""))
                 TestListenableWorkerBuilder<SyncWorker>(context).build().doWork()
-                assertEquals(SyncProblem.AUTH.title,
-                    active(SyncNotifier.PROBLEM_ID)?.notification?.extras?.getString("android.title"))
+                val shown = active(SyncNotifier.PROBLEM_ID)?.notification?.extras
+                assertEquals(SyncProblem.AUTH.title, shown?.getString("android.title"))
+                // The text tells the user what to do; the server response stays on the queue card.
+                assertEquals("Saved links stay in the queue. Check the API token in Settings.",
+                    shown?.getCharSequence("android.text")?.toString())
+                assertEquals("linkding rejected the API token (HTTP 401)\nInvalid token.",
+                    dao.findByUrl("https://example.com/notify")?.lastError)
 
                 // A rejected token stops sync; the next attempt needs Sync now, Retry, or new settings.
                 app.container.settings.resumeSync()

app/src/main/java/org/evsyukov/shareding/MainActivity.kt

@@ -615,6 +615,8 @@ internal fun TagInput(value: String, onValueChange: (String) -> Unit, label: Str
     }
 }
 
+private const val CONNECTION_OK = "Connection successful"
+
 @Composable
 private fun SettingsScreen(settings: Settings, padding: PaddingValues,
                            availableTags: List<String>, tagLoadError: Boolean,
@@ -685,7 +687,7 @@ private fun SettingsScreen(settings: Settings, padding: PaddingValues,
                             }
                             if (testVersion == version) {
                                 testFailed = result.isFailure
-                                testStatus = if (result.isSuccess) "Connection successful"
+                                testStatus = if (result.isSuccess) CONNECTION_OK
                                     else "Connection failed: ${result.exceptionOrNull()?.message ?: "Unknown error"}"
                                 testing = false
                             }
@@ -697,6 +699,12 @@ private fun SettingsScreen(settings: Settings, padding: PaddingValues,
                         else MaterialTheme.colorScheme.primary,
                         style = MaterialTheme.typography.bodyMedium)
                 }
+                // The test used the values typed here; sync keeps using the saved ones until Save.
+                if (testStatus == CONNECTION_OK && !savedConnectionIsCurrent) {
+                    Text("Save settings to sync with this connection.",
+                        style = MaterialTheme.typography.bodySmall,
+                        color = MaterialTheme.colorScheme.onSurfaceVariant)
+                }
             }
         }
         item {

app/src/main/java/org/evsyukov/shareding/ShareDingApplication.kt

@@ -78,14 +78,8 @@ class AppContainer(application: Application) {
 
     /** After a link is saved: schedules sync, or tells the user why the link will not be sent. */
     suspend fun linkSaved(): SyncProblem? {
-        val current = settings.state.value
-        val blocked = SyncProblem.blocking(current)
-        if (blocked == null) {
-            scheduler.requestSync()
-        } else {
-            notifier.remind(blocked, if (blocked == SyncProblem.NOT_CONFIGURED)
-                SyncProblem.NOT_CONFIGURED_DETAIL else current.lastError)
-        }
+        val blocked = SyncProblem.blocking(settings.state.value)
+        if (blocked == null) scheduler.requestSync() else notifier.remind(blocked)
         return blocked
     }
 

app/src/main/java/org/evsyukov/shareding/network/LinkdingApi.kt

@@ -2,6 +2,7 @@ package org.evsyukov.shareding.network
 
 import android.net.Network
 import com.google.gson.JsonArray
+import com.google.gson.JsonElement
 import com.google.gson.JsonObject
 import com.google.gson.JsonParser
 import java.net.URI
@@ -86,6 +87,40 @@ object TagNames {
 
 class ApiException(val code: Int, message: String) : Exception(message)
 
+/** Turns an HTTP error from linkding into text a person can read. */
+object ApiErrors {
+    fun message(code: Int, body: String): String {
+        val reason = when (code) {
+            401, 403 -> "linkding rejected the API token (HTTP $code)"
+            404 -> "linkding API not found (HTTP 404)"
+            else -> "linkding returned HTTP $code"
+        }
+        val detail = readable(body.trim())
+        return if (detail.isEmpty()) reason else "$reason\n$detail"
+    }
+
+    /** A JSON body becomes plain lines; anything else is kept as the server sent it. */
+    private fun readable(body: String): String {
+        if (!body.startsWith("{") && !body.startsWith("[")) return body
+        val json = runCatching { JsonParser.parseString(body) }.getOrNull() ?: return body
+        val lines = when {
+            json.isJsonObject -> json.asJsonObject.entrySet().map { (name, value) ->
+                // linkding puts a general message under these names; other names are form fields.
+                if (name == "detail" || name == "non_field_errors") text(value) else "$name: ${text(value)}"
+            }
+            json.isJsonArray -> json.asJsonArray.map(::text)
+            else -> return body
+        }
+        return lines.filter { it.isNotBlank() }.joinToString("\n").ifEmpty { body }
+    }
+
+    private fun text(value: JsonElement): String = when {
+        value.isJsonPrimitive -> value.asString
+        value.isJsonArray -> value.asJsonArray.joinToString(" ", transform = ::text)
+        else -> value.toString()
+    }
+}
+
 class LinkdingApi(
     private val clientFactory: (Network?, Long, Long) -> OkHttpClient = HttpClients::create,
 ) {
@@ -148,9 +183,7 @@ class LinkdingApi(
         return clientFactory(network, 10, 15).newCall(request).executeCancellable { response ->
             val code = response.code
             if (code !in 200..299) {
-                val reason = "linkding returned HTTP $code"
-                val detail = response.body?.string().orEmpty().trim()
-                throw ApiException(code, if (detail.isEmpty()) reason else "$reason\n$detail")
+                throw ApiException(code, ApiErrors.message(code, response.body?.string().orEmpty()))
             }
             if (readBody) response.body?.string() else null
         }

app/src/main/java/org/evsyukov/shareding/sync/SyncNotifier.kt

@@ -21,17 +21,17 @@ import org.evsyukov.shareding.network.Urls
  * Reasons saved links cannot be sent: errors that another retry cannot fix without a change in
  * settings or on the server, and a server that has not been set up yet.
  */
-enum class SyncProblem(val title: String) {
-    AUTH("linkding rejected the API token"),
-    NOT_FOUND("linkding API not found"),
-    HTTPS("linkding server URL must use HTTPS"),
-    TOKEN("Cannot read the API token"),
-    NOT_CONFIGURED("linkding is not set up");
+enum class SyncProblem(val title: String, private val fix: String) {
+    AUTH("linkding rejected the API token", "Check the API token in Settings."),
+    NOT_FOUND("linkding API not found", "Check the server URL in Settings."),
+    HTTPS("linkding server URL must use HTTPS", "Change the server URL in Settings."),
+    TOKEN("Cannot read the API token", "Enter the API token again in Settings."),
+    NOT_CONFIGURED("linkding is not set up", "Add the server URL and API token in Settings.");
 
-    companion object {
-        const val NOT_CONFIGURED_DETAIL =
-            "Saved links stay in the queue. Add the server URL and API token in Settings."
+    /** Notification text: what happens to saved links and what the user can do. */
+    val detail: String get() = "Saved links stay in the queue. $fix"
 
+    companion object {
         /** Why saved links cannot be sent right now, or null while sync can run. */
         fun blocking(settings: AppSettings): SyncProblem? = when {
             !settings.configured -> NOT_CONFIGURED
@@ -71,14 +71,14 @@ class SyncNotifier(private val context: Context, private val now: () -> Long = S
     private val prefs = context.getSharedPreferences("notifications", Context.MODE_PRIVATE)
     private val manager = NotificationManagerCompat.from(context)
 
-    fun problem(problem: SyncProblem, detail: String) {
+    fun problem(problem: SyncProblem) {
         if (prefs.getString(KEY_PROBLEM, null) == problem.name) return
-        if (post(PROBLEM_ID, problem.title, detail)) prefs.edit().putString(KEY_PROBLEM, problem.name).apply()
+        if (post(PROBLEM_ID, problem.title, problem.detail)) prefs.edit().putString(KEY_PROBLEM, problem.name).apply()
     }
 
     /** Shown on every save while sync cannot run, even after the user dismissed it. */
-    fun remind(problem: SyncProblem, detail: String) {
-        if (post(PROBLEM_ID, problem.title, detail)) prefs.edit().putString(KEY_PROBLEM, problem.name).apply()
+    fun remind(problem: SyncProblem) {
+        if (post(PROBLEM_ID, problem.title, problem.detail)) prefs.edit().putString(KEY_PROBLEM, problem.name).apply()
     }
 
     fun resolved() {

app/src/main/java/org/evsyukov/shareding/sync/SyncWorker.kt

@@ -104,7 +104,7 @@ class SyncWorker(context: Context, params: WorkerParameters) : CoroutineWorker(c
                              notify: Boolean = true): Result {
         container.settings.stopSync(problem.name, message)
         if (notify) {
-            container.notifier.problem(problem, message)
+            container.notifier.problem(problem)
             container.notifier.checkStale(container.db.bookmarks().oldestCreatedAt())
         }
         container.updateRecovery()

app/src/test/java/org/evsyukov/shareding/network/ApiErrorsTest.kt

@@ -0,0 +1,29 @@
+package org.evsyukov.shareding.network
+
+import org.junit.Assert.assertEquals
+import org.junit.Test
+
+class ApiErrorsTest {
+    @Test fun namesRejectedTokenAndMissingApi() {
+        assertEquals("linkding rejected the API token (HTTP 401)\nInvalid token.",
+            ApiErrors.message(401, """{"detail":"Invalid token."}"""))
+        assertEquals("linkding rejected the API token (HTTP 403)", ApiErrors.message(403, ""))
+        assertEquals("linkding API not found (HTTP 404)", ApiErrors.message(404, "  "))
+    }
+
+    @Test fun showsFieldErrorsAsText() {
+        assertEquals("linkding returned HTTP 400\nurl: Enter a valid URL.\ntag_names: Too long. Not a list.",
+            ApiErrors.message(400, """{"url":["Enter a valid URL."],"tag_names":["Too long.","Not a list."]}"""))
+        assertEquals("linkding returned HTTP 400\nThe bookmark already exists.",
+            ApiErrors.message(400, """{"non_field_errors":["The bookmark already exists."]}"""))
+        assertEquals("linkding returned HTTP 400\nFirst problem.\nSecond problem.",
+            ApiErrors.message(400, """["First problem.","Second problem."]"""))
+    }
+
+    @Test fun keepsBodiesThatAreNotJson() {
+        assertEquals("linkding returned HTTP 502\nupstream failed", ApiErrors.message(502, "upstream failed\n"))
+        assertEquals("linkding returned HTTP 500\n{broken", ApiErrors.message(500, "{broken"))
+        assertEquals("linkding returned HTTP 500\n{}", ApiErrors.message(500, "{}"))
+        assertEquals("linkding returned HTTP 503", ApiErrors.message(503, ""))
+    }
+}

app/src/test/java/org/evsyukov/shareding/network/LinkdingApiTest.kt

@@ -87,8 +87,7 @@ class LinkdingApiTest {
                 fail("HTTP error must fail")
             } catch (error: ApiException) {
                 assertEquals(503, error.code)
-                assertTrue(error.message.orEmpty().contains("linkding returned HTTP 503"))
-                assertTrue(error.message.orEmpty().contains("""{"detail":"temporarily unavailable"}"""))
+                assertEquals("linkding returned HTTP 503\ntemporarily unavailable", error.message)
             }
         }
     }