Added book chapter on error handling and fixed any out-of-date inform…

…ation.

doc/src/SUMMARY.md

@@ -9,6 +9,7 @@
   - [Ping](ch02-03-ping.md)
   - [Channels](ch02-04-channels.md)
   - [Unix Signals](ch02-05-signals.md)
+  - [Error handling](ch02-06-errors.md)
 - [I need async/await!](ch03-00-async-await.md)
   - [Run async code](ch03-01-run-async-code.md)
   - [Async IO types](ch03-02-async-io-types.md)

doc/src/ch02-06-errors.md

@@ -0,0 +1,90 @@
+# Error handling in Calloop
+
+## Super quick advice
+
+Use [*Thiserror*](https://crates.io/crates/thiserror) to create structured errors for your library, and [*Anyhow*](https://crates.io/crates/anyhow) to propagate and pass them across API boundaries, like in [the ZeroMQ example](ch04-06-the-full-zeromq-event-source-code.md).
+
+## Overview
+
+Most error handling crates/guides/documentation for Rust focus on one of two situations:
+
+- Creating errors that an API can propagate out to a user of the API, or
+- Making your library deal nicely with the `Result`s from closure or trait methods that it might call
+
+Calloop has to do both of these things. It needs to provide a library user with errors that work well with `?` and common error-handling idioms in their own code, and it needs to handle errors from the callbacks you give to `process_events()` or `insert_source()`. It *also* needs to provide some flexibility in the `EventSource` trait, which is used both for internal event sources and by users of the library.
+
+Because of this, error handling in Calloop leans more towards having separate error types for different concerns. This may mean that there is some extra conversion code in places like returning results from `process_events()`, or in callbacks that use other libraries. However, we try to make it smoother to do these conversions, and to make sure information isn't lost in doing so.
+
+The place where this becomes the most complex is in the `process_events()` method on the `EventSource` trait.
+
+## The Error type on the EventSource trait
+
+The `EventSource` trait contains an associated type named `Error`, which forms part of the return type from `process_events()`. This type must implement `std::error::Error` and be `Sync + Send`.
+
+If your crate already has some form of structured error handling, Calloop's error types should pose no problem to integrate into this. All of Calloop's errors implement `std::error::Error` and can be manipulated the same as any other error types.
+
+If you want a more flexible or general approach, and you're not sure where to start, here are some suggestions that might help.
+
+> Please note that in what follows, the name `Error` can refer to one of two different things:
+> - the trait `std::error::Error` - this will be whenever it qualifies a trait object ie. `dyn Error` means `dyn std::error::Error`
+> - the associated type `Error` on the `EventSource` trait ie. as `type Error = ...`
+
+### Thiserror and Anyhow
+
+[*Thiserror*](https://crates.io/crates/thiserror) and [*Anyhow*](https://crates.io/crates/anyhow) are two excellent error handling crates crated by David Tolnay. Thiserror provides procedural macros for creating structured error types with minimal runtime cost. Anyhow provides some extremely flexible ways to combine errors from different sources and propagate them. This is the approach used in [the ZeroMQ example](ch04-06-the-full-zeromq-event-source-code.md).
+
+One wrinkle in this approach is that `anyhow::Error` does not, in fact, implement `std::error::Error`. This means it can't directly be used as the associated type `calloop::EventSource::Error`. That's where Thiserror comes in.
+
+The basic idea is that you use Thiserror to create an error type to use as the associated type on your event source. This could be a single element struct like this:
+
+```rust,noplayground
+#[derive(thiserror::Error, Debug)]
+#[error(transparent)]
+pub struct MyError(#[from] anyhow::Error);
+```
+
+This creates a minimal implementation for a struct that forwards the important `std::error::Error` trait methods to the encapsulated `anyhow::Error` value. (You could also use Thiserror to create an error with a specific variant for, and conversion from, `zmq::Error` if that's useful.)
+
+But how do we get from one of Calloop's errors (or a third party library's) to this "anyhow" value? One way is to use Anyhow's `context` trait method, which is implemented for any implementation of `std::error::Error`. This is doubly useful: it creates an `anyhow::Error` from the original error, and also adds a message that appears in the traceback. For example:
+
+```rust,noplayground
+self.socket
+    .send_multipart(parts, 0)
+    .context("Failed to send message")?;
+```
+
+Here, the result of `send_multipart()` might be a `zmq::Error`, a type that is completely unrelated to Calloop. Calling `context()` wraps it in an `anyhow::Error` with the message *"Failed to send message"*, which will appear in a traceback if the error (or one containing it) is printed with `{:?}`. The `?` operator then converts it our own `MyError` type if it needs to return early.
+
+### Arc-wrapped errors
+
+Since any error can be converted to a `Box<dyn Error>`, this suggests another simple approach for error handling. Indeed it's pretty common to find libraries returning `Result<..., Box<dyn Error>>`.
+
+Unfortunately you cannot simply set `type Error = Box<dyn Error + Sync + Send>` in your event source. This is for the same reason as with Anyhow: `Box<dyn Error>` does not actually implement the `Error` trait.
+
+There is a smart pointer type in `std` that *does* allow this though: setting `type Error = std::sync::Arc<dyn Error + Sync + Send>` works fine. You can do this with the `map_err()` method on a `Result`:
+
+```rust,noplayground
+type Error = Arc<dyn Error + Sync + Send>;
+
+fn process_events<F>(...) -> Result<calloop::PostAction, Self::Error> where ... {
+    self.nested_source
+        .process_events(readiness, token, |_, _| {})
+        .map_err(|e| Arc::new(e) as Arc<dyn Error + Sync + Send>)?;
+}
+```
+
+The `Arc::new(e) as ...` is known as an [unsized coercion](https://doc.rust-lang.org/reference/type-coercions.html#unsized-coercions). You can even just do:
+
+```rust,noplayground
+    self.nested_source
+        .process_events(readiness, token, |_, _| {})
+        .map_err(Box::from)?;
+```
+
+...since the `?` takes care of the second step of the conversion (`Box` to `Arc` in this case).
+
+### Which to choose
+
+Arc-wrapping errors only really has the advantage of fewer 3rd-party dependencies, and whether that really is an advantage depends on context. If it's a matter of policy, or simply not needing anything more, use this approach.
+
+Anyhow and Thiserror are both extremely lean in terms of code size, performance and their own dependencies. The extra `context()` call is exactly the same number of lines of code as `map_err()` but has the advantage of providing more information. Using Thiserror also lowers the effort for more structured error handling in the future. If those seem useful to you, use this approach.

doc/src/ch04-02-creating-our-source-part-1-our-types.md

@@ -59,14 +59,16 @@ where
 > Remember that it's not just `Vec<T>` and other sequence types that implement `IntoIterator` — `Option<T>` implements it too! There is also `std::iter::Once<T>`. So if a user of our API wants to enforce that all "multi"-part messages actually contain exactly one part, they can use this API with `T` being, say, `std::iter::Once<zmq::Message>` (or even just `[zmq::Message; 1]` in Rust 2021 edition).
 
 ## Associated types
-The `EventSource` trait has three associated types:
+The `EventSource` trait has four associated types:
 
 - `Event` - when an event is generated that our caller cares about (ie. not some internal thing), this is the data we provide to their callback. This will be another sequence of messages, but because we're constructing it we can be more opinionated about the type and use the return type of `zmq::Socket::recv_multipart()` which is `Vec<Vec<u8>>`.
 
 - `Metadata` - this is a more persistent kind of data, perhaps the underlying file descriptor or socket, or maybe some stateful object that the callback can manipulate. It is passed by exclusive reference to the `Metadata` type. In our case we don't use this, so it's `()`.
 
 - `Ret` - this is the return type of the callback that's called on an event. Usually this will be a `Result` of some sort; in our case it's `std::io::Result<()>` just to signal whether some underlying operation failed or not.
 
+- `Error` - this is the error type returned by `process_events()` (not the user callback!). Having this as an associated type allows you to have more control over error propagation in nested event sources. We will use [Thiserror](https://crates.io/crates/thiserror) to have a transparent wrapper around [Anyhow](https://crates.io/crates/anyhow), both very useful error libraries. The wrapper will be named `ZmqError`.
+
 So together these are:
 
 ```rust,noplayground
@@ -78,6 +80,7 @@ where
     type Event = Vec<Vec<u8>>;
     type Metadata = ();
     type Ret = io::Result<()>;
+    type Error = ZmqError;
     // ...
 }
 ```

doc/src/ch04-04-creating-our-source-part-3-processing-events-almost.md

@@ -8,7 +8,7 @@ fn process_events<F>(
     readiness: calloop::Readiness,
     token: calloop::Token,
     mut callback: F,
-) -> calloop::Result<calloop::PostAction>
+) -> Result<calloop::PostAction, Self::Error>
 where
     F: FnMut(Self::Event, &mut Self::Metadata) -> Self::Ret,
 ```
@@ -31,7 +31,7 @@ Implementing `process_events()` for a type that contains various Calloop sources
 
 If we were woken up because of the ping source, then the ping source's `process_events()` will see that the token matches its own, and call the callback (possibly multiple times). If we were woken up because a message was sent through the MPSC channel, then the channel's `process_events()` will match on the token instead and call the callback for every message waiting. The zsocket is a little different, and we'll go over that in detail.
 
-Error handling sometimes requires extra conversions, but Calloop provides some help with that. `process_events()` should return a `Result` with Calloop's error type, and specifically the `calloop::Error::CallbackError` variant in most cases. If you import the trait `calloop::CalloopResult`, you can use the `map_callback_err()` on any `Result` to do the conversion when `?` does not handle it automatically. This also applies to some of the callbacks you need to give to Calloop's event sources eg. `Generic`.
+Error handling sometimes requires extra conversions, so see the [error handling section](ch02-05-errors.md). We use the approach with `thiserror` and `anyhow`, hence the `context()` calls on each fallible operation.
 
 So a first draft of our code might look like:
 
@@ -41,13 +41,14 @@ fn process_events<F>(
     readiness: calloop::Readiness,
     token: calloop::Token,
     mut callback: F,
-) -> calloop::Result<calloop::PostAction>
+) -> Result<calloop::PostAction, Self::Error>
 where
     F: FnMut(Self::Event, &mut Self::Metadata) -> Self::Ret,
 {
     // Runs if we were woken up on startup/registration.
     self.wake_ping_receiver
-        .process_events(readiness, token, |_, _| {})?;
+        .process_events(readiness, token, |_, _| {})
+        .context("Failed after registration")?;
 
     // Runs if we received a message over the MPSC channel.
     self.mpsc_receiver
@@ -57,7 +58,7 @@ where
             if let calloop::channel::Event::Msg(msg) = evt {
                 self.socket
                     .send_multipart(msg, 0)
-                    .unwrap();
+                    .context("Failed to send message")?;
             }
         })?;
 
@@ -67,7 +68,7 @@ where
             let events =
                 self.socket
                     .get_events()
-                    .map_callback_err()?;
+                    .context("Failed to read ZeroMQ events")?;
         
             if events.contains(zmq::POLLOUT) {
                 // Wait, what do we do here?
@@ -77,9 +78,10 @@ where
                 let messages =
                     self.socket
                         .recv_multipart(0)
-                        .map_callback_err()?;
+                        .context("Failed to receive message")?;
 
-                callback(messages, &mut ())?;
+                callback(messages, &mut ())
+                    .context("Error in event callback")?;
             }
         })?;
 
@@ -101,7 +103,9 @@ Thirdly, we commit one of the worst sins you can commit in an event-loop-based s
 self.mpsc_receiver
     .process_events(readiness, token, |evt, _| {
         if let calloop::channel::Event::Msg(msg) = evt {
-            self.socket.send_multipart(msg, 0).unwrap()?;
+            self.socket
+                .send_multipart(msg, 0)
+                .context("Failed to send message")?;
         }
     })?;
 ```
@@ -152,22 +156,26 @@ And our "zsocket is writeable" code becomes:
 ```rust,noplayground
 self.socket
     .process_events(readiness, token, |_, _| {
-        let events = self.socket.get_events()?;
+        let events = self
+            .socket
+            .get_events()
+            .context("Failed to read ZeroMQ events")?;
     
         if events.contains(zmq::POLLOUT) {
             if let Some(parts) = self.outbox.pop_front() {
                 self.socket
                     .send_multipart(parts, 0)
-                    .map_callback_err()?;
+                    .context("Failed to send message")?;
             }
        }
 
         if events.contains(zmq::POLLIN) {
             let messages =
                 self.socket
                     .recv_multipart(0)
-                    .map_callback_err()?;
-            callback(messages, &mut ())?;
+                    .context("Failed to receive message")?;
+            callback(messages, &mut ())
+                .context("Error in event callback")?;
         }
     })?;
 

doc/src/ch04-05-creating-our-source-part-4-processing-events-really.md

@@ -5,22 +5,26 @@ We have three events that could wake up our event source: the ping, the channel,
 Also notice that in the zsocket `process_events()` call, we don't use any of the arguments, including the event itself. That file descriptor is merely a signalling mechanism! Sending and receiving messages is what will actually clear any pending events on it, and reset it to a state where it will wake the event loop later.
 
 ```rust,noplayground
-let events = self.socket.events()?;
+let events = self
+    .socket
+    .get_events()
+    .context("Failed to read ZeroMQ events")?;
 
 if events.contains(zmq::POLLOUT) {
     if let Some(parts) = self.outbox.pop_front() {
         self.socket
             .send_multipart(parts, 0)
-            .map_callback_err()?;
+            .context("Failed to send message")?;
     }
 }
 
 if events.contains(zmq::POLLIN) {
     let messages =
         self.socket
             .recv_multipart(0)
-            .map_callback_err()?;
-    callback(messages, &mut ())?;
+            .context("Failed to receive message")?;
+    callback(messages, &mut ())
+        .context("Error in event callback")?;
 }
 ```
 
@@ -32,7 +36,7 @@ fn process_events<F>(
     readiness: calloop::Readiness,
     token: calloop::Token,
     mut callback: F,
-) -> calloop::Result<calloop::PostAction>
+) -> Result<calloop::PostAction, Self::Error>
 where
     F: FnMut(Self::Event, &mut Self::Metadata) -> Self::Ret,
 {
@@ -52,22 +56,26 @@ where
 
 	// Always process any pending zsocket events.
 
-    let events = self.socket.get_events().map_callback_err()?;
+    let events = self
+        .socket
+        .get_events()
+        .context("Failed to read ZeroMQ events")?;
 
     if events.contains(zmq::POLLOUT) {
         if let Some(parts) = self.outbox.pop_front() {
             self.socket
                 .send_multipart(parts, 0)
-                .map_callback_err()?;
+                .context("Failed to send message")?;
         }
     }
 
     if events.contains(zmq::POLLIN) {
         let messages =
             self.socket
                 .recv_multipart(0)
-                .map_callback_err()?;
-        callback(messages, &mut ())?;
+                .context("Failed to receive message")?;
+        callback(messages, &mut ())
+            .context("Error in event callback")?;
     }
 
     Ok(calloop::PostAction::Continue)
@@ -98,15 +106,19 @@ The full solution is to recognise that any user action on a ZeroMQ socket can ca
 
 ```rust,noplayground
 loop {
-    let events = self.socket.get_events().map_callback_err()?;
+    let events = self
+        .socket
+        .get_events()
+        .context("Failed to read ZeroMQ events")?;
+
     let mut used_socket = false;
 
     if events.contains(zmq::POLLOUT) {
         if let Some(parts) = self.outbox.pop_front() {
             self.socket
                 .as_ref()
                 .send_multipart(parts, 0)
-                .map_callback_err()?;
+                .context("Failed to send message")?;
             used_socket = true;
         }
     }
@@ -115,10 +127,11 @@ loop {
         let messages =
             self.socket
                 .recv_multipart(0)
-                .map_callback_err()?;
+                .context("Failed to receive message")?;
         used_socket = true;
 
-        callback(messages, &mut ())?;
+        callback(messages, &mut ())
+            .context("Error in event callback")?;
     }
 
     if !used_socket {

doc/src/ch04-06-the-full-zeromq-event-source-code.md

@@ -6,10 +6,16 @@ This is the full source code for a Calloop event source based on a ZeroMQ socket
 {{#rustdoc_include zmqsource.rs}}
 ```
 
-Dependencies are only `calloop` and `zmq`:
+Dependencies are:
+- calloop (whatever version this document was built from)
+- zmq 0.9
+- anyhow 1.0
+- thiserror 1.0
 
 ```toml
 [dependencies]
-calloop = "0.10"
+calloop = { path = '../..' }
 zmq = "0.9"
+anyhow = "1.0"
+thiserror = "1.0"
 ```

doc/src/zmqsource.rs

@@ -120,7 +120,7 @@ where
     type Event = Vec<Vec<u8>>;
     type Metadata = ();
     type Ret = io::Result<()>;
-    type Error = Error;
+    type Error = ZmqError;
 
     fn process_events<F>(
         &mut self,
@@ -266,6 +266,6 @@ where
 /// `.map_err(Box::from)?` anywhere you see `.context()?` above.
 #[derive(thiserror::Error, Debug)]
 #[error(transparent)]
-pub struct Error(#[from] anyhow::Error);
+pub struct ZmqError(#[from] anyhow::Error);
 
 pub fn main() {}