Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions src/client/node_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1101,11 +1101,14 @@ impl Client {
tracing::instrument(name = "wa.conn.ack_response", level = "debug", skip_all)
)]
pub(crate) async fn handle_ack_response(&self, node: &wacore_binary::NodeRef<'_>) -> bool {
let ack_id = node.get_attr("id");
let ack_error = node.get_attr("error");

// Surface server nack codes for diagnosability. A nacked send still
// resolves Ok to the caller, so without this the failure is invisible.
if let Some(error_code) = node.get_attr("error") {
if let Some(error_code) = &ack_error {
let code = error_code.as_str();
let id = node.get_attr("id").map(|v| v.as_str());
let id = ack_id.as_ref().map(|v| v.as_str());
match code.as_ref() {
"463" => {
warn!(
Expand Down Expand Up @@ -1136,7 +1139,33 @@ impl Client {
}
}

if let Some(id) = node.get_attr("id").map(|v| v.as_str())
// Dispatched before waiter resolution; gated on interest so the hot path
// allocates nothing when nobody is listening.
if self
.core
.event_bus
.has_handler_for(wacore::types::events::EventKind::ServerAck)
&& let Some(id) = &ack_id
{
let ack = wacore::types::events::ServerAck {
id: id.as_str().to_string(),
class: node
.get_attr("class")
.map(|v| v.as_str().to_string())
.unwrap_or_default(),
from: node.get_attr("from").and_then(|v| v.as_str().parse().ok()),
timestamp: node
.get_attr("t")
.and_then(|v| v.as_str().parse::<i64>().ok())
.and_then(|secs| chrono::DateTime::from_timestamp(secs, 0)),
error: ack_error.as_ref().map(|v| v.as_str().to_string()),
};
self.core
.event_bus
.dispatch(wacore::types::events::Event::ServerAck(ack));
}

if let Some(id) = ack_id.map(|v| v.as_str())
&& let Some(waiter) = self.response_waiters_guard().remove(id.as_ref())
{
// ACK responses are infrequent; re-encode into OwnedNodeRef for the channel.
Expand Down
68 changes: 68 additions & 0 deletions src/client/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,74 @@ async fn test_ack_without_matching_waiter() {
);
}

/// Every server `<ack>` with an id dispatches an observe-only
/// `Event::ServerAck` carrying the ack's class/from/t, independent of
/// waiter state; a nack carries its error code. Lets consumers measure
/// send → server-accept latency and see nack codes programmatically
/// instead of scraping warn! logs.
#[tokio::test]
async fn test_ack_dispatches_server_ack_event() {
use wacore::types::events::{Event, EventHandler};

let client = crate::test_utils::create_test_client().await;
let collector = Arc::new(crate::test_utils::TestEventCollector::default());
client.register_handler(collector.clone() as Arc<dyn EventHandler>);

// Plain message ack (no waiter registered): event fires with the ack's
// class, from and server timestamp; error is None.
let ack_node = NodeBuilder::new("ack")
.attr("id", "ack-evt-1")
.attr("class", "message")
.attr("from", "123456789@s.whatsapp.net")
.attr("t", "1720000000")
.build();
client.handle_ack_response(&ack_node.as_node_ref()).await;
assert!(
collector.events().iter().any(|e| matches!(
e.as_ref(),
Event::ServerAck(ack)
if ack.id == "ack-evt-1"
&& ack.class == "message"
&& ack.from.as_ref().is_some_and(|j| j.to_string() == "123456789@s.whatsapp.net")
&& ack.timestamp.is_some_and(|t| t.timestamp() == 1_720_000_000)
&& ack.error.is_none()
)),
"server <ack> should dispatch Event::ServerAck with class/from/t"
);

// Nack: the error code rides along; absent class/t stay empty/None.
let nack_node = NodeBuilder::new("ack")
.attr("id", "ack-evt-2")
.attr("error", "479")
.attr("from", SERVER_JID)
.build();
client.handle_ack_response(&nack_node.as_node_ref()).await;
assert!(
collector.events().iter().any(|e| matches!(
e.as_ref(),
Event::ServerAck(ack)
if ack.id == "ack-evt-2"
&& ack.class.is_empty()
&& ack.timestamp.is_none()
&& ack.error.as_deref() == Some("479")
)),
"server nack should dispatch Event::ServerAck carrying the error code"
);

// An ack without an id (e.g. non-message acks) dispatches nothing.
let anon_ack = NodeBuilder::new("ack").attr("from", SERVER_JID).build();
client.handle_ack_response(&anon_ack.as_node_ref()).await;
assert_eq!(
collector
.events()
.iter()
.filter(|e| matches!(e.as_ref(), Event::ServerAck(_)))
.count(),
2,
"an <ack> without an id must not dispatch Event::ServerAck"
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/// Test that the lid_pn_cache correctly stores and retrieves LID mappings.
///
/// This is critical for the LID-PN session mismatch fix. When we receive a message
Expand Down
34 changes: 33 additions & 1 deletion wacore/src/types/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ pub enum EventKind {
PairPasskeyRequest,
PairPasskeyConfirmation,
PairPasskeyError,
ServerAck,
// When adding a variant, mind the 64-kind ceiling below (EventInterest packs
// each discriminant as a bit in a u64) and keep the guard pointing at the
// last variant.
Expand All @@ -278,7 +279,7 @@ impl EventKind {

// Build-time tripwire: a new variant that would overflow EventInterest's bitmask
// fails compilation instead of silently corrupting the mask at runtime.
const _: () = assert!((EventKind::PairPasskeyError as u8) < EventKind::CAPACITY);
const _: () = assert!((EventKind::ServerAck as u8) < EventKind::CAPACITY);

/// A set of [`EventKind`]s a handler wants delivered. The event bus skips
/// materializing and dispatching events whose kind no handler wants, so a
Expand Down Expand Up @@ -651,6 +652,14 @@ pub enum Event {
/// `info.unavailable_request_id`) dispatch event-only.
Messages(MessageBatch),
Receipt(Receipt),
/// The server `<ack>`-ed (or nack-ed) an outgoing stanza.
///
/// Observe-only: dispatched for every server `<ack>` that carries an id,
/// before and independently of the internal ack-waiter resolution, so it
/// never interacts with the send/phash flow. Lets consumers measure
/// send → server-accept latency and surface nack codes programmatically
/// (today nacks are only visible as `warn!` logs).
ServerAck(ServerAck),
UndecryptableMessage(UndecryptableMessage),
#[serde(skip)]
Notification(Arc<OwnedNodeRef>),
Expand Down Expand Up @@ -843,6 +852,7 @@ impl Event {
Event::PairPasskeyRequest(_) => EventKind::PairPasskeyRequest,
Event::PairPasskeyConfirmation(_) => EventKind::PairPasskeyConfirmation,
Event::PairPasskeyError(_) => EventKind::PairPasskeyError,
Event::ServerAck(_) => EventKind::ServerAck,
}
}

Expand Down Expand Up @@ -1176,6 +1186,28 @@ pub struct Receipt {
pub offline: bool,
}

/// Payload of [`Event::ServerAck`]: the server acknowledged (or nacked) an
/// outgoing stanza. Server acks cover every outgoing stanza class — message,
/// receipt, notification, call — so consumers should filter on [`class`](Self::class)
/// before correlating ids.
#[derive(Debug, Clone, Serialize)]
pub struct ServerAck {
/// Id of the acked stanza (for a sent message, its message id).
pub id: String,
/// Stanza class the ack refers to (`"message"`, `"receipt"`,
/// `"notification"`, `"call"`, …). Empty when the server omits it.
pub class: String,
/// Chat/entity the ack refers to, when present and parseable.
pub from: Option<Jid>,
/// Server timestamp from the ack's `t` attribute, when present. For a
/// message ack this is the authoritative send timestamp (whatsmeow reads
/// the same attribute into `SendResponse.Timestamp`).
pub timestamp: Option<DateTime<Utc>>,
/// Nack code (e.g. `"479"`) when the server rejected the stanza; `None`
/// for a plain ack.
pub error: Option<String>,
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
#[derive(Debug, Clone, Serialize)]
pub struct ChatPresenceUpdate {
pub source: crate::types::message::MessageSource,
Expand Down