diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt index 27ccf61..848530d 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt @@ -1033,6 +1033,32 @@ class Session( * order to tidy up a label, and the label is not the point — the message * is. A contact who reinstalls in a month gets it in a month. */ + /** + * Whether waiting will ever resolve this, mirroring iOS `E2EError.waitingCanFix`. + * + * ⚠️ Matched on the refusal types the trust layer throws, not on message + * text. A string match here would break the first time somebody reworded + * an error, and would break silently — the queue would simply go back to + * retrying for ever. + */ + private fun permanentRefusal(e: Throwable?): Boolean = when (e) { + is io.flamenet.client.IdentityChangedException, + is io.flamenet.client.KeyNotLoggedException, + is io.flamenet.client.KeyLogForkException -> true + else -> false + } + + /** A short, contentless reason for the reading and the bubble. */ + private fun refusalReason(e: Throwable?): String = when (e) { + is io.flamenet.client.IdentityChangedException -> + "their security keys changed" + is io.flamenet.client.KeyNotLoggedException -> + "their device offered a key the server has no record of" + is io.flamenet.client.KeyLogForkException -> + "this server's record of published keys does not add up" + else -> "the message could not be sent" + } + fun flushOutbox(): Set { val pending = vault.outbox() if (pending.length() == 0) return emptySet() @@ -1059,7 +1085,12 @@ class Session( continue } - val sent = runCatching { conversations.send(to, text) }.getOrNull() + // ⚠️ The failure is CAUGHT, not discarded. `getOrNull()` threw the + // exception away, so this loop could not tell a contact who is + // merely offline from one whose keys changed — and retried both + // for ever. + val attempt = runCatching { conversations.send(to, text) } + val sent = attempt.getOrNull() if (sent?.reachedSomeone == true) { if (groupThread == null) { restate(Vault.threadForUser(to), at, Delivery.DELIVERED) @@ -1067,6 +1098,21 @@ class Session( } else { groupDelivered += groupThread to at } + } else if (permanentRefusal(attempt.exceptionOrNull())) { + // 🔴 **Some refusals waiting cannot fix** (ios#22). A send + // blocked because the contact's identity keys changed is + // refused identically on every retry until a person accepts + // them, while the bubble goes on promising "will send". + // + // ⚠️ Everything else still retries for ever, deliberately: a + // contact who is offline, or who has not registered a device + // yet, gets the message whenever they return. Giving up there + // would delete something somebody wrote to tidy up a label. + val why = refusalReason(attempt.exceptionOrNull()) + io.flamenet.client.DeliveryDiagnostics.sendFailed(why, to) + val thread = groupThread ?: Vault.threadForUser(to) + restate(thread, at, Delivery.FAILED) + touched += thread } else { stillWaiting.put(item) } diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/OutboxTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/OutboxTest.kt index b723e36..a8ff795 100644 --- a/app/src/test/kotlin/io/flamenet/messenger/app/OutboxTest.kt +++ b/app/src/test/kotlin/io/flamenet/messenger/app/OutboxTest.kt @@ -153,6 +153,67 @@ class OutboxTest { "a transport failure must not eat the message either") } + /** + * 🔴 **A refusal waiting cannot fix must STOP** (ios#22). + * + * The outbox retries for ever, which is right for a contact who is offline + * or has no device yet. It is wrong for a send the trust layer refuses: + * the peer's identity keys changed, so every retry is refused identically + * until a person accepts them — while the bubble goes on promising + * "will send", which is a promise the app cannot keep. + * + * ⚠️ Asserted on the OUTBOX and the BUBBLE together. A version that + * dropped the entry but left the bubble pending would pass a test that + * only checked one, and would read as "still sending" for ever. + */ + @Test + fun `a send refused for changed keys stops retrying and says why`() { + val session = session() + server.enqueue(json("""{"devices":[]}""")) + session.send(peer, "you there?") + assertEquals(1, session.vault.outbox().length()) + + // They come back with a REAL device, and the message goes — pinning + // their identity on first use. + val them = DeviceIdentity.create() + val bundleOf = { id: DeviceIdentity -> + val opk = id.generateOneTimePreKeys(1).first() + id.registrationWire().put( + "opk", org.json.JSONObject() + .put("id", opk.id) + .put("pub", android.util.Base64.encodeToString( + opk.publicKey, android.util.Base64.NO_WRAP))) + } + val first = bundleOf(them) + server.enqueue(json(org.json.JSONObject() + .put("devices", org.json.JSONArray().put(first)).toString())) + server.enqueue(json(org.json.JSONObject() + .put("devices", org.json.JSONArray().put(first)).toString())) + server.enqueue(json("""{"ok":true}""")) + session.flushOutbox() + assertEquals(0, session.vault.outbox().length()) + + // 🔴 Now they reinstall: a DIFFERENT identity under the same user. The + // trust layer refuses, and no amount of retrying changes that. + server.enqueue(json("""{"devices":[]}""")) // queues the second message + session.send(peer, "you there?") + assertEquals(1, session.vault.outbox().length()) + + val second = bundleOf(DeviceIdentity.create()) + val newDirectory = org.json.JSONObject() + .put("devices", org.json.JSONArray().put(second)).toString() + // Enqueued generously: the refusal happens at the directory, but the + // exact number of calls before it is not what this test is about. + repeat(4) { server.enqueue(json(newDirectory)) } + session.flushOutbox() + + assertEquals(0, session.vault.outbox().length(), + "a permanently refused send must not sit in the queue for ever") + val line = session.history(Vault.threadForUser(peer)).last { it.mine } + assertEquals(Session.Delivery.FAILED, line.delivery, + "the bubble must stop promising it will send") + } + /** * 🔴 **The promise, end to end.** Everything above only proves the message * is not lost; this proves it is eventually sent, which is the part the diff --git a/client/src/main/kotlin/io/flamenet/client/DeliveryDiagnostics.kt b/client/src/main/kotlin/io/flamenet/client/DeliveryDiagnostics.kt new file mode 100644 index 0000000..59c9a44 --- /dev/null +++ b/client/src/main/kotlin/io/flamenet/client/DeliveryDiagnostics.kt @@ -0,0 +1,100 @@ +package io.flamenet.client + +/** + * What the client already knows about delivery, kept where a person can read + * it (ios#21, ios#23 — the Android half). + * + * 🔴 **The client has always known this; it only ever told the log.** Every + * refusal went to `System.err` and nowhere else, so a send blocked by the + * identity guard reached the person sending it as "Something went wrong + * reaching that server" — which was neither true nor actionable. + * + * ⚠️ **Shape only, never content.** Counts, reasons and timestamps. No message + * text, no key material, no payloads, and no identifiers — see + * [normalisedReason]. A diagnostic reading that leaks what it is diagnosing is + * worse than none, and this one is meant to be safe to screenshot. + * + * ⚠️ In memory, deliberately. It describes the session you are in; persisting + * it would be a growing record of who you talked to and when, which is the + * metadata this project spends its effort not keeping. + */ +object DeliveryDiagnostics { + + data class Peer( + var envelopesSeen: Int = 0, + var delivered: Int = 0, + /** Reason → how many times. */ + val drops: MutableMap = mutableMapOf(), + var repairs: Int = 0, + var lastRepairAt: Long = 0L, + var lastSendFailure: String? = null, + var lastSendFailureAt: Long = 0L, + ) { + val dropTotal: Int get() = drops.values.sum() + + /** + * 🔴 A client that repeatedly repairs is a symptom, not a cure. One is + * ordinary — a contact reinstalled. Three against the same peer means + * something is not settling, and that is worth saying out loud. + */ + val repairsLookAbnormal: Boolean get() = repairs >= 3 + } + + private val peers = mutableMapOf() + + @Synchronized fun reading(userId: Long): Peer = peers[userId] ?: Peer() + + @Synchronized fun all(): Map = peers.toMap() + + @Synchronized fun sawEnvelope(from: Long) { + peers.getOrPut(from) { Peer() }.envelopesSeen++ + } + + @Synchronized fun delivered(from: Long) { + peers.getOrPut(from) { Peer() }.delivered++ + } + + @Synchronized fun dropped(reason: String, from: Long) { + val p = peers.getOrPut(from) { Peer() } + val slug = normalisedReason(reason) + p.drops[slug] = (p.drops[slug] ?: 0) + 1 + } + + @Synchronized fun repaired(userId: Long, at: Long = System.currentTimeMillis()) { + val p = peers.getOrPut(userId) { Peer() } + p.repairs++ + p.lastRepairAt = at + } + + @Synchronized fun sendFailed(reason: String, to: Long, + at: Long = System.currentTimeMillis()) { + val p = peers.getOrPut(to) { Peer() } + p.lastSendFailure = reason + p.lastSendFailureAt = at + } + + @Synchronized fun clearSendFailure(to: Long) { + peers[to]?.let { it.lastSendFailure = null; it.lastSendFailureAt = 0L } + } + + /** Signing out ends the session this describes. */ + @Synchronized fun reset() = peers.clear() + + /** + * Strip anything identifying before a reason is counted. + * + * ⚠️ Refusal messages name the thing that was wrong, which sometimes means + * naming a device id — "device … has published keys, but none match". + * A log may carry that; a screen the user is invited to screenshot may not. + * Runs of digits and hex become "…". + * + * ⚠️ Short hex ids are the ones that slip through: `9f0e` is neither a long + * hex run nor an all-digit run. Hence "four or more hex characters + * containing at least one digit", which catches truncated device ids + * without eating ordinary words like `base64`. + */ + fun normalisedReason(reason: String): String = + reason + .replace(Regex("""\b(?=[0-9a-fA-F]*[0-9])[0-9a-fA-F]{4,}\b"""), "…") + .replace(Regex("""\b\d+\b"""), "…") +} diff --git a/client/src/test/kotlin/io/flamenet/client/DeliveryDiagnosticsTest.kt b/client/src/test/kotlin/io/flamenet/client/DeliveryDiagnosticsTest.kt new file mode 100644 index 0000000..357d5bd --- /dev/null +++ b/client/src/test/kotlin/io/flamenet/client/DeliveryDiagnosticsTest.kt @@ -0,0 +1,79 @@ +package io.flamenet.client + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** What the app is allowed to say about why delivery failed (ios#21, #22, #23). */ +class DeliveryDiagnosticsTest { + + @Test + fun `identifiers are stripped from reasons before they are counted`() { + // 🔴 A reading meant to be safe to screenshot must not carry device ids. + // These are the real shapes: the refusal messages name what was wrong, + // and that sometimes means naming an id. + val cases = listOf( + "device ce0c0fb4 has published keys, but none match what this relay served", + "device claims 41 but is bound to 12", + "we do not hold signed prekey 7", + "device claims 3f8a9c2e11bd but is bound to 9f0e", + ) + for (raw in cases) { + val out = DeliveryDiagnostics.normalisedReason(raw) + assertFalse(out.any { it.isDigit() }, "'$out' still carries a digit from '$raw'") + } + } + + @Test + fun `a reason with nothing identifying survives intact`() { + // ⚠️ It has to stay readable, or the screen says nothing. "base64" + // contains a digit and must NOT be mangled. + val plain = "the payload was not valid base64" + assertEquals(plain, DeliveryDiagnostics.normalisedReason(plain)) + } + + @Test + fun `one repair is ordinary, repeated repairs are a symptom`() { + DeliveryDiagnostics.reset() + DeliveryDiagnostics.repaired(7) + assertFalse(DeliveryDiagnostics.reading(7).repairsLookAbnormal) + DeliveryDiagnostics.repaired(7) + DeliveryDiagnostics.repaired(7) + assertTrue(DeliveryDiagnostics.reading(7).repairsLookAbnormal) + } + + @Test + fun `a reading counts per conversation, not globally`() { + DeliveryDiagnostics.reset() + DeliveryDiagnostics.dropped("the payload was not valid base64", 1) + DeliveryDiagnostics.dropped("the payload was not valid base64", 1) + DeliveryDiagnostics.dropped("the envelope did not decode", 2) + assertEquals(2, DeliveryDiagnostics.reading(1).dropTotal) + assertEquals(1, DeliveryDiagnostics.reading(2).dropTotal) + assertEquals(0, DeliveryDiagnostics.reading(99).dropTotal) + } + + @Test + fun `the same reason twice is one row, not two`() { + // ⚠️ Normalisation is what makes this true: without it, two refusals + // naming different device ids would be two rows and the map would grow + // without bound. + DeliveryDiagnostics.reset() + DeliveryDiagnostics.dropped("device aaaa1111 offered a key the log has no record of", 5) + DeliveryDiagnostics.dropped("device bbbb2222 offered a key the log has no record of", 5) + assertEquals(1, DeliveryDiagnostics.reading(5).drops.size) + assertEquals(2, DeliveryDiagnostics.reading(5).dropTotal) + } + + @Test + fun `signing out ends the reading with the session`() { + DeliveryDiagnostics.reset() + DeliveryDiagnostics.sawEnvelope(4) + DeliveryDiagnostics.sendFailed("their security keys changed", 4) + DeliveryDiagnostics.reset() + assertEquals(0, DeliveryDiagnostics.reading(4).envelopesSeen) + assertNull(DeliveryDiagnostics.reading(4).lastSendFailure) + } +}