diff --git a/src/bot.rs b/src/bot.rs index 4dac9437e..6a213b714 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -607,6 +607,16 @@ impl Bot { .await { warn!(target: "Bot/PairCode", "Timeout waiting for socket: {}", e); + // The request never happens, so `pair_with_code` never runs + // and never dispatches for it. Reported here instead: to a + // consumer waiting on a code this is the same fact as any + // other failure — none is coming — and leaving this one path + // silent would put the indefinite wait back. + client_for_pair.core.event_bus.dispatch(Event::PairingCodeError( + crate::types::events::PairingCodeError::builder() + .error(e.to_string()) + .build(), + )); return; } @@ -622,6 +632,10 @@ impl Bot { info!(target: "Bot/PairCode", "Pair code generated: {}", code); } Err(e) => { + // Only logged here: `pair_with_code` already dispatched + // `Event::PairingCodeError`, which is what a consumer + // observes — this task is detached, so returning the + // error is not an option. warn!(target: "Bot/PairCode", "Failed to request pair code: {}", e); } } @@ -996,6 +1010,36 @@ impl BotBuilder { }) } + /// Run `handler` when a pair-code request fails, so no code will be issued + /// ([`Event::PairingCodeError`]). + /// + /// The counterpart to [`BotBuilder::on_pair_code`], and the only way to + /// observe the failure of a [`BotBuilder::with_pair_code`] request: that one + /// runs in a detached task, so its `Err` reaches no caller. + /// + /// Branch on `err.rejection` rather than the message. + /// [`PairCodeRejection::is_throttled`](crate::pair_code::PairCodeRejection::is_throttled) + /// is the case to slow down for — re-requesting on the original schedule + /// spends more of the budget the server just refused — and `err.backoff` + /// carries the server's own delay when it named one. + pub fn on_pair_code_error(self, handler: F) -> Self + where + F: Fn(crate::types::events::PairingCodeError, Arc) -> Fut + Send + Sync + 'static, + Fut: Future + Send + 'static, + { + self.on_event_for(&[EventKind::PairingCodeError], move |event, client| { + let fut = match &*event { + Event::PairingCodeError(e) => Some(handler(e.clone(), client)), + _ => None, + }; + async move { + if let Some(fut) = fut { + fut.await + } + } + }) + } + /// Run `handler` when the server asks the companion to refresh an /// in-progress pairing code ([`Event::PairingCodeRefresh`]). The `bool` is /// `force_manual`. The typical reaction is to request a fresh code via @@ -1157,6 +1201,12 @@ impl BotBuilder { /// (see [`BotBuilder::on_pair_code`]). This runs concurrently with QR code /// pairing - whichever completes first wins. /// + /// The request runs in a detached task, so a failure cannot be returned to + /// the caller: it arrives as `Event::PairingCodeError` instead (see + /// [`BotBuilder::on_pair_code_error`]). Subscribe to it if the consumer must + /// distinguish "still waiting for the user" from "no code is coming" — a + /// rate-limited request is otherwise indistinguishable from the former. + /// /// # Example /// ```rust,ignore /// use whatsapp_rust::pair_code::PairCodeOptions; diff --git a/src/pair_code.rs b/src/pair_code.rs index d0159af1c..92ba803cf 100644 --- a/src/pair_code.rs +++ b/src/pair_code.rs @@ -68,7 +68,7 @@ use wacore_binary::Jid; use wacore_binary::{NodeContent, NodeContentRef, NodeRef}; pub use wacore::companion_reg::{CompanionOs, CompanionWebClientType}; -pub use wacore::pair_code::{PairCodeError, PairCodeOptions}; +pub use wacore::pair_code::{PairCodeError, PairCodeOptions, PairCodeRejection}; /// Errors raised by the high-level pair-code flow. /// @@ -90,10 +90,76 @@ pub enum PairError { /// OS, so a display-shaped rejection is already ruled out — see /// [`wacore::companion_reg::CompanionOs`].) Any server `backoff` hint is /// preserved on the wrapped [`IqError`]. - #[error("pair-code IQ request failed")] + /// + /// Renders what it wraps, per the [rendering + /// convention](crate::error#rendering) — so the code and text reach a log + /// line that only prints the error, without the reader having to reach for + /// the `Debug` form. + #[error("{0}")] RequestFailed(#[from] IqError), } +// Imported inside each body, not at module scope: `ErrorChainExt::as_dyn_error` +// would then be ambiguous with thiserror's own `AsDynError` for every `#[from]` +// in this module. +impl PairError { + /// How the server refused the request, as a status to branch on. + /// + /// `None` when nothing was refused: local validation, no connection, or a + /// request that went unanswered. Prefer this to matching the message, which + /// is not a stable surface. + /// + /// Classified from the `code` and `text` together, so a pairing WA Web + /// would not accept yields `None` rather than the named arm — see + /// [`PairCodeRejection::from_server`]. The refused-but-unclassifiable case + /// is therefore indistinguishable here from "nothing was refused"; both + /// mean the same thing to a consumer, which is that there is no typed + /// status to act on and the message is all there is. + pub fn rejection(&self) -> Option { + use crate::error::ErrorChainExt; + self.server_rejection() + .and_then(|rejection| PairCodeRejection::from_server(rejection.code, rejection.text)) + } + + /// Whether this request lost the pairing flow to someone else rather than + /// ending it — so its failure says nothing about whether a code arrives. + /// + /// These are the failures [`Event::PairingCodeError`] must stay silent for, + /// because its meaning is "no code is coming" and here that is not what + /// happened: + /// + /// - [`PairCodeError::CodeAlreadyOutstanding`] — refused *because* an + /// earlier code is still inside its validity window. That code is on + /// screen and may yet be entered; the consumer already has it from the + /// [`Event::PairingCode`] that minted it. + /// - [`PairCodeError::Cancelled`] — the caller withdrew this request via + /// [`Client::cancel_pair_code`], and a replacement may already own the + /// slot. Reporting the *predecessor* would let a consumer read the live + /// replacement as failed and tear down a code that is about to arrive. + /// + /// Both are consequences of something the caller did, so neither is news to + /// them, and a direct caller still receives the `Err` either way. + pub fn lost_the_flow_to_another_request(&self) -> bool { + matches!( + self, + Self::PairCode(PairCodeError::CodeAlreadyOutstanding { .. } | PairCodeError::Cancelled) + ) + } + + /// How long the server asked the client to wait before retrying, from the + /// `backoff` attribute. + /// + /// Usually `None` — the server rarely populates it on this request, and WA + /// Web never reads it — but a value here is the server naming its own delay, + /// which beats an interval the consumer picked. + pub fn backoff(&self) -> Option { + use crate::error::ErrorChainExt; + self.server_rejection() + .and_then(|rejection| rejection.backoff) + .map(|secs| std::time::Duration::from_secs(u64::from(secs))) + } +} + impl Client { /// Initiates pair code authentication as an alternative to QR code pairing. /// @@ -154,6 +220,62 @@ impl Client { pub async fn pair_with_code( self: &Arc, options: PairCodeOptions, + ) -> Result { + // The failure is dispatched here rather than at each `return Err` + // below: stage 1 fails from a dozen places, and what a consumer needs + // from all of them is the same single fact — no code is coming. Wrapping + // the flow is also what stops a *later* early return from going + // unreported. `BotBuilder::with_pair_code` depends on it having no gaps, + // because it drives this from a detached task whose `Err` reaches nobody. + // + // Mirrors the success path, which likewise both returns the code and + // dispatches `Event::PairingCode`; a direct caller sees the failure + // twice, and a `with_pair_code` consumer sees it at all. + match self.pair_with_code_inner(options).await { + Ok(code) => Ok(code), + Err(e) if self.failure_is_not_this_flows_to_report(&e).await => Err(e), + Err(e) => { + self.core.event_bus.dispatch(Event::PairingCodeError( + crate::types::events::PairingCodeError::builder() + .maybe_rejection(e.rejection()) + .maybe_backoff(e.backoff()) + .error(e.to_string()) + .build(), + )); + Err(e) + } + } + } + + /// Whether reporting this failure would speak for a flow that is not the + /// failed request's to speak for. + /// + /// The event means "no code is coming", so the question is not *how* the + /// request failed but whether a code is nonetheless on its way. Answered on + /// the state, not on the error variant: the variants that can reach here + /// while a flow is live are open-ended — a duplicate request, a withdrawn + /// one, its IQ timing out, or a second caller simply passing a bad phone + /// number while the first code is still on screen — and enumerating them + /// has already been wrong four times. + /// + /// [`PairCodeError::Cancelled`] is still matched explicitly, because a + /// cancellation with no replacement leaves the slot idle: nothing is live, + /// yet the caller asked for exactly this and does not need telling. + async fn failure_is_not_this_flows_to_report(self: &Arc, e: &PairError) -> bool { + if e.lost_the_flow_to_another_request() { + return true; + } + // A failing request that still owned the slot has released it by now, so + // an outstanding flow here belongs to somebody else. + self.pair_code_state + .lock() + .await + .is_outstanding(wacore::time::now_secs()) + } + + async fn pair_with_code_inner( + self: &Arc, + options: PairCodeOptions, ) -> Result { // Strip non-digit characters from phone number (allows "+1-555-123-4567" format) let phone_number: String = options @@ -325,6 +447,16 @@ impl Client { let response = match self.send_iq(query).await { Ok(response) => response, Err(e) => { + // The same ownership recheck the success path does below, and + // for the same reason. A 30s IQ timeout easily outlives a + // `cancel_pair_code` plus its replacement, and reporting this + // request's transport failure would then put an uncorrelated + // error on the bus against the flow that now owns the slot. + // Losing the slot outranks how this request happened to end. + if !self.owns_code_claim(claim).await { + claim_guard.armed = false; + return Err(PairCodeError::Cancelled.into()); + } claim_guard.release_now().await; return Err(e.into()); } @@ -853,6 +985,352 @@ async fn handle_refresh_code(client: &Arc, reg_node: &NodeRef<'_>) -> bo mod tests { use super::*; + /// Pin the five arms against `WASmaxInMdIqMixinErrors.parseIqMixinErrors`, + /// the complete set WA Web's `companion_hello` response parser accepts, so a + /// renumber can't silently break a consumer's branching. + #[test] + fn rejection_codes_match_wa_web() { + assert_eq!(PairCodeRejection::BadRequest.code(), 400); + assert_eq!(PairCodeRejection::Forbidden.code(), 403); + assert_eq!(PairCodeRejection::RateOverlimit.code(), 429); + assert_eq!(PairCodeRejection::FeatureNotAvailable.code(), 452); + assert_eq!(PairCodeRejection::InternalServerError.code(), 500); + // A code outside WA Web's set keeps its number rather than collapsing + // into a named arm. + assert_eq!( + PairCodeRejection::from(418), + PairCodeRejection::Unknown(418) + ); + } + + /// `CodeAlreadyOutstanding` is the one failure that must *not* dispatch: a + /// code is still live, the consumer already has it from the `PairingCode` + /// that minted it, and an error event would say the opposite while inviting + /// a retry loop nothing but `cancel_pair_code` or expiry can break. + #[tokio::test] + async fn an_outstanding_code_is_not_reported_as_a_failure() { + let client = create_test_client().await; + let collector = Arc::new(crate::test_utils::TestEventCollector::default()); + client.subscribe_handler(collector.clone()).detach(); + + // Park a live code in the slot so the next request is the duplicate. + let now = wacore::time::now_secs(); + *client.pair_code_state.lock().await = PairCodeState::RequestingCode { + code_generation_ts: now, + claim: wacore::pair_code::PairCodeClaim::next(), + }; + + let err = client + .pair_with_code(PairCodeOptions { + phone_number: "15551234567".to_string(), + ..Default::default() + }) + .await + .expect_err("a second code must be refused while one is live"); + assert!( + err.lost_the_flow_to_another_request(), + "expected CodeAlreadyOutstanding, got: {err:?}" + ); + + // Let any dispatch that was going to happen get through. + tokio::task::yield_now().await; + assert!( + !collector + .events() + .iter() + .any(|e| matches!(&**e, Event::PairingCodeError(_))), + "a still-live code must not be reported as 'no code is coming'" + ); + } + + /// A request cancelled while its `companion_hello` is in flight must not + /// report either. It resolves *after* a replacement may already own the + /// slot, so the event would be uncorrelated with the flow actually running + /// and could make a consumer tear down a live code. + #[tokio::test] + async fn a_superseded_request_is_not_reported_as_a_failure() { + let (client, transport) = create_iq_test_client().await; + let collector = Arc::new(crate::test_utils::TestEventCollector::default()); + client.subscribe_handler(collector.clone()).detach(); + + let pending = { + let client = client.clone(); + tokio::spawn(async move { client.pair_with_code(options()).await }) + }; + poll_until("the companion_hello to be on the wire", || { + !transport.sent().is_empty() + }) + .await; + + client.cancel_pair_code().await; + answer_companion_hello(&client, &transport, 0, b"3@2:late").await; + + let err = pending + .await + .expect("the pair-code task should not panic") + .expect_err("a cancelled request must not report a usable code"); + assert!( + err.lost_the_flow_to_another_request(), + "expected Cancelled, got {err:?}" + ); + + tokio::task::yield_now().await; + assert!( + !collector + .events() + .iter() + .any(|e| matches!(&**e, Event::PairingCodeError(_))), + "a withdrawn request must not report against the flow that replaced it" + ); + } + + /// The same suppression must hold when the withdrawn request ends in an *IQ + /// failure* rather than a late success. A 30 s timeout or a server rejection + /// easily outlives a `cancel_pair_code` plus its replacement, and reporting + /// this request's transport failure would then land on the flow that now + /// owns the slot. + #[tokio::test] + async fn a_withdrawn_request_reports_cancellation_not_its_iq_failure() { + let (client, transport) = create_iq_test_client().await; + let collector = Arc::new(crate::test_utils::TestEventCollector::default()); + client.subscribe_handler(collector.clone()).detach(); + + let pending = { + let client = client.clone(); + tokio::spawn(async move { client.pair_with_code(options()).await }) + }; + poll_until("the companion_hello to be on the wire", || { + !transport.sent().is_empty() + }) + .await; + + client.cancel_pair_code().await; + + // Refuse the withdrawn request's IQ, rather than answering it. + let hello = crate::test_utils::decode_sent_iq(&transport, 0).await; + let id = hello + .get() + .attrs() + .optional_string("id") + .expect("companion_hello carries an id") + .into_owned(); + let refusal = NodeBuilder::new("iq") + .attrs([ + ("from", "s.whatsapp.net".to_string()), + ("type", "error".to_string()), + ("id", id.clone()), + ]) + .children([NodeBuilder::new("error") + .attrs([ + ("code", "429".to_string()), + ("text", "rate-overlimit".to_string()), + ]) + .build()]) + .build(); + crate::test_utils::answer_iq(&client, &id, &refusal).await; + + let err = pending + .await + .expect("the pair-code task should not panic") + .expect_err("a withdrawn request must not report a usable code"); + assert!( + err.lost_the_flow_to_another_request(), + "losing the slot outranks how the request ended, got {err:?}" + ); + + tokio::task::yield_now().await; + assert!( + !collector + .events() + .iter() + .any(|e| matches!(&**e, Event::PairingCodeError(_))), + "a withdrawn request's IQ failure must not report against its replacement" + ); + } + + /// Validation runs before the outstanding-flow check, so a second caller + /// with a bad number fails as `PhoneNumberTooShort` and never reaches the + /// suppressed variants. It must still stay silent while a code is live — + /// which is why the suppression asks the state, not the error. + #[tokio::test] + async fn a_validation_failure_beside_a_live_code_is_not_reported() { + let client = create_test_client().await; + let collector = Arc::new(crate::test_utils::TestEventCollector::default()); + client.subscribe_handler(collector.clone()).detach(); + + *client.pair_code_state.lock().await = PairCodeState::RequestingCode { + code_generation_ts: wacore::time::now_secs(), + claim: wacore::pair_code::PairCodeClaim::next(), + }; + + let err = client + .pair_with_code(PairCodeOptions { + phone_number: "123".to_string(), + ..Default::default() + }) + .await + .expect_err("a 3-digit number must be refused"); + assert!( + matches!(err, PairError::PairCode(PairCodeError::PhoneNumberTooShort)), + "validation must still win the race it already wins, got {err:?}" + ); + + tokio::task::yield_now().await; + assert!( + !collector + .events() + .iter() + .any(|e| matches!(&**e, Event::PairingCodeError(_))), + "a live code must not be reported as failed by an unrelated bad request" + ); + } + + /// WA Web asserts `code` and `text` as a pair and falls to its generic + /// error path when they disagree, so a contradicting text must not keep + /// reading as the named arm. + #[test] + fn a_contradicting_text_yields_no_classification() { + let pe: PairError = IqError::ServerError { + code: 429, + text: "something-else".into(), + error_type: None, + backoff: None, + } + .into(); + + assert_eq!( + pe.rejection(), + None, + "a pairing WA Web would reject must not drive throttle handling" + ); + // The code is still recoverable from the rendering, so refusing to + // classify does not lose it. + assert!(pe.to_string().contains("429"), "got: {pe}"); + } + + /// An absent `text` is not a contradiction. Deliberately laxer than WA Web: + /// demoting a bare 429 would clear `is_throttled` and put the issue's silent + /// failure back for the one refusal that most needs acting on. + #[test] + fn an_absent_text_still_classifies_by_code() { + let pe: PairError = IqError::ServerError { + code: 429, + text: String::new(), + error_type: None, + backoff: None, + } + .into(); + + assert_eq!(pe.rejection(), Some(PairCodeRejection::RateOverlimit)); + assert!(pe.rejection().is_some_and(PairCodeRejection::is_throttled)); + } + + /// The whole point of the typed status: a 429 is recoverable as + /// `RateOverlimit` without matching the message. + #[test] + fn rate_overlimit_is_recoverable_as_a_typed_status() { + let pe: PairError = IqError::ServerError { + code: 429, + text: "rate-overlimit".into(), + error_type: None, + backoff: Some(30), + } + .into(); + + assert_eq!(pe.rejection(), Some(PairCodeRejection::RateOverlimit)); + assert_eq!(pe.backoff(), Some(std::time::Duration::from_secs(30))); + assert!( + pe.rejection().is_some_and(PairCodeRejection::is_throttled), + "429 must read as throttled" + ); + // `RequestFailed` renders what it wraps, so a log line that prints only + // the error still names the refusal. + assert!( + pe.to_string().contains("429") && pe.to_string().contains("rate-overlimit"), + "Display should carry the server's code and text, got: {pe}" + ); + } + + /// `feature-not-available` is the one refusal that retrying cannot fix — it + /// must not read as throttled, or a consumer would back off forever instead + /// of falling back to the QR code the way WA Web does. + #[test] + fn feature_not_available_is_not_throttled() { + let pe: PairError = IqError::ServerError { + code: 452, + text: "feature-not-available".into(), + error_type: None, + backoff: None, + } + .into(); + + assert_eq!(pe.rejection(), Some(PairCodeRejection::FeatureNotAvailable)); + assert!(!PairCodeRejection::FeatureNotAvailable.is_throttled()); + assert_eq!(pe.backoff(), None); + } + + /// A failure that never reached the server has no status to report, so + /// `rejection` stays `None` rather than inventing one. + #[test] + fn local_failure_reports_no_rejection() { + let pe: PairError = PairCodeError::PhoneNumberTooShort.into(); + assert_eq!(pe.rejection(), None); + assert_eq!(pe.backoff(), None); + } + + /// The regression the event exists for: a failed request must be observable + /// on the bus, not only through the `Err` that + /// `BotBuilder::with_pair_code`'s detached task throws away. + /// + /// Uses a validation failure because it needs no server, and it covers the + /// harder half of the guarantee: the dispatch wraps the whole flow, so even + /// a path that returns before the IQ is built still reports. + #[tokio::test] + async fn failed_request_dispatches_pairing_code_error() { + let client = create_test_client().await; + let collector = Arc::new(crate::test_utils::TestEventCollector::default()); + client.subscribe_handler(collector.clone()).detach(); + + let err = client + .pair_with_code(PairCodeOptions { + phone_number: "123".to_string(), + ..Default::default() + }) + .await + .expect_err("a 3-digit number must be refused"); + assert!(matches!( + err, + PairError::PairCode(PairCodeError::PhoneNumberTooShort) + )); + + poll_until("a PairingCodeError to reach the bus", || { + collector + .events() + .iter() + .any(|e| matches!(&**e, Event::PairingCodeError(_))) + }) + .await; + + let events = collector.events(); + let dispatched = events + .iter() + .find_map(|e| match &**e { + Event::PairingCodeError(e) => Some(e.clone()), + _ => None, + }) + .expect("just polled for it"); + assert_eq!( + dispatched.rejection, None, + "a local validation failure never reached the server" + ); + assert_eq!(dispatched.backoff, None); + assert!( + dispatched.error.contains("too short"), + "the message should say what failed, got: {}", + dispatched.error + ); + } + #[test] fn pair_error_request_failed_preserves_iq_source() { let iq = IqError::ServerError { diff --git a/wacore/src/pair_code.rs b/wacore/src/pair_code.rs index 1482e77b6..f518cad3b 100644 --- a/wacore/src/pair_code.rs +++ b/wacore/src/pair_code.rs @@ -620,6 +620,130 @@ impl PairCodeUtils { } } +/// How the server refused a `companion_hello`, as a matchable status. +/// +/// The five named variants are the complete set WA Web's own response parser +/// accepts (`WASmaxInMdIqMixinErrors.parseIqMixinErrors`, reached from +/// `WASmaxInMdCompanionHelloResponseError`); anything else makes its RPC throw +/// "unknown error". They exist so a consumer can branch on the refusal instead +/// of matching the formatted message, which is not a stable surface. +/// +/// The numbers are the `code` attribute, and each is the enum's whole wire form +/// — [`code()`](Self::code) is what `Serialize` emits and what `From` reads +/// back. WA Web pairs each code with a literal `text` +/// (`429`/`rate-overlimit`, `452`/`feature-not-available`, …) and rejects a +/// response whose two disagree, so construct these through +/// [`from_server`](Self::from_server) rather than from a code alone: it is the +/// only constructor that sees both attributes, and the only one that can decline +/// to classify. +/// +/// WA Web branches on exactly two of them (`DevicePhoneNumberCodeScreen`, on +/// `CompanionHelloError.type.name`): [`RateOverlimit`](Self::RateOverlimit) +/// becomes "too many attempts, try again later" and +/// [`FeatureNotAvailable`](Self::FeatureNotAvailable) becomes "not available to +/// you yet, link with QR code instead". The rest share a generic "try again or +/// link with the QR code". In every case it resets the linking flow and waits +/// for the person to act — it never retries on its own, and never reads the +/// `backoff` hint, so treat that value as the server's advice rather than a +/// schedule WA Web is known to follow. +#[derive(Debug, Clone, Copy, PartialEq, Eq, crate::WireEnum)] +#[wire(kind = "int")] +pub enum PairCodeRejection { + /// The request was malformed — **or** throttled for this phone number: the + /// server reuses `bad-request` for its per-number pair-code limit rather + /// than answering `rate-overlimit`. So this is not reliably a permanent + /// failure; see [`PairCodeRejection::is_throttled`]. + #[wire = 400] + BadRequest, + #[wire = 403] + Forbidden, + /// The connection is asking for codes too fast. The server states the rate + /// is too high; the only correct response is to slow down. + #[wire = 429] + RateOverlimit, + /// Phone-number linking is not enabled for this account. Retrying will not + /// change that — WA Web sends the user to the QR code instead. + #[wire = 452] + FeatureNotAvailable, + #[wire = 500] + InternalServerError, + /// A `code` outside the set WA Web accepts. Its own RPC would raise + /// "unknown error" here; we keep the number so a consumer can log it and a + /// server-side addition is visible rather than silently reshaped. + #[wire_fallback] + Unknown(i32), +} + +impl PairCodeRejection { + /// Whether this refusal is the server rate-limiting the request. + /// + /// True for [`RateOverlimit`](Self::RateOverlimit) and + /// [`BadRequest`](Self::BadRequest), because the server throttles pair-code + /// requests per phone number under `bad-request` instead of + /// `rate-overlimit`. That makes the predicate deliberately wider than the + /// literal 429: a `bad-request` may equally be genuinely invalid content, + /// and the two are indistinguishable on the wire. Treat a true here as + /// "back off, then retry at most a bounded number of times" — not as proof + /// the request would ever succeed. + pub fn is_throttled(self) -> bool { + matches!(self, Self::RateOverlimit | Self::BadRequest) + } + + /// The `text` WA Web pairs with this code; `None` for + /// [`Unknown`](Self::Unknown), which has no expected pairing. + pub fn text(self) -> Option<&'static str> { + Some(match self { + Self::BadRequest => "bad-request", + Self::Forbidden => "forbidden", + Self::RateOverlimit => "rate-overlimit", + Self::FeatureNotAvailable => "feature-not-available", + Self::InternalServerError => "internal-server-error", + Self::Unknown(_) => return None, + }) + } + + /// Classify a server `` from both of its attributes, or `None` when + /// the two disagree and no classification is honest. + /// + /// WA Web asserts the pair (`literal(attrInt, …, "code", 429)` beside + /// `literal(attrString, …, "text", "rate-overlimit")`) and drops to its + /// generic error path when they disagree, so a changed pairing must not keep + /// reading as the named arm. + /// + /// `None` rather than `Unknown(code)` for that case, because `Unknown` could + /// not carry it: the wire form of this enum **is** `code()`, so + /// `Unknown(429)` serializes to `429` and rehydrates as + /// [`RateOverlimit`](Self::RateOverlimit) — a consumer that persisted or + /// forwarded the value would get the demotion silently undone and apply + /// throttling anyway. There is no in-band value that both records the code + /// and refuses to alias the arm it came from. The code is not lost: the + /// caller still has the error's own rendering, which names it. + /// + /// An **absent** `text` is not a contradiction, and the code alone decides. + /// Deliberately laxer than WA Web, which would reject it: refusing to + /// classify a bare `429` would also clear + /// [`is_throttled`](Self::is_throttled), turning the one refusal a consumer + /// most needs to act on back into a silent one. A missing attribute is not + /// evidence that the code means something else. + pub fn from_server(code: u16, text: &str) -> Option { + let by_code = Self::from(i32::from(code)); + match by_code.text() { + Some(expected) if !text.is_empty() && text != expected => None, + _ => Some(by_code), + } + } +} + +impl core::fmt::Display for PairCodeRejection { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + // Rendered as the stanza it came from, so a log line reads the same way. + match self.text() { + Some(text) => write!(f, "{text} ({})", self.code()), + None => write!(f, "unknown ({})", self.code()), + } + } +} + /// Errors raised by wacore-side pair-code validation, key derivation, and /// protocol-bundle building. The high-level crate wraps this in /// `whatsapp_rust::pair_code::PairError` and adds an IQ-failure variant for the diff --git a/wacore/src/types/events.rs b/wacore/src/types/events.rs index cebcb7fb0..5c75f7cac 100755 --- a/wacore/src/types/events.rs +++ b/wacore/src/types/events.rs @@ -273,6 +273,7 @@ pub enum EventKind { PairPasskeyError, ServerAck, PairingQrCodesExhausted, + PairingCodeError, // When adding a variant, mind the 128-kind ceiling below (EventInterest packs // each discriminant as a bit in a u128) and keep the guard pointing at the // last variant. @@ -286,7 +287,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::PairingQrCodesExhausted as u8) < EventKind::CAPACITY); +const _: () = assert!((EventKind::PairingCodeError as u8) < EventKind::CAPACITY); /// A set of [`EventKind`]s a handler wants delivered. Producers can query the /// aggregate interest before building expensive payloads, and dispatch avoids @@ -818,6 +819,7 @@ pub enum Event { PairingQrCode(PairingQrCode), PairingCode(PairingCode), PairingCodeRefresh(PairingCodeRefresh), + PairingCodeError(PairingCodeError), PairingQrCodesExhausted(PairingQrCodesExhausted), QrScannedWithoutMultidevice(QrScannedWithoutMultidevice), ClientOutdated(ClientOutdated), @@ -995,6 +997,7 @@ impl Event { Event::PairingQrCode(_) => EventKind::PairingQrCode, Event::PairingCode(_) => EventKind::PairingCode, Event::PairingCodeRefresh(_) => EventKind::PairingCodeRefresh, + Event::PairingCodeError(_) => EventKind::PairingCodeError, Event::PairingQrCodesExhausted(_) => EventKind::PairingQrCodesExhausted, Event::QrScannedWithoutMultidevice(_) => EventKind::QrScannedWithoutMultidevice, Event::ClientOutdated(_) => EventKind::ClientOutdated, @@ -1237,6 +1240,63 @@ pub struct PairingCodeRefresh { pub force_manual: bool, } +/// A phone-number pair-code request failed, so no code will be shown. +/// +/// The counterpart to [`PairingCode`] on the failure path, and the only surface +/// that reports it when pairing is driven by `BotBuilder::with_pair_code` — +/// that request runs in a detached task, so nothing returns its error to the +/// caller. `Client::pair_with_code` dispatches this in addition to returning +/// `Err`, matching how the success path both returns the code and emits +/// [`PairingCode`]. +/// +/// Fires for every failure, including local validation (a phone number that is +/// too short never reaches the server): a consumer waiting on a code needs to +/// learn that it is not coming, whatever the reason. [`rejection`](Self::rejection) +/// is what distinguishes the two — `None` means the request never got an answer +/// from the server. +/// +/// A claim the failed request itself took is released before this fires, so +/// nothing is left holding the flow and `pair_with_code` can be called again. +/// +/// Two failures do **not** arrive here, because for them a code may still be on +/// its way and this event would say the opposite — a consumer acting on it +/// would tear down a code that is about to arrive: +/// +/// - `CodeAlreadyOutstanding` — refused precisely because an earlier code is +/// still live, and the consumer already has it from the [`PairingCode`] that +/// minted it. Retrying is futile until `cancel_pair_code` runs or the window +/// closes. +/// - `Cancelled` — the caller withdrew this request, and a replacement may +/// already own the slot. A superseded request can return this *after* its +/// replacement started, so the event would be uncorrelated with the flow that +/// is actually running. +/// +/// Both follow from something the caller did, so neither is news, and a direct +/// caller still gets the `Err`. +/// +/// Whether to retry at all is the point of the fields: back off on +/// [`PairCodeRejection::is_throttled`](crate::pair_code::PairCodeRejection::is_throttled), +/// stop on +/// [`PairCodeRejection::FeatureNotAvailable`](crate::pair_code::PairCodeRejection::FeatureNotAvailable). +#[derive(Debug, Clone, Serialize, bon::Builder)] +#[non_exhaustive] +pub struct PairingCodeError { + /// The server's refusal, when it answered with one. `None` when the failure + /// was local (validation, no connection) or the request went unanswered + /// (timeout) — nothing was refused, so there is no status to report. + #[serde(skip_serializing_if = "Option::is_none")] + pub rejection: Option, + /// How long the server asked the client to wait, from the `backoff` + /// attribute. Usually absent — WA Web does not read it on this path — but + /// when present it is the server naming its own retry delay, which beats + /// any interval the consumer would pick. + #[serde(skip_serializing_if = "Option::is_none")] + pub backoff: Option, + /// The failure rendered for logs. Do not branch on it; use + /// [`rejection`](Self::rejection). + pub error: String, +} + /// The server's `` refs are used up: there is no QR left to /// render until the connection is re-established. /// @@ -1870,6 +1930,63 @@ mod tests { use buffa::Message; use waproto::whatsapp as wa; + /// A new kind must go at the end. The discriminant doubles as an + /// `EventInterest` bit index and is what a consumer persists or transmits, + /// so inserting one in the middle silently re-points every stored mask + /// after it at the wrong events. + /// + /// Pinned by value rather than by ordering: a spot check of the run's + /// start, the pair-code block a new kind is most tempting to sit inside, + /// and the two most-subscribed kinds past it. + #[test] + fn event_kind_discriminants_are_append_only() { + assert_eq!(EventKind::Connected as u8, 0); + assert_eq!(EventKind::PairingCode as u8, 6); + assert_eq!(EventKind::PairingCodeRefresh as u8, 7); + assert_eq!(EventKind::QrScannedWithoutMultidevice as u8, 8); + assert_eq!(EventKind::Messages as u8, 10); + assert_eq!(EventKind::Receipt as u8, 11); + + // The two already parked at the end for this same reason, and the + // newest past both. Pinned absolutely rather than as an offset from its + // neighbour: a relative check still passes when a kind is inserted + // *before* the pair, which shifts all three together. + assert_eq!(EventKind::ServerAck as u8, 57); + assert_eq!(EventKind::PairingQrCodesExhausted as u8, 58); + assert_eq!(EventKind::PairingCodeError as u8, 59); + } + + /// Every rejection a consumer can be handed must survive being persisted + /// and read back as itself. + /// + /// The wire form of `PairCodeRejection` is its `code()`, so an `Unknown` + /// carrying a *named* code would serialize to that code and rehydrate as the + /// named arm — silently upgrading a value we declined to classify. Nothing + /// may construct such a value; `from_server` returns `None` instead, and + /// this pins that the reachable ones round-trip. + #[test] + fn pair_code_rejections_do_not_alias_on_a_round_trip() { + use crate::pair_code::PairCodeRejection as R; + + for original in [ + R::BadRequest, + R::Forbidden, + R::RateOverlimit, + R::FeatureNotAvailable, + R::InternalServerError, + R::Unknown(418), + ] { + let json = serde_json::to_string(&original).expect("serializes"); + let back: R = serde_json::from_str(&json).expect("deserializes"); + assert_eq!(back, original, "{original:?} rehydrated as {back:?}"); + } + + // The aliasing that forced `from_server` to return `None`: kept as a + // live demonstration so the reason cannot be lost to a refactor. + assert_eq!(R::Unknown(429).code(), R::RateOverlimit.code()); + assert_eq!(R::from_server(429, "something-else"), None); + } + #[test] fn group_update_builder_defaults_additive_scalar_fields() { let update = GroupUpdate::builder()