A message nobody can receive must not report success
Reported: a message sent from Android while the other end was signed out on
iOS was lost, and a group text vanished the same way.
Envelopes are addressed per device, and signing out deregisters the device.
sendPlaintext looped over an empty bundle list, made no request at all, and
returned Sent(delivered=[], failed={}) - which every caller reads as "it
went". The message was written into the sender's own thread and existed
nowhere else. Group sends call this once per member, so the same path lost
group text.
iOS never had this: transmit throws on an empty device list. The platforms
disagreed about a delivery guarantee for as long as both have existed, and
nothing caught it, because the tests that covered sending had both ends
signed in - and a happy-path test cannot tell a delivery from a no-op, since
both are silent.
Recorded in the failure map rather than thrown, so a group send still reaches
the members who ARE reachable. The banner now says the message was not queued
and has to be sent again, instead of "Delivered to 0 of 1 devices".
SendToNobodyTest pins it; verified failing with the guard removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
0983e760b4
commit
80b903e168
@@ -885,9 +885,7 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
it.copy(
|
||||
busy = false,
|
||||
openChat = it.openChat?.copy(lines = current.history(chat.threadId)),
|
||||
error = if (sent.failed.isEmpty()) null
|
||||
else "Delivered to ${sent.delivered.size} of " +
|
||||
"${sent.delivered.size + sent.failed.size} devices.",
|
||||
error = describeDelivery(sent),
|
||||
)
|
||||
}
|
||||
}.onFailure { failure -> update { it.copy(busy = false, error = describe(failure)) } }
|
||||
@@ -1197,14 +1195,30 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
it.copy(
|
||||
busy = false,
|
||||
openChat = it.openChat?.copy(lines = current.history(chat.threadId)),
|
||||
error = if (sent.failed.isEmpty()) null
|
||||
else "Delivered to ${sent.delivered.size} of " +
|
||||
"${sent.delivered.size + sent.failed.size} devices.",
|
||||
error = describeDelivery(sent),
|
||||
)
|
||||
}
|
||||
}.onFailure { failure -> update { it.copy(busy = false, error = describe(failure)) } }
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* What to say about a send.
|
||||
*
|
||||
* ⚠️ "Delivered to 0 of 1 devices" is a ratio pretending to be an
|
||||
* explanation. The case that actually happens — the other person is signed
|
||||
* out, so they have no devices at all — deserves the sentence, because the
|
||||
* message is not queued anywhere and will not arrive later.
|
||||
*/
|
||||
private fun describeDelivery(sent: io.flamenet.client.Conversations.Sent): String? = when {
|
||||
sent.failed.isEmpty() -> null
|
||||
sent.delivered.isEmpty() ->
|
||||
"Not delivered — they are not signed in on any device. " +
|
||||
"This message was not queued; send it again once they are back."
|
||||
else -> "Delivered to ${sent.delivered.size} of " +
|
||||
"${sent.delivered.size + sent.failed.size} devices."
|
||||
}
|
||||
|
||||
/**
|
||||
* Bytes for a received attachment, fetched and cached on first use.
|
||||
*
|
||||
@@ -1266,9 +1280,7 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
// Partial delivery is stated, not hidden. "Sent" when one
|
||||
// of someone's three devices got it is a lie the user acts
|
||||
// on.
|
||||
error = if (sent.failed.isEmpty()) null
|
||||
else "Delivered to ${sent.delivered.size} of " +
|
||||
"${sent.delivered.size + sent.failed.size} devices.",
|
||||
error = describeDelivery(sent),
|
||||
)
|
||||
}
|
||||
}.onFailure { failure ->
|
||||
|
||||
@@ -370,6 +370,26 @@ class Conversations(
|
||||
val failed = LinkedHashMap<String, String>()
|
||||
val outgoing = mutableListOf<RelayClient.ToDevice>()
|
||||
|
||||
// 🔴 **Nobody to send to is a FAILURE, and it used to report success.**
|
||||
// A message is addressed per device. With no devices the loop below ran
|
||||
// zero times, `outgoing` stayed empty, no request was made — and `Sent`
|
||||
// came back with an empty `failed`, which every caller reads as "it
|
||||
// went". The message was written into our own thread and simply never
|
||||
// existed anywhere else.
|
||||
//
|
||||
// That is the whole bug behind "I wasn't signed in on the other end and
|
||||
// the message got lost": signing out deregisters the device, so the
|
||||
// recipient genuinely has none, and this is the one path that has to
|
||||
// notice. iOS already threw here; this side never did.
|
||||
//
|
||||
// ⚠️ Recorded in `failed` rather than thrown, because the group senders
|
||||
// above call this once per member with no try around it — throwing
|
||||
// would abandon everyone after the first person who is signed out.
|
||||
if (bundles.none { it.deviceId != identity.deviceId }) {
|
||||
failed["user:$toUserId"] = "not signed in on any device"
|
||||
return Sent(delivered, failed, sealed = false)
|
||||
}
|
||||
|
||||
// Seal only if we hold their key. A silent fall back to an
|
||||
// unsealed send is the failure §14.5 warns about, so the outcome is
|
||||
// returned rather than assumed — the UI can then say which it was
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
package io.flamenet.client
|
||||
|
||||
import io.flamenet.e2e.DeviceIdentity
|
||||
import io.flamenet.e2e.Vault
|
||||
import okhttp3.mockwebserver.MockResponse
|
||||
import okhttp3.mockwebserver.MockWebServer
|
||||
import javax.crypto.KeyGenerator
|
||||
import javax.crypto.SecretKey
|
||||
import kotlin.test.AfterTest
|
||||
import kotlin.test.BeforeTest
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* A message nobody can receive must not report success.
|
||||
*
|
||||
* 🔴 **This shipped, and it lost real messages.** Signing out deregisters the
|
||||
* device, so a signed-out recipient genuinely has no devices. Envelopes are
|
||||
* addressed per device, so `sendPlaintext` looped over an empty bundle list,
|
||||
* made no request at all, and returned `Sent(delivered = [], failed = {})`.
|
||||
* Every caller reads an empty `failed` as "it went" — the message was written
|
||||
* into the sender's own thread and existed nowhere else. It was reported as
|
||||
* "I wasn't signed in on the other end and the message got lost", and the same
|
||||
* path is why a group text vanished: the group senders call this once per
|
||||
* member.
|
||||
*
|
||||
* ⚠️ **iOS never had this bug** — `transmit` throws on an empty device list.
|
||||
* The two platforms disagreed about a delivery guarantee for as long as both
|
||||
* have existed, and nothing failed, because the only tests that covered
|
||||
* sending had both ends signed in. That is the actual lesson here: a test that
|
||||
* only exercises the happy path cannot tell a delivery from a no-op, since
|
||||
* both of them are silent.
|
||||
*
|
||||
* So this asserts the *unhappy* path directly, on the seam where the two
|
||||
* platforms drifted.
|
||||
*/
|
||||
class SendToNobodyTest {
|
||||
|
||||
private lateinit var server: MockWebServer
|
||||
|
||||
@BeforeTest fun start() { server = MockWebServer().also { it.start() } }
|
||||
@AfterTest fun stop() { server.shutdown() }
|
||||
|
||||
private class MapStorage : Vault.Storage {
|
||||
private val records = mutableMapOf<String, ByteArray>()
|
||||
override fun read(name: String) = records[name]
|
||||
override fun write(name: String, bytes: ByteArray) { records[name] = bytes }
|
||||
override fun delete(name: String) { records.remove(name) }
|
||||
override fun names() = records.keys.toList()
|
||||
}
|
||||
|
||||
private fun key(): SecretKey =
|
||||
KeyGenerator.getInstance("AES").apply { init(256) }.generateKey()
|
||||
|
||||
private fun conversations(): Conversations {
|
||||
val identity = DeviceIdentity.create()
|
||||
val vault = Vault(MapStorage(), key()).also { it.saveIdentity(identity) }
|
||||
val relay = RelayClient(server.url("/").toString(), "alice", "app pass word")
|
||||
return Conversations(identity, vault, relay)
|
||||
}
|
||||
|
||||
private fun json(body: String) = MockResponse()
|
||||
.setResponseCode(200)
|
||||
.setHeader("Content-Type", "application/json")
|
||||
.setBody(body)
|
||||
|
||||
@Test
|
||||
fun `sending to somebody with no registered devices is reported as failed`() {
|
||||
// What the relay answers for a user who is signed out everywhere.
|
||||
server.enqueue(json("""{"devices":[]}"""))
|
||||
|
||||
val sent = conversations().send(toUserId = 99, text = "are you there?")
|
||||
|
||||
assertTrue(sent.delivered.isEmpty(), "nothing can have been delivered")
|
||||
assertFalse(
|
||||
sent.failed.isEmpty(),
|
||||
"a send that reached nobody reported an empty `failed`, which every " +
|
||||
"caller reads as success — this is the message-loss bug",
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* ⚠️ And it must not quietly post the envelope anyway. The bug was not only
|
||||
* in the return value: with no devices there is nothing to address, so a
|
||||
* request here would mean the relay was being handed an envelope for a
|
||||
* device that does not exist.
|
||||
*/
|
||||
@Test
|
||||
fun `no envelope is posted when there is nobody to address`() {
|
||||
server.enqueue(json("""{"devices":[]}"""))
|
||||
|
||||
conversations().send(toUserId = 99, text = "are you there?")
|
||||
|
||||
// Exactly one request: the device lookup. Nothing after it.
|
||||
assertEquals(1, server.requestCount,
|
||||
"a send to nobody should stop after the lookup")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user