fix(whatsapp): never report a send as sent without server confirmation - #78
fix(whatsapp): never report a send as sent without server confirmation#78jqueguiner wants to merge 1 commit into
Conversation
wa-rs 0.2 returns a message id as soon as the stanza bytes are written to the noise socket. Nothing waits for the server ack, so a send on a dying socket returns Ok(id) and the CLI prints "Message sent" for a message the server never saw. Measured in production on 2026-09-11. The library's ack machinery is not reachable from a consumer: Client::response_waiters is pub(crate), and incoming <ack/> stanzas are consumed by handlers::basic::AckHandler without being dispatched on the public event bus (there is no Event::ServerAck). So confirm acceptance with stream ordering instead. All outbound frames go through one NoiseSocket sender task, so frames hit the TCP stream in call order. After the message, issue a w:p ping IQ and wait, bounded, for its pong: a pong proves the server consumed the stream past our message frame. Adds crates/void-whatsapp/src/connector/delivery.rs: - precheck(): fails fast when the connection has no live socket or is not logged in, before anything is written, - confirm_accepted(): 12s bounded barrier after the write, with timeout, bad-server-reply and transport outcomes kept distinguishable, - with_send_timeout(): hard cap on the write itself, which wa-rs does not bound. Wired into all four send paths in ops.rs (send, reply, and their notes-to-self variants). No error variant can be read as a success, and the two unknown cases say delivery is unknown rather than claiming loss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Live end-to-end verification on the fixed buildRan against a real WhatsApp session after installing this build and restarting the sync daemon, since Daemon restarted on the new binary: Happy path, send to self: Returned in 0.5 s, so the barrier does not slow the happy path. Confirmed delivered, not just claimed. Server receipts in the sync log: And present in WhatsApp's own store, read through a WAL-inclusive copy, with the id matching what the CLI printed: That id match is the point: before this PR the printed id proved nothing, because it was minted locally regardless of what the server did. One caveat worth recordingI tried to reproduce the original failure by stopping the daemon and sending. That does not exercise the barrier: with no daemon, the CLI opens its own connection ( |
What
A WhatsApp send can no longer be reported as sent unless the server confirms it.
New module
crates/void-whatsapp/src/connector/delivery.rs:precheck(): refuses to write when the connection has no live socket, or has a socket but is not logged in yet (the reconnect-handshake state). Nothing is written, and the error says so.confirm_accepted(): bounded barrier after the write. 12 s deadline, plus a 15 s outer cap becausesend_iqonly bounds the response wait, not thesend_nodethat precedes it.with_send_timeout(): 30 s hard cap on the write itself, whichwa-rsdoes not bound at all (the noise sender task can hang ontransport.send).Wired into all four send paths in
ops.rs:send_via_sync,reply_via_sync, and their notes-to-self variants.How the barrier proves acceptance, since the ack is not reachable (see Why): all outbound frames go through a single
NoiseSocketsender task (socket/noise_socket.rs,encrypt_and_sendpushes a job onto an mpsc queue and awaits that job's result), so frames hit the TCP stream in call order and the caller does not return until its own frame was handed to the transport. Issuing aw:pping IQ after the message and waiting for its pong is therefore a stream barrier: a pong proves the server consumed the stream past our message frame.Outcomes stay distinguishable, and none of them reads as a success:
The timeout and bad-server-reply cases say delivery is unknown. The transport case says the message very likely never arrived. Neither overclaims.
Why
Root cause confirmed, and it is in
wa-rs 0.2.0, read at~/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/wa-rs-0.2.0/:send.rs:40send_message_with_optionsgenerates the id locally, callssend_message_impl, returnsOk(request_id).send_message_implends onself.send_node(stanza_to_send).client.rs:2014send_nodemarshals, encrypts, writes, returnsOk(()).Nothing waits for the server
<ack/>. On 2026-09-11 the CLI printedMessage sent (id: 3EB01E021E254E73BF2930)in 0.66 s for a message that was in no store afterwards, and the wa-rs message loop exited 52 s later.An ack-aware send is not expressible from a consumer of wa-rs 0.2.0. Two things block it:
Client::response_waitersispub(crate)(client.rs:107).send_iquses it (request.rs:135), but there is no public way to register a waiter for an arbitrary stanza id.<ack/>stanzas never reach the public event bus.handlers/basic.rsAckHandler::handlecallsclient.handle_ack_response(node)and returnstrueto consume the stanza.handle_ack_response(client.rs:1315) only resolves aresponse_waitersentry. There is noEvent::ServerAckinwa_rs_core::types::events::Event, so a consumer cannot observe the ack for its own send.Missing upstream API, in order of usefulness:
Client::send_message_and_wait_ack(to, message, timeout) -> Result<String, SendError>, returning only after the server<ack/>for that message id.Event::ServerAck { id, class, error: Option<u16> }dispatched fromAckHandlerbefore it consumes the stanza, which would let a consumer correlate acks itself.Client::register_response_waiter(id) -> oneshot::Receiver<Node>.Event::Receiptis dispatched publicly, but a receipt is the recipient's delivery confirmation, not the server's acceptance. It does not arrive at all while the recipient is offline, so it cannot back a 12 s send verdict.Given that, this PR takes the documented fallback: liveness precheck plus a bounded post-send verification. The verification is stronger than a store or receipt poll, because the ping barrier is a positive proof that the server consumed our bytes.
Verified
Every command below was redirected to a file, the real exit code echoed, then the file read back.
Tails of those files:
New tests, from
/tmp/v_test.txt:The failing case, pinned
connector::tests::a_send_that_returns_an_id_but_is_never_confirmed_is_a_failureis the 2026-09-11 bug reproduced at the seam. The write is stubbed to returnOk("3EB01E021E254E73BF2930"), exactly what wa-rs does for a stanza the server never sees, and the barrier that follows cannot round-trip. The composed result is an error, not an id.The dead-socket tests build a real
wa_rs::Client(realPersistenceManager, real transport factory, in-memory store) that has never connected, then inject it into a realWhatsAppConnectorand call the realsend_via_sync/reply_via_sync.Before and after, on the same tests.
ops.rsstashed back to its pre-fix state:Read that honestly: on a fully dead client the old code already errored, with the opaque
Client is not connectedfrom deep inside wa-rs. So the fully-disconnected case is not the production bug, and this PR improves the wording there rather than flipping a success into a failure. The actual behaviour flip is the barrier, and it is the case the production incident hit: the socket looked usable, the write returnedOk(id), the stanza was lost. That path had no check at all before this PR, and is now covered bya_send_that_returns_an_id_but_is_never_confirmed_is_a_failureandbarrier_on_dead_socket_reports_transport_failure.Happy path
Covered by
barrier_pong_confirms_the_send(a pong classifies as accepted) andwith_send_timeout_passes_through_success(the id passes through untouched). The added cost on a healthy connection is onew:pping round-trip, the same IQ the keepalive loop already sends every 20 to 30 s.No live end-to-end send was performed, and no WhatsApp message was sent to anyone during this work. Reason stated below.
Not in this PR
void sendroutes through RPC to the daemon (writes.rs:159), so the send executes in the daemon process, not the CLI. Proving this live would mean killing and restarting the runningvoid sync --daemon-inner(PID 76381, up since the previous evening) on the new binary. That is a disruptive change to a live WhatsApp session, it was not asked for, and it could not be confirmed interactively. Restart the daemon on this build and one notes-to-self send will exercise it.wa_rs::transport::mockis#[cfg(test)]-gated upstream, and completing the handshake requires a real server static key, so the full stanza path cannot be faked from void. The barrier is instead covered at the seam and at unit level, everyIqErrorbranch included.Upstream rebase check
This branch is
origin/main(add6674) plus this one commit, nothing else. Re-verified on that base, not only on the local build:Closes #77