Skip to content

docs: correct timeout and blocking-call documentation; harden derive macro paths - #126

Merged
hiking90 merged 1 commit into
mainfrom
docs/review-fix-timeouts-derive
Sep 18, 2026
Merged

hiking90 merged 1 commit into
mainfrom
docs/review-fix-timeouts-derive

Conversation

@hiking90

Copy link
Copy Markdown
Owner

A review pass over src/ and rsactor-derive/ found documentation that contradicts what the code does, and two hygiene/validation gaps in the derive macros. Behaviour of the library itself is unchanged; the derive changes alter which handler signatures compile.

Timeouts can run a handler twice

ask_timeout_inner and ask_priority_inner wrap admission and the reply wait in a single tokio::time::timeout. The runtime loop does not check the reply sender's is_closed() before dispatching an envelope (that check exists only in the shutdown drain), so a deadline that expires after admission leaves the handler running and only discards the reply.

Error::is_retryable nevertheless listed ask-side Timeout as retryable with no caveat. Changes in src/error.rs and src/actor_ref.rs:

  • is_retryable docs: an ask-side Timeout means the original request may still be running or already finished — retry only idempotent messages. A tell-side Timeout means admission failed, so resending cannot run the handler twice.
  • ask_with_timeout / ask_priority: state that the timeout ends the caller's wait only.
  • debugging_tips: tell-side timeouts (Tell, TellPriority, BlockingTell, BlockingTellPriority) get their own arm. They previously received "use tell instead", which is meaningless for a tell.
  • The is_retryable example retried on tell, which never returns a retryable error (it awaits capacity and fails only with Error::Send), so the retry branch was unreachable. It now uses tell_with_timeout.

The return value of is_retryable is unchanged; changing the dispatch path would not help an already-running handler anyway.

Blocking calls and wait_stopped

  • blocking_tell_priority / blocking_ask_priority warned of an "unrecoverable hang" when called from the actor's own handler. Their timeout is mandatory and bounds the wait, so the loop stalls for timeout and the call returns Error::Timeout. blocking_tell / blocking_ask now separate timeout: None (permanent hang) from Some(timeout).
  • wait_stopped(&self) holds a strong reference, so it keeps the actor alive against ref-drop termination; the first sentence said the opposite. "(after on_stop ran)" is narrowed: sender.closed() also resolves on the on_start-failure and panic-unwind paths, where on_stop does not run.

Derive macros (rsactor-derive)

  • Receiver check. receiver.mutability is the mut token, so by-value mut self passed and self: &mut Self was rejected. The check now inspects receiver.ty for a &mut reference. Verified in a probe crate: self: &mut Self compiles and runs; mut self is rejected at the receiver span.
  • Absolute paths. Expansions now use ::rsactor::Actor, ::rsactor::ActorRef, ::rsactor::Message, ::rsactor::__log_handler_error, ::core::result::Result::{Ok, Err}, ::core::convert::Infallible and ::core::any::type_name. Verified in a probe crate that both #[derive(Actor)] and #[message_handlers] compile in a scope containing mod rsactor {} and use Shadow::* (a local enum with Ok/Err variants).

Other

  • src/handler.rs module example moved actor_b into .into() and then used it (E0382). It now converts a clone, keeping both the From<&ActorRef<T>> and From<ActorRef<T>> examples.

Verification

Check Result
cargo fmt --check clean
cargo clippy --all-targets --all-features -- -D warnings clean
cargo test --workspace --all-features 448 passed, 0 failed
RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features clean

Limits

  • Renaming the dependency (package = "rsactor" under another name) still breaks the ::rsactor paths. Supporting it needs proc-macro-crate, which is a separate decision.
  • A follow-up review of these changes reported smaller gaps that this PR does not address (not independently verified here):
    • the receiver check does not look at the referent, so self: &mut Box<Self> passes validation and fails later with E0599;
    • the third-parameter check accepts &mut ActorRef<Self>, which fails later with E0308;
    • the new tell-side timeout tip suggests a larger mailbox capacity even for TellPriority, whose slot is fixed at 1;
    • the blocking self-call warning describes a stalled loop, but on a current_thread runtime the call panics and the actor terminates.
  • The panic-unwind record wording fix that came out of the same review went to feat(actor): name the actor in an error record when its task ends during a panic unwind #125, since that text exists only on that branch. The logging-docs finding from the review was already resolved on main by feat(logging): add log feature bridging tracing events to the log crate #123.

🤖 Generated with Claude Code

…macro paths

- Document that ask timeouts do not cancel admitted messages; retry only
  idempotent messages, and give tell timeouts their own error suggestions
- Clarify blocking_* self-call behavior with a timeout (stall, then Timeout)
- Fix wait_stopped docs on ref-drop termination and on_stop reachability
- Use fully qualified ::rsactor/::core paths in derive macro expansions
- Validate handler receivers by type so self: &mut Self is accepted and
  by-value mut self is rejected
- Fix handler.rs doc example that moved actor_b
@hiking90
hiking90 merged commit 6d8d4ed into main Sep 18, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant