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

Stop blaming the server for failures that never reached it

Browse Source 
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 lands in the catch-all never touched the network.

Seen while testing the released 0.29 build on an emulator. Sign-up against a
relay that answered 200 to signup, token, device registration and identity put
that sentence on the screen. 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. It is also what produced the
duplicate token request in the relay's access log.

Same shape as the relay-pin bug fixed in 80bb4c4: a specific failure arriving
as a shrug about the network, and the fix is the same. Say what is known.
RelayException.Transport now has its own branch and is the only place allowed
to blame the connection, because it is the only one that knows. The catch-all
says the failure was on this phone, and carries the cause's own sentence when
there is a readable one, refusing anything that looks like a class name.

Seven tests, falsified: restoring the old sentence turns three of them red.
App unit suite 633 green.

Not fixed here: WHY that sign-up failed locally after a 200. The message was
hiding it, and now it will say.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-09-21 15:06:38 -04:00
co-authored by Claude Opus 5
parent f8efd4d12c
commit 2c22ffed75
2 changed files with 138 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt
+32 -1
View File
@@ -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."
}
}
}
app/src/test/kotlin/io/flamenet/messenger/app/ALocalFailureIsNotBlamedOnTheServerTest.kt
+106
View File
@@ -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",
)
}
}