error management in all server starts and Server::new()

bin/src/logging.rs

@@ -51,7 +51,6 @@ macro_rules! log {
     };
 }
 
-// this should return a Result to trickle up errors
 pub fn init(
     tag: String,
     spec: &str,

bin/src/worker.rs

@@ -162,13 +162,13 @@ pub fn begin_worker_process(
         .with_context(|| "Could not setup metrics")?;
     }
 
-    let mut server = Server::new_from_config(
+    let mut server = Server::try_new_from_config(
         worker_to_main_channel,
         ScmSocket::new(worker_to_main_scm_fd),
         worker_config,
         config_state,
         true,
-    );
+    ).with_context(||"Could not create server from config")?;
 
     info!("starting event loop");
     server.run();

lib/src/http.rs

@@ -1825,6 +1825,7 @@ impl ProxyConfiguration<Session> for Proxy {
     }
 }
 
+/// This is not directly used by Sōzu but is available for example and testing purposes
 pub fn start(
     config: HttpListener,
     channel: ProxyChannel,
@@ -1911,7 +1912,8 @@ pub fn start(
         server_config,
         None,
         false,
-    );
+    )
+    .with_context(|| "Failed at creating server")?;
 
     println!("starting event loop");
     server.run();

lib/src/https_openssl.rs

@@ -11,6 +11,7 @@ use std::{
     sync::{Arc, Mutex},
 };
 
+use anyhow::Context;
 use foreign_types_shared::{ForeignType, ForeignTypeRef};
 use mio::{net::*, unix::SourceFd, *};
 use nom::HexDisplay;
@@ -2403,7 +2404,7 @@ yD0TrUjkXyjV/zczIYiYSROg9OE5UgYqswIBAg==
 }
 
 use crate::server::HttpsProvider;
