No call signal is lost to whichever poll loop happens to win
Two loops poll one mailbox: the service's, which keeps it watched while nothing is on screen, and the view model's. An envelope goes to whichever asks first -- the relay marks it delivered and does not hand it out again. The service's handler knew what to do with an offer and an end and had `else -> Unit` for everything else, so an ANSWER it happened to fetch was thrown away. Measured, not inferred: an iPhone answered, the relay accepted the answer, this side logged the envelope decrypting, and the call manager's own "call signal in" line never appeared. The signal reached the process and stopped one layer short of the call. The caller then sat on "Calling..." for the full forty-five seconds and gave up, every time the race went that way. An END fared no better: it cleared a notification and left a live call running. A call now registers itself so either loop can reach it, and the service forwards every signal after its `when` rather than inside a branch of it -- in a branch it would cover only the actions somebody remembered, which is the bug restated, so there is a test holding it outside. A CALL CANNOT WAIT ON A TWENTY SECOND HOLD The same loop asked the relay to hold every request open for twenty seconds. Right for messages, wrong for the two signals somebody is watching a screen for. iOS polls hard for the length of a call and this never had the same thing, so an answer sent at once was accepted twenty seconds later: offer at :07.9, answer accepted at :28.5, with the relay holding it the whole time. It polls with no hold at a one second cadence while a call is up, and goes back to the hold when there is not one. The same pair now measures 2.8s. ENVELOPES THAT ARRIVE TWICE `hasSeenEnvelope` is guarded on the id being present and nothing sent one on the ordinary route, so it was asked about nothing. One offer on the relay was delivered twice 2.4 seconds apart and the second was answered "busy" by the phone ringing for the first. Every send carries an id now, with a default on the field so a new call site cannot forget one, and an offer for the call this phone is already on is no longer mistaken for a second caller. CLOSING THE APP DURING A CALL `onTaskRemoved` is what fires when the app is swiped out of Recents, and it runs while the process is still alive, which is the point: by `onDestroy` there may be no time left to reach the network. On a thread with a bounded join rather than a coroutine, because the scope is cancelled a moment later and a launched coroutine would be killed before the envelope left. 1606 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
74429be153
commit
5945a87189
@@ -205,6 +205,20 @@ class RelayClient(
|
||||
data class ToDevice(
|
||||
val deviceId: String,
|
||||
val payload: String,
|
||||
/**
|
||||
* One id for this envelope, so the far side can tell a repeat from a
|
||||
* second message.
|
||||
*
|
||||
* Both clients already refuse an envelope id they have seen before and
|
||||
* neither ever sent one on this route, so every ordinary envelope
|
||||
* reached the relay with a null id and the check on the other side
|
||||
* could never fire. Envelopes DO arrive twice: two loops poll one
|
||||
* mailbox and race on the inbox cursor. Measured -- one call offer on
|
||||
* the relay, delivered twice 2.4 seconds apart, the second answered
|
||||
* "busy" by the phone ringing for the first. Messages double the same
|
||||
* way, more quietly.
|
||||
*/
|
||||
val envelopeId: String = java.util.UUID.randomUUID().toString(),
|
||||
) {
|
||||
constructor(deviceId: String, envelope: Envelope) : this(deviceId, envelope.toPayload())
|
||||
}
|
||||
@@ -258,7 +272,8 @@ class RelayClient(
|
||||
array.put(
|
||||
JSONObject()
|
||||
.put("to_device", it.deviceId)
|
||||
.put("payload", it.payload),
|
||||
.put("payload", it.payload)
|
||||
.put("envelope_id", it.envelopeId),
|
||||
)
|
||||
}
|
||||
},
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
package io.flamenet.client
|
||||
|
||||
import java.io.File
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* An envelope that arrives twice must be recognised as the same envelope.
|
||||
*
|
||||
* The de-duplication was already written on both sides and could never fire:
|
||||
* it is guarded on the envelope id being non-empty, and nothing sent one on
|
||||
* the ordinary route. The field existed only on the federated send, so every
|
||||
* envelope on the relay had a null id.
|
||||
*
|
||||
* Envelopes do arrive twice. Two loops poll one mailbox and race on the inbox
|
||||
* cursor. Measured: one call offer on the relay, two deliveries 2.4 seconds
|
||||
* apart, the second answered "busy" by the phone that was ringing for the
|
||||
* first. Messages and call log entries double the same way and more quietly.
|
||||
*/
|
||||
class EveryEnvelopeHasAnIdTest {
|
||||
private fun source(path: String): String {
|
||||
val file = File("src/main/kotlin/io/flamenet/client/$path")
|
||||
assertTrue(file.exists(), "missing $path")
|
||||
return file.readText()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an outgoing envelope carries an id`() {
|
||||
val relay = source("RelayClient.kt")
|
||||
assertTrue(
|
||||
relay.contains("val envelopeId: String = java.util.UUID.randomUUID().toString()"),
|
||||
"without a default every call site has to remember, and one will not",
|
||||
)
|
||||
assertTrue(
|
||||
relay.contains(""".put("envelope_id", it.envelopeId)"""),
|
||||
"the id has to reach the wire, not just the data class",
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The check on the way in is what the id is for, and it is guarded on the
|
||||
* id being present -- which is exactly why a missing id failed silently
|
||||
* rather than loudly.
|
||||
*/
|
||||
@Test
|
||||
fun `an envelope seen before is dropped on the way in`() {
|
||||
val conversations = source("Conversations.kt")
|
||||
assertTrue(conversations.contains("if (message.envelopeId.isNotEmpty()) {"))
|
||||
assertTrue(conversations.contains("if (vault.hasSeenEnvelope(message.envelopeId)) {"))
|
||||
assertTrue(
|
||||
conversations.contains("vault.rememberEnvelope(message.envelopeId)"),
|
||||
"remembered, or the second copy is only dropped by luck",
|
||||
)
|
||||
}
|
||||
|
||||
/** The watermark still advances past a duplicate, or it is fetched for ever. */
|
||||
@Test
|
||||
fun `a duplicate still advances the watermark`() {
|
||||
val conversations = source("Conversations.kt")
|
||||
val block = conversations.substringAfter("if (vault.hasSeenEnvelope(message.envelopeId)) {")
|
||||
.take(400)
|
||||
assertTrue(
|
||||
block.contains("inboxWatermark = message.id") ||
|
||||
block.contains("setWatermarkFor(home, message.id)"),
|
||||
"a skipped envelope that never advances is fetched for ever",
|
||||
)
|
||||
assertFalse(
|
||||
block.substringBefore("continue").contains("out.add"),
|
||||
"and it must not be ingested anyway",
|
||||
)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user