extant2000/flamenet-messenger-android
1
0
Fork 0
Code Issues 7 Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'fix/calls-say-when-the-key-changed' into 'main'

Browse Source 
Say when a call was refused because the key changed (android#20)

See merge request paulhitt/flamenet-messenger-android!68
This commit was merged in pull request #95.
This commit is contained in:
Paul Hitt
2026-09-05 14:43:07 +00:00
parent de968ac1aa 97961974dd
commit 94a28ea467
2 changed files with 191 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt
+127 -79
View File
@@ -4,6 +4,7 @@ import android.app.Application
import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.viewModelScope
import io.flamenet.client.ChainRpc
import io.flamenet.client.IdentityChangedException
import io.flamenet.client.DeviceClock
import io.flamenet.client.AccountsClient
import io.flamenet.client.RelayClient
@@ -2214,6 +2215,33 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
// WebRTC callback thread it is raised from.
viewModelScope.launch(Dispatchers.IO) {
runCatching { session?.sendCallSignal(peerId, signal) }
// 🔴 **This was `runCatching { }` and nothing else, so
// every failure here vanished.** The one that matters
// is a changed key: the send path REFUSES to encrypt to
// an unreviewed identity — TOFU working exactly as
// intended — and the exception was swallowed, so the
// offer never left and the screen sat in DIALLING until
// it timed out. Reported as "calls wouldn't connect and
// it wasn't obvious why", which is precisely right.
//
// ⚠️ The fix is one tap and it is on a screen the
// person is not looking at: the accept-the-new-key
// banner lives in the CHAT, and somebody calling from
// the Calls tab never sees it. So say where to go.
.onFailure { failure ->
if (failure is IdentityChangedException) {
calls?.hangUp(notifyPeer = false)
update {
it.copy(
call = null,
error = "Their security code changed, so the call "
+ "was not placed. Open the conversation with "
+ "${nameFor(it, peerId)} to review it, then "
+ "call again.",
)
}
}
}
}
},
onStateChanged = { state ->
@@ -3511,86 +3539,16 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
* service answers them identically on purpose, and a message here that
* distinguishes them would undo that at the last step.
*/
private fun describe(failure: Throwable): String = when (failure) {
is AccountsClient.Failure.BadCredentials -> "That handle or passphrase isn't right."
is AccountsClient.Failure.SessionExpired ->
"Your session expired. Sign out and back in to keep going."
private fun describe(failure: Throwable): String = describeFailure(failure)
is AccountsClient.Failure.HandleTaken -> "That handle is already taken."
// ⚠️ Each of these says what to DO. A refusal somebody cannot act on is
// the same as no explanation: they retry the same thing and are refused
// again. "Too similar" without "changing a digit won't help" sends
// people round that loop deliberately.
is AccountsClient.Failure.EmailTaken ->
"An account with that email already exists. Try signing in instead."
is AccountsClient.Failure.HandleConfusable ->
"That handle looks too much like one that already exists. " +
"Changing a digit won't help — try a different word."
is AccountsClient.Failure.DisposableEmail ->
"That email provider isn't accepted. Use an address you'll still be " +
"able to read later — it's how you get back in."
is AccountsClient.Failure.BadHandle ->
"Handles can only contain lowercase letters and numbers."
is AccountsClient.Failure.BadEmail -> "That doesn't look like a valid email address."
is AccountsClient.Failure.WeakPassword ->
"Your passphrase needs to be at least 8 characters."
is AccountsClient.Failure.RateLimited -> "Too many attempts. Wait a few minutes."
// ⚠️ Not the underlying message: at this layer it is a Java networking
// string ("Failed to connect to /10.0.2.2:443"), which names an address
// the person never typed.
is AccountsClient.Failure.Transport ->
"Couldn't reach that server. Check the address and your connection."
// ⚠️ NOT "that passphrase is wrong". The relay rejects a TOKEN, which
// the user never typed and cannot check. Telling them their passphrase
// is wrong when their session merely aged out sends them off to change
// a password that was fine — observed on a send ten hours after a
// successful sign-in.
is RelayException.Unauthorized ->
"Your session expired. Sign out and back in to keep going."
is RelayException.NotFound -> "This server doesn't look like a Flamenet relay."
// The relay now says WHEN (Retry-After); a refusal with a clock is
// the difference between a person who waits and one who hammers.
is RelayException.RateLimited -> failure.retryAfterSeconds?.let { s ->
val mins = ((s + 59) / 60).coerceAtLeast(1)
"Too many attempts. Try again in about $mins minute" +
if (mins == 1) "." else "s."
} ?: "Too many attempts. Wait a few minutes."
is Vault.Corrupt -> "This phone's stored keys could not be read."
// 🔴 **Anything not named above used to reach the screen verbatim.**
// `RelayException.Http` carries "relay returned 409: {body}" as its
// message, so somebody choosing a handle that was already in use was
// shown a status code and a JSON fragment. Nothing in that tells them
// to pick a different name, which is the entire content of the answer.
//
// ⚠️ The body is deliberately dropped rather than appended. It is
// written for whoever is reading the logs, and half of these carry the
// handle back in it.
is RelayException.Http -> when (failure.status) {
409 -> "That handle is already taken. Try a different one."
400, 422 -> "That server couldn't accept those details. " +
"Check the handle and try again."
// 🔴 **Say what the relay said.** A 403 is a rule refusing this
// specific thing, and the relay writes a sentence that names it
// and what to do — "this account already has 3 registered devices,
// remove one from the Devices screen". "That isn't allowed on this
// server" replaces an answer somebody can act on with one they
// cannot, and leaves them guessing at a rule nobody stated.
//
// ⚠️ Only when it really is a sentence. `body` falls back to the
// raw payload when there is no `message`, so anything that looks
// like JSON or a bare code keeps the generic line rather than
// putting a fragment on screen.
403 -> failure.body
.takeIf { it.isNotBlank() && !it.trimStart().startsWith("{") && " " in it }
?: "That isn't allowed on this server."
in 500..599 -> "That server is having trouble right now. " +
"Try again in a moment."
else -> "That server couldn't complete this. Try again in a moment."
}
is RelayException.Malformed -> "That server sent back something unexpected."
else -> "Something went wrong reaching that server."
}
/**
* The failure-to-sentence mapping, as a pure function of its argument.
*
* ⚠️ Lifted out of the view model so it can be tested without an
* `Application`. It never touched instance state — it reads only the
* throwable — so this is a move, not a rewrite, and the alternative was
* a Robolectric run to assert a string.
*/
/**
* ⚠️ A recorder outlives the view model unless something stops it, and a
@@ -3632,3 +3590,93 @@ object FileLevels {
LOCAL_LEVELS.firstOrNull { it.first == classification }?.second
?: "Level $classification"
}
internal fun describeFailure(failure: Throwable): String = when (failure) {
is AccountsClient.Failure.BadCredentials -> "That handle or passphrase isn't right."
is AccountsClient.Failure.SessionExpired ->
"Your session expired. Sign out and back in to keep going."
is AccountsClient.Failure.HandleTaken -> "That handle is already taken."
// ⚠️ Each of these says what to DO. A refusal somebody cannot act on is
// the same as no explanation: they retry the same thing and are refused
// again. "Too similar" without "changing a digit won't help" sends
// people round that loop deliberately.
is AccountsClient.Failure.EmailTaken ->
"An account with that email already exists. Try signing in instead."
is AccountsClient.Failure.HandleConfusable ->
"That handle looks too much like one that already exists. " +
"Changing a digit won't help — try a different word."
is AccountsClient.Failure.DisposableEmail ->
"That email provider isn't accepted. Use an address you'll still be " +
"able to read later — it's how you get back in."
is AccountsClient.Failure.BadHandle ->
"Handles can only contain lowercase letters and numbers."
is AccountsClient.Failure.BadEmail -> "That doesn't look like a valid email address."
is AccountsClient.Failure.WeakPassword ->
"Your passphrase needs to be at least 8 characters."
is AccountsClient.Failure.RateLimited -> "Too many attempts. Wait a few minutes."
// ⚠️ Not the underlying message: at this layer it is a Java networking
// string ("Failed to connect to /10.0.2.2:443"), which names an address
// the person never typed.
is AccountsClient.Failure.Transport ->
"Couldn't reach that server. Check the address and your connection."
// ⚠️ NOT "that passphrase is wrong". The relay rejects a TOKEN, which
// the user never typed and cannot check. Telling them their passphrase
// is wrong when their session merely aged out sends them off to change
// a password that was fine — observed on a send ten hours after a
// successful sign-in.
is RelayException.Unauthorized ->
"Your session expired. Sign out and back in to keep going."
is RelayException.NotFound -> "This server doesn't look like a Flamenet relay."
// The relay now says WHEN (Retry-After); a refusal with a clock is
// the difference between a person who waits and one who hammers.
is RelayException.RateLimited -> failure.retryAfterSeconds?.let { s ->
val mins = ((s + 59) / 60).coerceAtLeast(1)
"Too many attempts. Try again in about $mins minute" +
if (mins == 1) "." else "s."
} ?: "Too many attempts. Wait a few minutes."
// 🔴 **Named, because the generic fallback sent people to the network.**
// "Something went wrong reaching that server" is what this used to
// produce for a changed key, and the server is fine — the refusal is
// this phone declining to encrypt to an identity nobody has reviewed.
// iOS names it ("Their security code changed"); this said nothing
// anywhere, in the call path or the message path.
is IdentityChangedException ->
"Their security code changed. Open the conversation to review it " +
"before sending anything else."
is Vault.Corrupt -> "This phone's stored keys could not be read."
// 🔴 **Anything not named above used to reach the screen verbatim.**
// `RelayException.Http` carries "relay returned 409: {body}" as its
// message, so somebody choosing a handle that was already in use was
// shown a status code and a JSON fragment. Nothing in that tells them
// to pick a different name, which is the entire content of the answer.
//
// ⚠️ The body is deliberately dropped rather than appended. It is
// written for whoever is reading the logs, and half of these carry the
// handle back in it.
is RelayException.Http -> when (failure.status) {
409 -> "That handle is already taken. Try a different one."
400, 422 -> "That server couldn't accept those details. " +
"Check the handle and try again."
// 🔴 **Say what the relay said.** A 403 is a rule refusing this
// specific thing, and the relay writes a sentence that names it
// and what to do — "this account already has 3 registered devices,
// remove one from the Devices screen". "That isn't allowed on this
// server" replaces an answer somebody can act on with one they
// cannot, and leaves them guessing at a rule nobody stated.
//
// ⚠️ Only when it really is a sentence. `body` falls back to the
// raw payload when there is no `message`, so anything that looks
// like JSON or a bare code keeps the generic line rather than
// putting a fragment on screen.
403 -> failure.body
.takeIf { it.isNotBlank() && !it.trimStart().startsWith("{") && " " in it }
?: "That isn't allowed on this server."
in 500..599 -> "That server is having trouble right now. " +
"Try again in a moment."
else -> "That server couldn't complete this. Try again in a moment."
}
is RelayException.Malformed -> "That server sent back something unexpected."
else -> "Something went wrong reaching that server."
}
app/src/test/kotlin/io/flamenet/messenger/app/ChangedKeyIsExplainedTest.kt
+64
View File
@@ -0,0 +1,64 @@
package io.flamenet.messenger.app
import io.flamenet.client.IdentityChangedException
import kotlin.test.Test
import kotlin.test.assertTrue
/**
* A refused-because-the-key-changed failure has to say so (android#20).
*
* 🔴 **Reported from a real session: "the calls wouldn't connect because the
* keys were changed. it wasn't obvious why the call failed."** It was not
* obvious because nothing said it. The call's signalling lambda was
* `runCatching { … }` with no `onFailure`, so `IdentityChangedException` — the
* send path refusing to encrypt to an unreviewed identity, which is TOFU
* working — vanished. The offer never left and the screen sat in DIALLING.
*
* ⚠️ **And the fix is on a screen the caller is not looking at.** Accepting a
* changed key is a banner in the CHAT; somebody calling from the Calls tab
* never sees it. So the message has to name the conversation, not just the
* problem.
*
* ⚠️ iOS has said "Their security code changed" on this path since it was
* built. This is a parity gap, not a new idea — which is why the assertion is
* about the substance of the sentence rather than its exact wording.
*/
class ChangedKeyIsExplainedTest {
private val message = describeFailure(IdentityChangedException(7L))
@Test
fun `it names the security code rather than the network`() {
assertTrue(
message.contains("security code", ignoreCase = true),
"a changed key was described as \"$message\" — the person is sent " +
"hunting a network problem that does not exist",
)
}
/**
* 🔴 **The actionable half.** A refusal somebody cannot act on is the same
* as no explanation: they retry the same thing and are refused again. This
* file's neighbours in `describe()` all say what to DO, and that rule is
* written down there — this one has to keep it.
*/
@Test
fun `it says where to go and what to do`() {
assertTrue(
message.contains("conversation", ignoreCase = true) &&
message.contains("review", ignoreCase = true),
"\"$message\" does not point at the conversation, which is the only " +
"place the key can be accepted",
)
}
/** ⚠️ Not the generic fallback, which is what this used to produce. */
@Test
fun `it is not the generic something-went-wrong line`() {
assertTrue(
!message.contains("Something went wrong", ignoreCase = true) &&
!message.contains("reaching that server", ignoreCase = true),
"a changed key still falls through to the generic server line: \"$message\"",
)
}
}