A session we ANSWER can now be v3, and receive failures say why
`E2ESession.respond` hard-coded `version = if (pq == null) 1 else 2`, so
a session this device answered could never be v3 — while `initiate`
twelve lines above correctly returns 3 whenever the post-quantum ratchet
is on. A conversation iOS started therefore ran as v3 on one side and v2
on the other, and every message this device sent into it was refused by
the peer as a downgrade (§13.4) and dropped on arrival. iOS to Android
worked; Android to iOS did not, for as long as the pairing existed.
Kotlin talking to Kotlin never noticed, because both ends drive the
ratchet from their own PQ state and ignore the label. Only a peer that
enforces the version sees it. `HybridSessionTest` asserted the wrong
value — `assertEquals(2, bobSide.version)`, two lines below requiring
the initiator to be 3 — so the suite passed while encoding the defect.
Corrected, with the reason recorded next to it.
The version now follows the initiator rather than assuming 3 whenever PQ
is present: a v2 peer sends a PQ block too, and answering that in v3
breaks the pairing in the other direction.
Receive-side failures are no longer swallowed. `catch (_: Exception) {}`
threw away the reason for every undecryptable envelope, which left this
half of every delivery investigation unreadable — the relay says
delivered, the app shows nothing, and the one place that knew why
discarded it. The envelope is still skipped; it now names the id, the
device and the exception. Sends report the protocol shape they emit, and
receives report which of the three chains opened them.
Also here:
- proguard-rules.pro, which did not exist while build.gradle.kts
referenced it. R8 accepts a missing entry in silence, so release
builds ran on the platform defaults and answering a call aborted the
process inside WebRTC's JNI_OnLoad — it resolves Java classes by
name, and R8 had renamed them. The build now fails if the file is
missing rather than shipping without it.
- Server refusals read as sentences. `RelayException.Http` had no case
in `describe()`, so choosing a taken handle showed "relay returned
409:" and a JSON fragment, which says nothing about picking another
name. Transport failures showed a Java networking string naming an
address the person never typed.
- Switching between signing in and creating an account clears the last
error. It belonged to the other mode and stayed on screen, so taking
the advice it gave looked like it had failed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
db6ad2a8ce
commit
ee17d773b7
@@ -149,7 +149,21 @@ class CallManager(
|
||||
onGathered = null
|
||||
gatherGuard?.let { main.removeCallbacks(it) }
|
||||
gatherGuard = null
|
||||
val sdp = connection?.localDescription?.description ?: return
|
||||
// 🔴 **This `?: return` sent nobody anything, silently.** After the
|
||||
// 8s gather timeout the local description can still be null — because
|
||||
// setLocalDescription had not finished, or had FAILED and said nothing
|
||||
// (see SimpleSdpObserver). The call then dies here: no offer, no
|
||||
// envelope on the relay, no ring on the other phone, and no error.
|
||||
// Verified against the database — zero envelopes had ever been created
|
||||
// between two accounts that had "placed" several calls.
|
||||
val sdp = connection?.localDescription?.description ?: run {
|
||||
android.util.Log.w("fnmsg-call", "NOT SENDING the offer/answer: no localDescription after " +
|
||||
"gathering (state=${connection?.iceGatheringState()}). " +
|
||||
"The other side will never see this call.")
|
||||
return
|
||||
}
|
||||
android.util.Log.i("fnmsg-call", "sending signal after gathering, sdp=${sdp.length} bytes, " +
|
||||
"state=${connection?.iceGatheringState()}")
|
||||
send(sdp)
|
||||
}
|
||||
|
||||
@@ -213,7 +227,15 @@ class CallManager(
|
||||
continualGatheringPolicy = PeerConnection.ContinualGatheringPolicy.GATHER_ONCE
|
||||
}
|
||||
val observer = object : PeerConnection.Observer {
|
||||
override fun onIceCandidate(candidate: IceCandidate?) = Unit
|
||||
override fun onIceCandidate(candidate: IceCandidate?) {
|
||||
// 🔴 The candidate TYPE is the whole diagnosis. `host` only
|
||||
// means we never reached STUN/TURN and cannot cross a NAT;
|
||||
// `relay` means TURN answered and the call can connect even
|
||||
// when a direct path is impossible.
|
||||
val sdp = candidate?.sdp ?: return
|
||||
val kind = Regex("typ (\\w+)").find(sdp)?.groupValues?.get(1) ?: "?"
|
||||
android.util.Log.i("fnmsg-call", "ice candidate: typ=$kind")
|
||||
}
|
||||
override fun onIceCandidatesRemoved(candidates: Array<out IceCandidate>?) = Unit
|
||||
override fun onSignalingChange(state: PeerConnection.SignalingState?) = Unit
|
||||
// 🔴 **Nothing may throw out of here, and nothing may call back
|
||||
@@ -222,6 +244,7 @@ class CallManager(
|
||||
// out — so ANY escaping Kotlin exception aborts the process:
|
||||
// SIGABRT, no AndroidRuntime trace, nothing to catch.
|
||||
override fun onIceConnectionChange(newState: PeerConnection.IceConnectionState?) {
|
||||
android.util.Log.i("fnmsg-call", "ice connection -> $newState")
|
||||
runCatching {
|
||||
when (newState) {
|
||||
PeerConnection.IceConnectionState.CONNECTED,
|
||||
@@ -239,6 +262,7 @@ class CallManager(
|
||||
}
|
||||
override fun onIceConnectionReceivingChange(receiving: Boolean) = Unit
|
||||
override fun onIceGatheringChange(newState: PeerConnection.IceGatheringState?) {
|
||||
android.util.Log.i("fnmsg-call", "ice gathering -> $newState")
|
||||
// Hop to the main thread rather than touching the connection
|
||||
// from here — same JNI rule as above: no WebRTC calls, and no
|
||||
// exceptions, from inside a WebRTC callback.
|
||||
@@ -327,7 +351,7 @@ class CallManager(
|
||||
}
|
||||
pc.createOffer(object : SimpleSdpObserver() {
|
||||
override fun onCreateSuccess(sdp: SessionDescription) {
|
||||
pc.setLocalDescription(SimpleSdpObserver(), sdp)
|
||||
pc.setLocalDescription(SimpleSdpObserver("offer:setLocal"), sdp)
|
||||
// The complete SDP — with candidates — exists only after
|
||||
// gathering; see onGathered. DIALLING holds until the answer
|
||||
// arrives, so the caller's screen says what is actually true.
|
||||
@@ -428,12 +452,12 @@ class CallManager(
|
||||
}
|
||||
current = Active(offer.callId, fromUser, incoming = true, video = offer.video)
|
||||
pc.setRemoteDescription(
|
||||
SimpleSdpObserver(),
|
||||
SimpleSdpObserver("answer:setRemote"),
|
||||
SessionDescription(SessionDescription.Type.OFFER, sdp),
|
||||
)
|
||||
pc.createAnswer(object : SimpleSdpObserver() {
|
||||
override fun onCreateSuccess(answer: SessionDescription) {
|
||||
pc.setLocalDescription(SimpleSdpObserver(), answer)
|
||||
pc.setLocalDescription(SimpleSdpObserver("answer:setLocal"), answer)
|
||||
// Same as the offer: the answer is complete only after
|
||||
// gathering, and one sent early names no address to reach.
|
||||
main.post {
|
||||
@@ -614,9 +638,24 @@ object PendingIncomingCall {
|
||||
}
|
||||
|
||||
/** Most SDP callbacks have nothing to say; this stops four stubs per call site. */
|
||||
private open class SimpleSdpObserver : SdpObserver {
|
||||
/**
|
||||
* 🔴 **This discarded every SDP failure, and that is why calls were undebuggable.**
|
||||
* `createOffer` or `setLocalDescription` could fail for any reason and nothing
|
||||
* anywhere recorded it — the offer then simply never went out, the callee never
|
||||
* rang, and the database showed no envelope had ever been created. Four hours of
|
||||
* "the call failed" with no cause, because the two callbacks that knew returned
|
||||
* `Unit`.
|
||||
*
|
||||
* ⚠️ A `label` so the log says WHICH step failed. "setLocalDescription failed"
|
||||
* and "createAnswer failed" send you to different places.
|
||||
*/
|
||||
private open class SimpleSdpObserver(private val label: String = "sdp") : SdpObserver {
|
||||
override fun onCreateSuccess(sdp: SessionDescription) = Unit
|
||||
override fun onSetSuccess() = Unit
|
||||
override fun onCreateFailure(error: String?) = Unit
|
||||
override fun onSetFailure(error: String?) = Unit
|
||||
override fun onCreateFailure(error: String?) {
|
||||
android.util.Log.w("fnmsg-call", "$label: createOffer/createAnswer FAILED: $error")
|
||||
}
|
||||
override fun onSetFailure(error: String?) {
|
||||
android.util.Log.w("fnmsg-call", "$label: setLocal/setRemoteDescription FAILED: $error")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -479,7 +479,13 @@ private fun ColumnScope.SignIn(model: MessengerViewModel, phase: MessengerViewMo
|
||||
}
|
||||
|
||||
TextButton(
|
||||
onClick = { registering = !registering },
|
||||
onClick = {
|
||||
registering = !registering
|
||||
// The last attempt's message was about the OTHER mode; leaving it
|
||||
// up makes switching look like it failed.
|
||||
ageError = null
|
||||
model.clearAuthError()
|
||||
},
|
||||
modifier = Modifier.fillMaxWidth().testTag("toggle-register"),
|
||||
) {
|
||||
Text(if (registering) "I already have an account" else "Create an account")
|
||||
|
||||
@@ -404,6 +404,22 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Forget whatever the last sign-in attempt said.
|
||||
*
|
||||
* 🔴 **Switching between signing in and creating an account has to do
|
||||
* this.** The message belongs to the mode it was produced in — "that
|
||||
* handle is already taken" is advice about signing up, and it stayed on
|
||||
* screen after the person took that advice and switched to signing in,
|
||||
* where it reads as a refusal of the thing they are now trying to do.
|
||||
*/
|
||||
fun clearAuthError() {
|
||||
val current = _phase.value
|
||||
if (current is Phase.SignedOut && current.error != null) {
|
||||
_phase.value = current.copy(error = null)
|
||||
}
|
||||
}
|
||||
|
||||
fun signIn(handle: String, passphrase: String, register: Boolean, email: String = "") {
|
||||
val config = site ?: run {
|
||||
_phase.value = Phase.SignedOut(error = "That doesn't look like a server address.")
|
||||
@@ -2460,7 +2476,11 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
|
||||
is AccountsClient.Failure.HandleTaken -> "That handle is already taken."
|
||||
is AccountsClient.Failure.RateLimited -> "Too many attempts. Wait a few minutes."
|
||||
is AccountsClient.Failure.Transport -> failure.message.orEmpty()
|
||||
// ⚠️ 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
|
||||
@@ -2477,7 +2497,27 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
if (mins == 1) "." else "s."
|
||||
} ?: "Too many attempts. Wait a few minutes."
|
||||
is Vault.Corrupt -> "This phone's stored keys could not be read."
|
||||
else -> failure.message ?: "Something went wrong reaching that server."
|
||||
|
||||
// 🔴 **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."
|
||||
403 -> "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."
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -630,8 +630,21 @@ class Session(
|
||||
}
|
||||
|
||||
/** TURN credentials, or null when this relay has none configured. */
|
||||
/**
|
||||
* 🔴 **A null here silently downgrades every call to STUN-only.**
|
||||
* `CallManager.iceServers(null)` falls back to Google's STUN server, which
|
||||
* cannot relay — so two peers behind NAT never connect, the call "just
|
||||
* fails", and coturn records nothing because it was never contacted. The
|
||||
* failure was swallowed by `getOrNull()`, so nothing anywhere said why.
|
||||
*/
|
||||
fun turnCredentials(): org.json.JSONObject? =
|
||||
runCatching { relay.turnCredentials() }.getOrNull()
|
||||
runCatching { relay.turnCredentials() }
|
||||
.onFailure {
|
||||
android.util.Log.w("fnmsg-call", "no TURN credentials " +
|
||||
"(${it.javaClass.simpleName}: ${it.message}) — this call will use " +
|
||||
"STUN only and will not connect across NAT", it)
|
||||
}
|
||||
.getOrNull()
|
||||
|
||||
fun sendGroup(group: RelayClient.Group, text: String): Conversations.Sent {
|
||||
val sent = conversations.sendToGroup(group, text, userId)
|
||||
|
||||
Reference in New Issue
Block a user