Skip to content

Commit

Permalink
Avoid .unwrap() on IO and user errors
Browse files Browse the repository at this point in the history
Before:

```
Error: Os { code: 2, kind: NotFound, message: "No such file or directory" }
```

After:

```
Error: Failed to read TLS config

Caused by:
    No such file or directory (os error 2)
```
  • Loading branch information
progval committed Aug 25, 2023
1 parent 0fde64b commit 83eae64
Show file tree
Hide file tree
Showing 20 changed files with 151 additions and 82 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 19 additions & 4 deletions sable_ipc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ impl<T: Serialize> Sender<T> {
let bytes = DefaultOptions::new()
.with_limit(self.max_len)
.serialize(data)?;
self.socket.as_ref().unwrap().send(&bytes).await?;
self.socket
.as_ref()
.expect("Tried to send to closed IPC socket")
.send(&bytes)
.await?;

Ok(())
}
Expand All @@ -83,7 +87,11 @@ impl<T: Serialize> Sender<T> {
/// Using the returned FD for anything other than `Self::from_raw_fd` may cause unpredictable
/// behaviour in the corresponding `Receiver`.
pub unsafe fn into_raw_fd(mut self) -> std::io::Result<RawFd> {
let std_socket = self.socket.take().unwrap().into_std()?;
let std_socket = self
.socket
.take()
.expect("Tried to get write FD of closed IPC socket")
.into_std()?;
Ok(std_socket.into_raw_fd())
}
}
Expand Down Expand Up @@ -118,7 +126,10 @@ impl<T: DeserializeOwned> Receiver<T> {
}

