diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 48641e0..6b2a202 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -4558,8 +4558,39 @@ internal fun describeFailure(failure: Throwable): String = "That server sent back something unexpected." } + // The genuine "could not reach it" case, named so the catch-all below + // does not have to pretend to be it. + is RelayException.Transport -> { + "Couldn't reach that server. Check the address and this phone's " + + "connection, then try again." + } + + // Everything unnamed. + // + // This used to say "Something went wrong reaching that server", which + // is a claim about the network, and most of what reaches here never + // touched the network. Observed on 2026-09-21: sign-up on 0.29 against + // a relay that answered 200 to every request showed that sentence, so + // the account existed and the person was told the server could not be + // reached. They tap again, which is the one action the message implies + // and the one least likely to help. + // + // Same failure as the relay-pin branch above: a specific problem + // arriving as a shrug about the network. The fix is the same. Say what + // is actually known, which is that it failed and where it did not. + // `RelayException.Transport` above is the branch that may blame the + // connection, because it is the only one that knows. else -> { - "Something went wrong reaching that server." + val detail = + failure.message + ?.trim() + ?.takeIf { it.isNotEmpty() && it.length <= 120 && !it.contains("Exception") } + if (detail != null) { + "Something went wrong on this phone: $detail" + } else { + "Something went wrong on this phone, and the server was not " + + "the problem. Trying again may not help." + } } } diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/ALocalFailureIsNotBlamedOnTheServerTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/ALocalFailureIsNotBlamedOnTheServerTest.kt new file mode 100644 index 0000000..8390cc1 --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/ALocalFailureIsNotBlamedOnTheServerTest.kt @@ -0,0 +1,106 @@ +package io.flamenet.messenger.app + +import io.flamenet.client.AccountsClient +import io.flamenet.client.RelayException +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * `describeFailure` had one sentence for everything it could not name: + * "Something went wrong reaching that server." That is a claim about the + * network, and most of what reaches the catch-all never touched the network. + * + * Observed on 2026-09-21, driving the released 0.29 build against a relay that + * answered 200 to sign-up, token, device registration and identity: the screen + * said the server could not be reached. The account had been created. The + * person is told to check a connection that is working, and the one action the + * sentence implies, tapping again, is the one least likely to help. + * + * Same shape as the relay-pin bug this file's sibling covers: a specific + * failure arriving as a shrug about the network. + */ +class ALocalFailureIsNotBlamedOnTheServerTest { + private val networkWords = listOf("reaching that server", "reach that server", "connection") + + /** The case that was wrong: nothing to do with the network. */ + @Test + fun `a local failure does not claim the server was unreachable`() { + val message = describeFailure(IllegalStateException("the vault is not open")) + assertFalse( + networkWords.any { message.contains(it, ignoreCase = true) }, + "\"$message\" blames the network for a failure that never touched it", + ) + } + + @Test + fun `a local failure says where the problem was`() { + val message = describeFailure(IllegalStateException("the vault is not open")) + assertTrue( + message.contains("this phone", ignoreCase = true), + "\"$message\" does not say where it went wrong, which is the only " + + "thing the app actually knows", + ) + } + + /** + * The house rule from the branches above: say what is known. A cause with + * a readable sentence is more use than a generic line. + */ + @Test + fun `a readable cause reaches the person`() { + val message = describeFailure(IllegalStateException("the vault is not open")) + assertTrue( + message.contains("the vault is not open"), + "\"$message\" threw away a message written to be read", + ) + } + + /** A stack-trace-shaped string is not something to put on a screen. */ + @Test + fun `a class name is not shown as an explanation`() { + val message = describeFailure(RuntimeException("java.lang.NullPointerException: boom")) + assertFalse( + message.contains("Exception"), + "\"$message\" puts an exception class in front of a person", + ) + } + + /** The genuine case keeps its sentence, and it is now the only one that may. */ + @Test + fun `a transport failure is still allowed to blame the connection`() { + val message = describeFailure(RelayException.Transport("connect timed out")) + assertTrue( + message.contains("reach", ignoreCase = true), + "\"$message\" no longer tells someone with no signal what is wrong", + ) + assertTrue( + message.contains("connection", ignoreCase = true), + "\"$message\" does not mention the connection, which is the thing to check", + ) + } + + /** + * The account service already had its own branch and keeps it. Checked + * here only so the catch-all rewrite cannot quietly swallow it. + */ + @Test + fun `an account service that cannot be reached still blames the connection`() { + val message = describeFailure(AccountsClient.Failure.Transport("connect timed out")) + assertTrue( + message.contains("reach", ignoreCase = true) && + message.contains("connection", ignoreCase = true), + "\"$message\" no longer tells someone with no signal what is wrong", + ) + } + + /** A named relay error keeps the branch it already had. */ + @Test + fun `an http status is still explained on its own terms`() { + val message = describeFailure(RelayException.Http(409, "")) + assertTrue( + message.contains("handle", ignoreCase = true), + "\"$message\" stopped explaining a taken handle", + ) + } +}