feat!: overhaul the public bot API ahead of 1.0 - #852
Conversation
- Bot::run(self) is a single await and Bot::spawn() returns a BotHandle with graceful shutdown() - the builder pre-fills transport/HTTP/runtime from the default features, so only the backend is required - event handlers accumulate instead of silently replacing, with typed registrars (on_message, on_qr_code, on_pair_code, on_connected, on_logged_out) - MessageContext gains reply/reply_quoting, Client gains send_text, wa::Message gains text builders - whole-crate re-exports (wacore, wacore_binary, waproto) so git consumers need a single dependency - README quick start compiles again and is enforced as a doctest
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughRefactors Bot construction, adds multi-handler registration routed through a CallbackBusAdapter, changes lifecycle to consuming run()/spawn()/shutdown(), adds MessageContext reply helpers and Client::send_text, introduces a prelude and wacore re-exports, and updates examples/tests/README to the new builder/run patterns. ChangesBot Builder and Event System Refactor
Sequence DiagramsequenceDiagram
participant UserCode as Client app
participant BotBuilder
participant Bot
participant EventBus
participant CallbackBusAdapter
participant RegisteredHandler
UserCode->>BotBuilder: configure with_backend / on_* callbacks
BotBuilder->>Bot: build()
Bot->>EventBus: start_background()
Bot->>EventBus: register CallbackBusAdapter (with combined_interest)
EventBus->>CallbackBusAdapter: incoming Event
CallbackBusAdapter->>RegisteredHandler: spawn task for interested handler
RegisteredHandler->>Client: invoke user callback (MessageContext)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Note: ensure tests compiling against feature defaults and typestate transitions are validated; these changes alter public construction and runtime behavior and must be exercised by CI. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
1 issue found across 13 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
The background example calls tokio::signal::ctrl_c(), which a consumer copying the snippet would be missing (our doctests inherit the feature from the crate's own tokio dependency, so they could not catch it).
Why
A rigorous pass over our public API against serenity 0.12 and teloxide surfaced a set of first-mile problems worth fixing before 1.0, while breaking changes are still cheap.
The two biggest ones were embarrassing in hello world itself: the README quick start did not compile (the handler receives
Arc<Event>but the example matched onEvent, E0308, verified empirically), and the lifecycle requiredbot.run().await?.await?where the first.awaitand the first?were literally free of function (the oldrun_graphhad zero await points and no error path).On top of that, a survey of real consumers on GitHub (moltis, picobot, Papo, zeroclaw) showed every single one declaring 5 to 7 of our crates as separate git dependencies (
whatsapp-rust+wacore+wacore-binary+waproto+ both satellites), because the README taught imports from the satellite crates andwacore/wacore-binarywere not reachable through the main crate.What changed
Lifecycle.
Bot::run(self)now runs the bot on the current task with a single await, andBot::spawn(self)starts it in the background and returns aBotHandlewithclient(), a gracefulshutdown()(flushes the device snapshot, buffered receipts and message secrets viaClient::disconnect, then waits for the loop to exit), andabort()as a documented escape hatch. Awaiting the handle resolves to()instead of leakingfutures::channel::oneshot::Canceled. Dropping the handle still aborts the task (the e2e harness relies on that).Builder defaults. With the default cargo features, transport, HTTP client and runtime start as
Provided(Tokio WebSocket, ureq, Tokio), so only the backend has to be chosen; thewith_*setters are now available in every typestate, so they also work as overrides. The typestate markers got names (MissingBackend,MissingTransport,MissingHttpClient,MissingRuntime), so a missing-field compile error now says which field is missing.with_backendtakesimpl Backend + 'static(no caller-sideArc), withwith_backend_arcfor an already-shared backend.BotBuilderErrorlost its fakeOther(anyhow)variant and now carries the typedStore(StoreError)thatbuild()can actually produce.Event handlers. Registrations accumulate instead of silently replacing the previous handler. New typed registrars cover the common path:
on_message(receives a readyMessageContext),on_qr_code,on_pair_code,on_connected,on_logged_out, withon_event/on_event_forunchanged as the catch-all.with_event_handlerregisters a struct-basedEventHandlerdirectly on the bus for stateful handlers (kills the clone-dance that closure captures force on consumers). The bus still skips materializing events nobody wants: the adapter registers the union of all handler interests (EventInterest::union) and filters per handler at dispatch.Messaging sugar.
wa::Message::text(...)andwa::Message::text_with_context(...)via the newMessageBuilderExtinwacore::proto_helpers,ctx.reply(...)andctx.reply_quoting(...)onMessageContext, andClient::send_text(jid, text). The raw protobuf path stays untouched for advanced cases.Single-dependency consumption. The crate now re-exports
wacore,wacore_binaryandwaprotowholesale, so every path is reachable aswhatsapp_rust::wacore::...and a git consumer needs exactly one dependency line.UreqHttpClientmoved from the misplacedwhatsapp_rust::transportre-export towhatsapp_rust::http. A newwhatsapp_rust::preludecovers the common bot path in one import (includingwaas the protobuf alias).Docs that cannot rot. The README was rewritten around the new API (quick start is one dependency plus tokio) and is now included as crate docs via
#![doc = include_str!], so both README examples compile as doctests on every CI run. The demomain.rsshowcases the typed registrars,ctx.reply_quoting, and ctrl-c handling viahandle.shutdown().Breaking changes and migration
bot.run().await?.await?becomesbot.run().await(foreground) orlet handle = bot.spawn();(background, what Veloz-style consumers want).runconsumes the bot; grabbot.client()before, or usehandle.client()..with_backend(arc_backend)becomes.with_backend_arc(arc_backend); plain stores can drop theArc::newentirely.use whatsapp_rust::transport::UreqHttpClientbecomesuse whatsapp_rust::http::UreqHttpClient.bot::Missingwas replaced by the named markers; code that spelled out builder types must update.BotBuilderError::Otheris gone; the only variant isStore(StoreError).The send/connect anyhow-to-typed-errors migration from the same review is intentionally staged as a follow-up PR to keep this one reviewable.
Validation
cargo clippy --workspace --all-targets -- -D warningsclean (also fixed a latentchrono::Local::nowdisallowed-method hit in the benchmark example that workspace-level feature unification exposes).cargo test -p whatsapp-rust -p wacore: 1769 unit tests plus doctests green, including the two README examples as compile-checked doctests.cargo build -p whatsapp-rust --lib --target wasm32-unknown-unknown --no-default-features --features debug-diagnosticsbuilds (exercises the no-defaults typestate path).