pub async fn recv(&self) -> Result<T> {
let sock = self.socket.as_ref().unwrap();
let sock = self
.socket
.as_ref()
.expect("Tried to read from closed IPC socket");

loop {
sock.readable().await?;
Expand Down Expand Up @@ -151,7 +162,11 @@ impl<T: DeserializeOwned> Receiver<T> {
/// Using the returned FD for anything other than `Self::from_raw_fd` may cause unpredictable
/// behaviour in the corresponding `Sender`.
pub unsafe fn into_raw_fd(mut self) -> std::io::Result<RawFd> {
let std_socket = self.socket.take().unwrap().into_std()?;
let std_socket = self
.socket
.take()
.expect("Tried to get read FD of closed IPC socket")
.into_std()?;
Ok(std_socket.into_raw_fd())
}
}
Expand Down
3 changes: 2 additions & 1 deletion sable_ircd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ arc-swap = { version = "1.5", features = [ "serde" ] }
parking_lot = { version = "0.12", features = [ "serde" ] }
async-trait = "0.1.57"
structopt = "0.3"
base64 = "0.21"
base64 = "0.21"
anyhow = "1.0"
2 changes: 1 addition & 1 deletion sable_ircd/src/bin/sable_ircd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ struct Opts {
/// Because the tokio runtime can't survive forking, `main()` loads the application
/// configs (in order to report as many errors as possible before daemonising), daemonises,
/// initialises the tokio runtime, and begins the async entry point [`sable_main`].
pub fn main() -> Result<(), Box<dyn std::error::Error>> {
pub fn main() -> Result<(), anyhow::Error> {
let opts = Opts::from_args();

sable_server::run::run_server::<sable_ircd::server::ClientServer>(
Expand Down
6 changes: 3 additions & 3 deletions sable_ircd/src/command/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::errors::HandlerError;
#[derive(Debug)]
pub enum CommandError {
/// Something returned an `Error` that we don't know how to handle
UnderlyingError(Box<dyn std::error::Error + Send + Sync>),
UnderlyingError(anyhow::Error),
/// Something went wrong but we don't have an `Error` impl for it
UnknownError(String),
/// The command couldn't be processed successfully, and the client has already been
Expand Down Expand Up @@ -135,12 +135,12 @@ impl From<UntargetedNumeric> for CommandError {

impl From<ConnectionError> for CommandError {
fn from(e: ConnectionError) -> Self {
Self::UnderlyingError(Box::new(e))
Self::UnderlyingError(e.into())
}
}

impl From<HandlerError> for CommandError {
fn from(e: HandlerError) -> Self {
Self::UnderlyingError(Box::new(e))
Self::UnderlyingError(e.into())
}
}
51 changes: 31 additions & 20 deletions sable_ircd/src/server/server_type.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use super::*;
use crate::connection_collection::ConnectionCollectionState;
use anyhow::Context;
use async_trait::async_trait;
use client_listener::SavedListenerCollection;
use sable_server::ServerSaveError;

/// Saved state of a [`ClientServer`] for later resumption
#[derive(serde::Serialize, serde::Deserialize)]
Expand All @@ -23,16 +25,17 @@ impl sable_server::ServerType for ClientServer {
tls_data: &TlsData,
node: Arc<NetworkNode>,
history_receiver: UnboundedReceiver<NetworkHistoryUpdate>,
) -> Self {
) -> anyhow::Result<Self> {
let (action_submitter, action_receiver) = unbounded_channel();
let (auth_sender, auth_events) = unbounded_channel();
let (client_send, client_recv) = unbounded_channel();

let client_listeners = ListenerCollection::new(client_send).unwrap();
let client_listeners = ListenerCollection::new(client_send)
.context("Could not initialize listener collection")?;

client_listeners
.load_tls_certificates(tls_data.key.clone(), tls_data.cert_chain.clone())
.unwrap();
.context("Could not load TLS certificates")?;

for listener in config.listeners.iter() {
let conn_type = if listener.tls {
Expand All @@ -41,17 +44,23 @@ impl sable_server::ServerType for ClientServer {
ConnectionType::Clear
};
client_listeners
.add_listener(listener.address.parse().unwrap(), conn_type)
.unwrap();
.add_listener(
listener.address.parse().with_context(|| {
format!("Invalid listener address: {}", listener.address)
})?,
conn_type,
)
.context("Cannot add listener")?;
}

Self {
Ok(Self {
action_receiver: Mutex::new(action_receiver),
connection_events: Mutex::new(client_recv),
history_receiver: Mutex::new(history_receiver),
auth_events: Mutex::new(auth_events),

auth_client: AuthClient::new(auth_sender).unwrap(),
auth_client: AuthClient::new(auth_sender)
.context("Could not initialize auth client")?,

action_submitter,
command_dispatcher: CommandDispatcher::new(),
Expand All @@ -60,39 +69,42 @@ impl sable_server::ServerType for ClientServer {
client_caps: CapabilityRepository::new(),
node: node,
listeners: Movable::new(client_listeners),
}
})
}

/// Save the server's state for later resumption
async fn save(mut self) -> ClientServerState {
ClientServerState {
async fn save(mut self) -> Result<ClientServerState, ServerSaveError> {
Ok(ClientServerState {
connections: self.connections.into_inner().save_state(),
auth_state: self.auth_client.save_state().await.unwrap(),
auth_state: self
.auth_client
.save_state()
.await
.map_err(ServerSaveError::IoError)?,
client_caps: self.client_caps,
listener_state: self
.listeners
.take()
.unwrap()
.save()
.await
.expect("failed to save listener state"),
}
.map_err(ServerSaveError::IoError)?,
})
}

/// Restore from a previously saved state.
fn restore(
state: ClientServerState,
node: Arc<NetworkNode>,
history_receiver: UnboundedReceiver<NetworkHistoryUpdate>,
) -> Self {
) -> std::io::Result<Self> {
let (auth_send, auth_recv) = unbounded_channel();
let (action_send, action_recv) = unbounded_channel();
let (client_send, client_recv) = unbounded_channel();

let listeners = ListenerCollection::resume(state.listener_state, client_send)
.expect("failed to restore listener collection");
let listeners = ListenerCollection::resume(state.listener_state, client_send)?;

Self {
Ok(Self {
node,
action_receiver: Mutex::new(action_recv),
action_submitter: action_send,
Expand All @@ -102,14 +114,13 @@ impl sable_server::ServerType for ClientServer {
&listeners,
)),
command_dispatcher: command::CommandDispatcher::new(),
auth_client: AuthClient::resume(state.auth_state, auth_send)
.expect("Failed to reload auth client"),
auth_client: AuthClient::resume(state.auth_state, auth_send)?,
auth_events: Mutex::new(auth_recv),
isupport: Self::build_basic_isupport(),
client_caps: state.client_caps,
history_receiver: Mutex::new(history_receiver),
listeners: Movable::new(listeners),
}
})
}

async fn run(self: Arc<Self>, shutdown: broadcast::Receiver<ShutdownAction>) {
Expand Down
1 change: 1 addition & 0 deletions sable_network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ wildmatch = "2.1"
concurrent_log = { version = "0.2.4", features = [ "serde" ] }
once_cell = "1.13"
ipnet = { version = "2", features = [ "serde" ] }
anyhow = "1.0"

[dependencies.serde]
version = "1"
Expand Down
4 changes: 2 additions & 2 deletions sable_network/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use serde::Deserialize;
use std::{error::Error, fs::File, io::BufReader, path::PathBuf};
use std::{fs::File, io::BufReader, path::PathBuf};

use crate::prelude::ConfigError;

Expand All @@ -16,7 +16,7 @@ pub struct TlsData {
}

impl TlsConfig {
pub fn load_from_disk(&self) -> Result<TlsData, Box<dyn Error>> {
pub fn load_from_disk(&self) -> Result<TlsData, anyhow::Error> {
let cert_file = File::open(&self.cert_file)?;
let mut cert_reader = BufReader::new(cert_file);
let cert_chain = rustls_pemfile::certs(&mut cert_reader)?;
Expand Down
4 changes: 2 additions & 2 deletions sable_network/src/policy/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub enum PermissionError {
Channel(ChannelName, ChannelPermissionError),
User(UserPermissionError),
Registration(RegistrationPermissionError),
InternalError(Box<dyn std::error::Error + Send + Sync>),
InternalError(anyhow::Error),
}

impl From<UserPermissionError> for PermissionError {
Expand All @@ -65,6 +65,6 @@ impl From<RegistrationPermissionError> for PermissionError {

impl From<LookupError> for PermissionError {
fn from(e: LookupError) -> Self {
Self::InternalError(Box::new(e))
Self::InternalError(e.into())
}
}
1 change: 1 addition & 0 deletions sable_network/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub use network::GossipNetwork;
pub use network::GossipNetworkState;
pub use network::NetworkError;

pub use replicated_log::EventLogSaveError;
pub use replicated_log::ReplicatedEventLog;
pub use replicated_log::ReplicatedEventLogState;

Expand Down
2 changes: 1 addition & 1 deletion sable_network/src/sync/replicated_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub enum EventLogSaveError {
#[error("{0}")]
InternalError(&'static str),
#[error("Unknown error: {0}")]
UnknownError(#[from] Box<dyn std::error::Error + Send + Sync>),
UnknownError(#[from] anyhow::Error),
}

/// Saved state for a [ReplicatedEventLog], used to save and restore across an upgrade
Expand Down
1 change: 1 addition & 0 deletions sable_server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ daemonize = "0.4"
nix = "0.24"
memfd = "0.4"
parking_lot = { version = "0.12", features = [ "serde" ] }
anyhow = "1.0"
4 changes: 2 additions & 2 deletions sable_server/src/management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ impl ManagementServer {
conn: TcpStream,
acceptor: Arc<TlsAcceptor>,
data: Arc<ManagementServiceData>,
) -> Result<(), Box<dyn std::error::Error>> {
) -> Result<(), anyhow::Error> {
let stream = acceptor.accept(conn).await?;
let (_, tls_state) = stream.get_ref();
let client_cert = tls_state
Expand Down Expand Up @@ -269,7 +269,7 @@ impl ManagementServer {
self.command_receiver.recv().await
}

pub async fn wait(self) -> Result<(), Box<dyn std::error::Error>> {
pub async fn wait(self) -> Result<(), anyhow::Error> {
Ok(self.server_task.await??)
}
}
Loading

0 comments on commit 83eae64

Please sign in to comment.