-// this function is not used anywhere it seems
+/// this function is not used, but is available for example and testing purposes
 pub fn start(
     config: HttpsListener,
     channel: ProxyChannel,
@@ -2457,7 +2458,7 @@ pub fn start(
     let registry = event_loop
         .registry()
         .try_clone()
-        .with_context("Failed at creating a registry")?;
+        .with_context(|| "Failed at creating a registry")?;
     let mut configuration = Proxy::new(registry, sessions.clone(), pool.clone(), backends.clone());
     let address = config.address;
     if configuration.add_listener(config, token).is_some()
@@ -2480,7 +2481,8 @@ pub fn start(
             server_config,
             None,
             false,
-        );
+        )
+        .with_context(|| "Failed to create server")?;
 
         info!("starting event loop");
         server.run();

lib/src/https_rustls/configuration.rs

@@ -9,6 +9,7 @@ use std::{
     sync::Arc,
 };
 
+use anyhow::Context;
 use mio::{net::*, *};
 use rustls::{cipher_suite::*, ServerConfig, ServerConnection};
 use slab::Slab;
@@ -696,10 +697,16 @@ impl ProxyConfiguration<Session> for Proxy {
 }
 
 use crate::server::HttpsProvider;
-pub fn start(config: HttpsListener, channel: ProxyChannel, max_buffers: usize, buffer_size: usize) {
+/// This is not directly used by Sōzu but is available for example and testing purposes
+pub fn start(
+    config: HttpsListener,
+    channel: ProxyChannel,
+    max_buffers: usize,
+    buffer_size: usize,
+) -> anyhow::Result<()> {
     use crate::server;
 
-    let event_loop = Poll::new().expect("could not create event loop"); // we should be able to trickle up this error
+    let event_loop = Poll::new().with_context(|| "could not create event loop")?;
 
     let pool = Rc::new(RefCell::new(Pool::with_capacity(
         1,
@@ -742,11 +749,14 @@ pub fn start(config: HttpsListener, channel: ProxyChannel, max_buffers: usize, b
 
     let address = config.address;
     let sessions = SessionManager::new(sessions, max_buffers);
-    let registry = event_loop.registry().try_clone().unwrap();
+    let registry = event_loop
+        .registry()
+        .try_clone()
+        .with_context(|| "Could not clone the mio registry")?;
     let mut configuration = Proxy::new(registry, sessions.clone(), pool.clone(), backends.clone());
     if configuration
         .add_listener(config, token)
-        .expect("failed to create listener") // we should be able to trickle up this error
+        .with_context(|| "failed to create listener")?
         .is_some()
         && configuration.activate_listener(&address, None).is_some()
     {
@@ -769,10 +779,12 @@ pub fn start(config: HttpsListener, channel: ProxyChannel, max_buffers: usize, b
             server_config,
             None,
             false,
-        );
+        )
+        .with_context(|| "Failed at creating server")?;
 
         info!("starting event loop");
         server.run();
         info!("ending event loop");
     }
+    Ok(())
 }

lib/src/protocol/http/parser/request.rs

@@ -310,7 +310,7 @@ pub fn validate_request_header(
         HeaderValue::Upgrade(s) => {
             let mut st = state;
             if let Some(conn) = st.get_mut_connection() {
-                // do we really want to crash here?
+                // do we really want to panic here?
                 conn.upgrade = Some(str::from_utf8(s).expect("should be ascii").to_string())
             }
             st

lib/src/protocol/http/parser/response.rs

@@ -227,7 +227,7 @@ pub fn validate_response_header(
         }
         HeaderValue::Upgrade(protocol) => {
             let proto = str::from_utf8(protocol)
-                .expect("the parsed protocol should be a valid utf8 string") // do we really want to crash here?
+                .expect("the parsed protocol should be a valid utf8 string") // do we really want to panic here?
                 .to_string();
             trace!("parsed a protocol: {:?}", proto);
             trace!("state is {:?}", state);

lib/src/protocol/openssl.rs

@@ -56,11 +56,11 @@ impl TlsHandshake {
                 let ssl = self
                     .ssl
                     .take()
-                    .expect("TlsHandshake should have a Ssl backend"); // do we really want to crash here?
+                    .expect("TlsHandshake should have a Ssl backend"); // do we really want to panic here?
                 let sock = self
                     .front
                     .take()
-                    .expect("TlsHandshake should have a front socket"); // do we really want to crash here?
+                    .expect("TlsHandshake should have a front socket"); // do we really want to panic here?
                 match ssl.accept(sock) {
                     Ok(stream) => {
                         self.stream = Some(stream);

lib/src/server.rs

@@ -8,6 +8,7 @@ use std::{
     rc::Rc,
 };
 
+use anyhow::Context;
 use mio::net::*;
 use mio::*;
 use slab::Slab;
@@ -274,15 +275,14 @@ pub struct Server {
 }
 
 impl Server {
-    pub fn new_from_config(
+    pub fn try_new_from_config(
         worker_to_main_channel: ProxyChannel,
         worker_to_main_scm: ScmSocket,
         config: Config,
         config_state: ConfigState,
         expects_initial_status: bool,
-    ) -> Self {
-        // we should be able to trickle up this error
-        let event_loop = Poll::new().expect("could not create event loop");
+    ) -> anyhow::Result<Self> {
+        let event_loop = Poll::new().with_context(|| "could not create event loop")?;
         let pool = Rc::new(RefCell::new(Pool::with_capacity(
             config.min_buffers,
             config.max_buffers,
@@ -326,7 +326,7 @@ impl Server {
         let registry = event_loop
             .registry()
             .try_clone()
-            .expect("could not clone the mio Registry"); // we should trickle this up
+            .with_context(|| "could not clone the mio Registry")?;
 
         let https = HttpsProvider::new(
             use_openssl,
@@ -365,7 +365,7 @@ impl Server {
         server_config: ServerConfig,
         config_state: Option<ConfigState>,
         expects_initial_status: bool,
-    ) -> Self {
+    ) -> anyhow::Result<Self> {
         FEATURES.with(|_features| {
             // initializing feature flags
         });
@@ -376,7 +376,7 @@ impl Server {
                 Token(0),
                 Interest::READABLE | Interest::WRITABLE,
             )
-            .expect("should register the channel"); // we should be able to trickle up this error
+            .with_context(|| "should register the channel")?;
 
         METRICS.with(|metrics| {
             if let Some(sock) = (*metrics.borrow_mut()).socket_mut() {
@@ -388,33 +388,44 @@ impl Server {
 
         let base_sessions_count = sessions.borrow().slab.len();
 
-        let http = Rc::new(RefCell::new(http.unwrap_or_else(|| {
-            let registry = poll
-                .registry()
-                .try_clone()
-                .expect("could not clone the mio Registry");
-            http::Proxy::new(registry, sessions.clone(), pool.clone(), backends.clone())
-        })));
-        let https = https.unwrap_or_else(|| {
-            let registry = poll
-                .registry()
-                .try_clone()
-                .expect("could not clone the mio Registry");
-            HttpsProvider::new(
-                false,
-                registry,
-                sessions.clone(),
-                pool.clone(),
-                backends.clone(),
-            )
-        });
-        let tcp = Rc::new(RefCell::new(tcp.unwrap_or_else(|| {
-            let registry = poll
-                .registry()
-                .try_clone()
-                .expect("could not clone the mio Registry");
-            tcp::Proxy::new(registry, sessions.clone(), backends.clone())
-        })));
+        let http = Rc::new(RefCell::new(match http {
+            Some(http) => http,
+            None => {
+                let registry = poll
+                    .registry()
+                    .try_clone()
+                    .with_context(|| "could not clone the mio Registry")?;
+                http::Proxy::new(registry, sessions.clone(), pool.clone(), backends.clone())
+            }
+        }));
+
+        let https = match https {
+            Some(https) => https,
+            None => {
+                let registry = poll
+                    .registry()
+                    .try_clone()
+                    .with_context(|| "could not clone the mio Registry")?;
+                HttpsProvider::new(
+                    false,
+                    registry,
+                    sessions.clone(),
+                    pool.clone(),
+                    backends.clone(),
+                )
+            }
+        };
+
+        let tcp = Rc::new(RefCell::new(match tcp {
+            Some(tcp) => tcp,
+            None => {
+                let registry = poll
+                    .registry()
+                    .try_clone()
+                    .with_context(|| "could not clone the mio Registry")?;
+                tcp::Proxy::new(registry, sessions.clone(), backends.clone())
+            }
+        }));
 
         let mut server = Server {
             poll,
@@ -479,7 +490,7 @@ impl Server {
         info!("received listeners: {:?}", listeners);
         server.scm_listeners = listeners;
 
-        server
+        Ok(server)
     }
 }
 
@@ -767,7 +778,7 @@ impl Server {
                         id: self
                             .shutting_down
                             .take()
-                            .expect("should have shut down correctly"), // this would be better if trickled up and logged properly
+                            .expect("should have shut down correctly"), // panicking here makes sense actually
                         status: ProxyResponseStatus::Ok,
                         content: None,
                     });