Skip to content

Commit

Permalink
Error reporting QoL
Browse files Browse the repository at this point in the history
- make Actor::Error Display in addition to Debug
- print handle() (and friends) errors using `{:#}` rather than `{:?}`
  - this hides the backtrace print of anyhow errors
- differentiate handle() (and friends) errors while shutting down vs. running and log them with lower severity
- make it more clear and less spammy when trying to shut down a system while it is already shutting down
  • Loading branch information
strohel committed Aug 13, 2024
1 parent 4ea3712 commit 0e10b57
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 40 deletions.
2 changes: 1 addition & 1 deletion benches/benches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ struct ChainLink {

impl Actor for ChainLink {
type Context = Context<Self::Message>;
type Error = ();
type Error = String;
type Message = u64;

fn name() -> &'static str {
Expand Down
92 changes: 55 additions & 37 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
//! struct TestActor {}
//! impl Actor for TestActor {
//! type Context = Context<Self::Message>;
//! type Error = ();
//! type Error = String;
//! type Message = usize;
//!
//! fn name() -> &'static str {
//! "TestActor"
//! }
//!
//! fn handle(&mut self, _context: &mut Self::Context, message: Self::Message) -> Result<(), ()> {
//! fn handle(&mut self, _context: &mut Self::Context, message: Self::Message) -> Result<(), String> {
//! println!("message: {}", message);
//!
//! Ok(())
Expand Down Expand Up @@ -608,35 +608,52 @@ impl System {
},
Received::Message(msg) => {
trace!("[{}] message received by {}", system_handle.name, A::name());
if let Err(err) = actor.handle(context, msg) {
error!(
"[{}] {} handle error: {:?}, shutting down.",
system_handle.name,
A::name(),
err
);
let _ = system_handle.shutdown();

if let Err(error) = actor.handle(context, msg) {
Self::report_error_shutdown(system_handle, A::name(), "handle", error);
return;
}
},
Received::Timeout => {
let deadline = context.receive_deadline.take().expect("implied by timeout");
if let Err(err) = actor.deadline_passed(context, deadline) {
error!(
"[{}] {} deadline_passed error: {:?}, shutting down.",
system_handle.name,
if let Err(error) = actor.deadline_passed(context, deadline) {
Self::report_error_shutdown(
system_handle,
A::name(),
err
"deadline_passed",
error,
);
let _ = system_handle.shutdown();

return;
}
},
}
}
}

fn report_error_shutdown(
system_handle: &SystemHandle,
actor_name: &str,
method_name: &str,
error: impl std::fmt::Display,
) {
let is_running = match *system_handle.system_state.read() {
SystemState::Running => true,
SystemState::ShuttingDown | SystemState::Stopped => false,
};

let system_name = &system_handle.name;

// Note that the system may have transitioned from running to stopping (but not the other
// way around) in the mean time. Slightly imprecise log and an extra no-op call is fine.
if is_running {
error!("[{system_name}] {actor_name} {method_name} error: {error:#}, shutting down.");
let _ = system_handle.shutdown();
} else {
warn!(
"[{system_name}] {actor_name} {method_name} error {error:#} while shutting down, \
ignoring."
);
}
}
}

impl Drop for System {
Expand All @@ -658,7 +675,6 @@ impl SystemHandle {
pub fn shutdown(&self) -> Result<(), ActorError> {
let current_thread = thread::current();
let current_thread_name = current_thread.name().unwrap_or("Unknown thread id");
info!("Thread [{}] shutting down the actor system", current_thread_name);

// Use an inner scope to prevent holding the lock for the duration of shutdown
{
Expand All @@ -674,17 +690,15 @@ impl SystemHandle {
return Ok(());
},
SystemState::Running => {
debug!(
"Thread [{}] setting the system_state value to ShuttingDown",
current_thread_name
info!(
"Thread [{}] shutting down the actor system {}",
current_thread_name, self.name
);
*system_state_lock = SystemState::ShuttingDown;
},
}
}

info!("[{}] system shutting down.", self.name);

if let Some(callback) = self.callbacks.preshutdown.as_ref() {
info!("[{}] calling pre-shutdown callback.", self.name);
if let Err(err) = callback() {
Expand Down Expand Up @@ -845,7 +859,7 @@ pub trait Actor {
// 'static required to create trait object in Addr, https://stackoverflow.com/q/29740488/4345715
type Message: Send + 'static;
/// The type to return on error in the handle method.
type Error: std::fmt::Debug;
type Error: std::fmt::Debug + std::fmt::Display;
/// What kind of context this actor accepts. Usually [`Context<Self::Message>`].
type Context;

Expand Down Expand Up @@ -888,13 +902,13 @@ pub trait Actor {
/// # struct TickingActor;
/// impl Actor for TickingActor {
/// # type Context = Context<Self::Message>;
/// # type Error = ();
/// # type Error = String;
/// # type Message = ();
/// # fn name() -> &'static str { "TickingActor" }
/// # fn handle(&mut self, _: &mut Self::Context, _: ()) -> Result<(), ()> { Ok(()) }
/// # fn handle(&mut self, _: &mut Self::Context, _: ()) -> Result<(), String> { Ok(()) }
/// // ...
///
/// fn deadline_passed(&mut self, context: &mut Self::Context, deadline: Instant) -> Result<(), ()> {
/// fn deadline_passed(&mut self, context: &mut Self::Context, deadline: Instant) -> Result<(), String> {
/// // do_periodic_housekeeping();
///
/// // A: Schedule one second from now (even if delayed); drifting tick.
Expand Down Expand Up @@ -1106,14 +1120,14 @@ mod tests {
struct TestActor;
impl Actor for TestActor {
type Context = Context<Self::Message>;
type Error = ();
type Error = String;
type Message = usize;

fn name() -> &'static str {
"TestActor"
}

fn handle(&mut self, _: &mut Self::Context, message: usize) -> Result<(), ()> {
fn handle(&mut self, _: &mut Self::Context, message: usize) -> Result<(), String> {
println!("message: {}", message);

Ok(())
Expand Down Expand Up @@ -1167,14 +1181,14 @@ mod tests {
}
impl Actor for LocalActor {
type Context = Context<Self::Message>;
type Error = ();
type Error = String;
type Message = ();

fn name() -> &'static str {
"LocalActor"
}

fn handle(&mut self, _: &mut Self::Context, _: ()) -> Result<(), ()> {
fn handle(&mut self, _: &mut Self::Context, _: ()) -> Result<(), String> {
Ok(())
}

Expand Down Expand Up @@ -1204,22 +1218,26 @@ mod tests {

impl Actor for TimeoutActor {
type Context = Context<Self::Message>;
type Error = ();
type Error = String;
type Message = Option<Instant>;

fn name() -> &'static str {
"TimeoutActor"
}

fn handle(&mut self, ctx: &mut Self::Context, msg: Self::Message) -> Result<(), ()> {
fn handle(
&mut self,
ctx: &mut Self::Context,
msg: Self::Message,
) -> Result<(), String> {
self.handle_count.fetch_add(1, Ordering::SeqCst);
if msg.is_some() {
ctx.receive_deadline = msg;
}
Ok(())
}

fn deadline_passed(&mut self, _: &mut Self::Context, _: Instant) -> Result<(), ()> {
fn deadline_passed(&mut self, _: &mut Self::Context, _: Instant) -> Result<(), String> {
self.timeout_count.fetch_add(1, Ordering::SeqCst);
Ok(())
}
Expand Down Expand Up @@ -1299,7 +1317,7 @@ mod tests {

impl Actor for PriorityActor {
type Context = Context<Self::Message>;
type Error = ();
type Error = String;
type Message = usize;

fn handle(
Expand Down Expand Up @@ -1356,7 +1374,7 @@ mod tests {
struct Subscriber;
impl Actor for Subscriber {
type Context = Context<Self::Message>;
type Error = ();
type Error = String;
type Message = ();

fn started(&mut self, context: &mut Self::Context) {
Expand Down
4 changes: 2 additions & 2 deletions src/timed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,14 +292,14 @@ mod tests {

impl Actor for TimedTestActor {
type Context = TimedContext<Self::Message>;
type Error = ();
type Error = String;
type Message = usize;

fn name() -> &'static str {
"TimedTestActor"
}

fn handle(&mut self, context: &mut Self::Context, message: usize) -> Result<(), ()> {
fn handle(&mut self, context: &mut Self::Context, message: usize) -> Result<(), String> {
{
let mut guard = self.received.lock().unwrap();
guard.push(message);
Expand Down

0 comments on commit 0e10b57

Please sign in to comment.