From 9d44593e73d2b290f60dc37a6aa441d240a60e37 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Sat, 8 Jul 2023 16:18:45 +0200 Subject: [PATCH] Improve internal server errors significantly --- src/domain.rs | 7 ++++++ src/service/http.rs | 60 +++++++++++++++++++++++++++------------------ 2 files changed, 43 insertions(+), 24 deletions(-) diff --git a/src/domain.rs b/src/domain.rs index 7fbe9fd..e7b4c92 100644 --- a/src/domain.rs +++ b/src/domain.rs @@ -3,6 +3,7 @@ pub mod checker; pub mod config; pub mod manager; +use std::fmt; use std::str::FromStr; use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; @@ -21,6 +22,12 @@ impl Name { } } +impl fmt::Display for Name { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.utf8_str) + } +} + impl FromStr for Name { type Err = ::Err; diff --git a/src/service/http.rs b/src/service/http.rs index 28eb871..7acb3b3 100644 --- a/src/service/http.rs +++ b/src/service/http.rs @@ -91,7 +91,7 @@ struct DomainSyncArgs { } impl<'svc> Service { - fn serve(&self, status_code: u16, path: &'_ str, body: Body) -> Response { + fn serve(&self, status_code: u16, path: &str, body: Body) -> Response { let content_type = mime_guess::from_path(path) .first_or_octet_stream() .to_string(); @@ -103,20 +103,19 @@ impl<'svc> Service { { Ok(res) => res, Err(err) => { + // if the status code was already a 500, don't try to render _another_ 500, it'll + // probably fail for the same reason. At this point something is incredibly wrong, + // just panic. if status_code == 500 { panic!("failed to build {}: {}", path, err); } - self.serve( - 500, - "error.txt", - format!("failed to build {}: {}", path, err).into(), - ) + self.internal_error(format!("failed to build {}: {}", path, err).as_str()) } } } - fn render(&self, status_code: u16, name: &'_ str, value: T) -> Response + fn render(&self, status_code: u16, name: &str, value: T) -> Response where T: Serialize, { @@ -134,7 +133,7 @@ impl<'svc> Service { self.serve(status_code, name, rendered.into()) } - fn render_error_page(&self, status_code: u16, e: &'_ str) -> Response { + fn render_error_page(&self, status_code: u16, e: &str) -> Response { #[derive(Serialize)] struct Response<'a> { error_msg: &'a str, @@ -150,7 +149,15 @@ impl<'svc> Service { ) } - fn render_page(&self, name: &'_ str, data: T) -> Response + fn internal_error(&self, e: &str) -> Response { + log::error!("Internal error: {e}"); + self.render_error_page( + 500, + "An unexpected error occurred. The server administrator will be able to help.", + ) + } + + fn render_page(&self, name: &str, data: T) -> Response where T: Serialize, { @@ -164,7 +171,7 @@ impl<'svc> Service { ) } - fn serve_origin(&self, domain: domain::Name, path: &'_ str) -> Response { + fn serve_origin(&self, domain: domain::Name, path: &str) -> Response { let mut path_owned; let path = match path.ends_with('/') { @@ -185,7 +192,7 @@ impl<'svc> Service { self.render_error_page(404, "File not found") } Err(domain::manager::GetFileError::Unexpected(e)) => { - self.render_error_page(500, format!("failed to fetch file {path}: {e}").as_str()) + self.internal_error(format!("failed to fetch file {path}: {e}").as_str()) } } } @@ -199,11 +206,9 @@ impl<'svc> Service { let query = req.uri().query().unwrap_or(""); match serde_urlencoded::from_str::(query) { Ok(args) => f(args).await, - Err(err) => self.serve( - 400, - "error.txt", - format!("failed to parse query args: {}", err).into(), - ), + Err(err) => { + self.render_error_page(400, format!("failed to parse query args: {}", err).as_str()) + } } } @@ -218,8 +223,13 @@ impl<'svc> Service { Ok(config) => Some(config), Err(domain::manager::GetConfigError::NotFound) => None, Err(domain::manager::GetConfigError::Unexpected(e)) => { - return self - .render_error_page(500, format!("retrieving configuration: {}", e).as_str()); + return self.internal_error( + format!( + "retrieving configuration for domain {}: {}", + &args.domain, e + ) + .as_str(), + ); } }; @@ -256,8 +266,9 @@ impl<'svc> Service { let config_hash = match config.hash() { Ok(hash) => hash, Err(e) => { - return self - .render_error_page(500, format!("failed to hash domain config: {e}").as_str()) + return self.internal_error( + format!("failed to hash domain config {config:?}: {e}").as_str(), + ) } }; @@ -326,9 +337,7 @@ impl<'svc> Service { let domains = match self.domain_manager.all_domains() { Ok(domains) => domains, - Err(e) => { - return self.render_error_page(500, format!("failed get all domains: {e}").as_str()) - } + Err(e) => return self.internal_error(format!("failed get all domains: {e}").as_str()), }; let mut domains: Vec = domains @@ -406,7 +415,10 @@ impl<'svc> Service { .await } (&Method::GET, "/domains.html") => self.domains(), - _ => self.render_error_page(404, "Page not found!"), + _ => self.render_error_page( + 404, + "This is not the page you're looking for. This page doesn't even exist!", + ), } } }