Improve error messages, don't log as much stuff

This commit is contained in:
asonix 2023-11-13 17:07:05 -06:00
parent 532c88da82
commit 53f95a6206
8 changed files with 169 additions and 132 deletions

52
Cargo.lock generated
View file

@ -575,6 +575,33 @@ version = "0.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "702fc72eb24e5a1e48ce58027a675bc24edd52096d5397d4aea7c6dd9eca0bd1"
[[package]]
name = "color-eyre"
version = "0.6.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5a667583cca8c4f8436db8de46ea8233c42a7d9ae424a82d338f2e4675229204"
dependencies = [
"backtrace",
"color-spantrace",
"eyre",
"indenter",
"once_cell",
"owo-colors",
"tracing-error",
]
[[package]]
name = "color-spantrace"
version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1ba75b3d9449ecdccb27ecbc479fdc0b87fa2dd43d2f8298f9bf0e59aacc8dce"
dependencies = [
"once_cell",
"owo-colors",
"tracing-core",
"tracing-error",
]
[[package]]
name = "colorchoice"
version = "1.0.0"
@ -754,6 +781,16 @@ version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5"
[[package]]
name = "eyre"
version = "0.6.8"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4c2b6b5a29c02cdc822728b7d7b8ae1bab3e3b05d44522770ddd49722eeac7eb"
dependencies = [
"indenter",
"once_cell",
]
[[package]]
name = "fastrand"
version = "2.0.1"
@ -1043,6 +1080,12 @@ version = "0.1.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "206ca75c9c03ba3d4ace2460e57b189f39f43de612c2f85836e65c929701bb2d"
[[package]]
name = "indenter"
version = "0.3.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ce23b50ad8242c51a442f3ff322d56b02f08852c77e4c0b4d3fd684abc89c683"
[[package]]
name = "indexmap"
version = "1.9.3"
@ -1434,6 +1477,12 @@ version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39"
[[package]]
name = "owo-colors"
version = "3.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c1b04fb49957986fdce4d6ee7a65027d55d4b6d2265e5848bbb507b58ccfdb6f"
[[package]]
name = "parking_lot"
version = "0.11.2"
@ -1514,6 +1563,7 @@ dependencies = [
"awc",
"bcrypt",
"clap",
"color-eyre",
"console-subscriber",
"dotenv",
"mime",
@ -1633,7 +1683,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "265baba7fabd416cf5078179f7d2cbeca4ce7a9041111900675ea7c4cb8a4c32"
dependencies = [
"anyhow",
"itertools 0.10.5",
"itertools 0.11.0",
"proc-macro2",
"quote",
"syn 2.0.39",

View file

@ -44,6 +44,7 @@ tracing-subscriber = { version = "0.3", features = [
] }
url = { version = "2.2", features = ["serde"] }
uuid = { version = "1", features = ["serde", "v4"] }
color-eyre = "0.6.2"
[dependencies.tracing-actix-web]
@ -54,7 +55,7 @@ features = ["emit_event_on_error", "opentelemetry_0_21"]
[dependencies.tracing-awc]
version = "0.1.8"
default-features = false
features = ["emit_event_on_error", "opentelemetry_0_21"]
features = ["opentelemetry_0_21"]
[dev-dependencies]
ructe = "0.17.0"

View file

@ -1,9 +1,10 @@
use std::time::Duration;
use std::{sync::Arc, time::Duration};
use crate::pict::{Details, Extension, Images, Upload, Uploads};
use actix_web::{
body::BodyStream, http::StatusCode, web, HttpRequest, HttpResponse, ResponseError,
use crate::{
pict::{Details, Extension, Images, Upload, Uploads},
Error,
};
use actix_web::{body::BodyStream, http::StatusCode, web, HttpRequest, HttpResponse};
use awc::Client;
use url::Url;
@ -29,7 +30,7 @@ impl Connection {
}
#[tracing::instrument(skip_all)]
pub(crate) async fn claim(&self, upload: Upload) -> Result<Images, UploadError> {
pub(crate) async fn claim(&self, upload: Upload) -> Result<Images, Error> {
let mut attempts = 0;
const CLAIM_ATTEMPT_LIMIT: usize = 10;
loop {
@ -37,21 +38,22 @@ impl Connection {
Ok(mut res) => {
match res.status() {
StatusCode::OK => {
return res.json::<Images>().await.map_err(|_| UploadError::Json);
}
StatusCode::UNPROCESSABLE_ENTITY => {
let images =
res.json::<Images>().await.map_err(|_| UploadError::Json)?;
tracing::warn!("{}", images.msg());
return Err(UploadError::Status);
return res
.json::<Images>()
.await
.map_err(|_| UploadError::Json.into());
}
StatusCode::NO_CONTENT => {
// continue
}
_ => {
return Err(UploadError::Status);
let images =
res.json::<Images>().await.map_err(|_| UploadError::Json)?;
let code = images.code().unwrap_or("uknown-error").to_string();
let msg = images.msg().to_string();
return Err(UploadError::UploadFailure(code, msg).into());
}
}
}
@ -59,7 +61,7 @@ impl Connection {
attempts += 1;
if attempts > CLAIM_ATTEMPT_LIMIT {
return Err(UploadError::Status);
return Err(UploadError::Status.into());
}
tokio::time::sleep(Duration::from_secs(1)).await;
@ -69,49 +71,49 @@ impl Connection {
}
}
#[tracing::instrument(skip_all)]
pub(crate) async fn thumbnail(
&self,
size: u16,
file: &str,
extension: Extension,
req: &HttpRequest,
) -> Result<HttpResponse, actix_web::Error> {
) -> Result<HttpResponse, Error> {
if !VALID_SIZES.contains(&size) {
return Err(SizeError(size).into());
return Err(UploadError::Size(size).into());
}
self.proxy(self.thumbnail_url(size, file, extension), req)
.await
}
pub(crate) async fn image(
&self,
file: &str,
req: &HttpRequest,
) -> Result<HttpResponse, actix_web::Error> {
#[tracing::instrument(skip_all)]
pub(crate) async fn image(&self, file: &str, req: &HttpRequest) -> Result<HttpResponse, Error> {
self.proxy(self.image_url(file), req).await
}
pub(crate) async fn details(&self, file: &str) -> Result<Details, UploadError> {
#[tracing::instrument(skip_all)]
pub(crate) async fn details(&self, file: &str) -> Result<Details, Error> {
let mut response = self
.client
.get(self.details_url(file))
.send()
.await
.map_err(|_| UploadError::Request)?;
.map_err(|e| UploadError::Request(Arc::from(e.to_string())))?;
if !response.status().is_success() {
return Err(UploadError::Status);
return Err(UploadError::Status.into());
}
response.json().await.map_err(|_| UploadError::Json)
response.json().await.map_err(|_| UploadError::Json.into())
}
#[tracing::instrument(skip_all)]
pub(crate) async fn upload(
&self,
req: &HttpRequest,
body: web::Payload,
) -> Result<Uploads, UploadError> {
) -> Result<Uploads, Error> {
let client_request = self.client.request_from(self.upload_url(), req.head());
let mut client_request = if let Some(addr) = req.head().peer_addr {
@ -125,23 +127,24 @@ impl Connection {
let mut res = client_request
.send_stream(body)
.await
.map_err(|_| UploadError::Request)?;
.map_err(|e| UploadError::Request(Arc::from(e.to_string())))?;
let uploads = res.json::<Uploads>().await.map_err(|_| UploadError::Json)?;
Ok(uploads)
}
pub(crate) async fn delete(&self, file: &str, token: &str) -> Result<(), UploadError> {
#[tracing::instrument(skip_all)]
pub(crate) async fn delete(&self, file: &str, token: &str) -> Result<(), Error> {
let res = self
.client
.delete(self.delete_url(file, token))
.send()
.await
.map_err(|_| UploadError::Request)?;
.map_err(|e| UploadError::Request(Arc::from(e.to_string())))?;
if !res.status().is_success() {
return Err(UploadError::Status);
return Err(UploadError::Status.into());
}
Ok(())
@ -191,11 +194,8 @@ impl Connection {
url.to_string()
}
async fn proxy(
&self,
url: String,
req: &HttpRequest,
) -> Result<HttpResponse, actix_web::Error> {
#[tracing::instrument(skip_all)]
async fn proxy(&self, url: String, req: &HttpRequest) -> Result<HttpResponse, Error> {
let client_request = self.client.request_from(url, req.head());
let client_request = if let Some(addr) = req.head().peer_addr {
client_request.append_header(("X-Forwarded-For", addr.to_string()))
@ -207,7 +207,7 @@ impl Connection {
.no_decompress()
.send()
.await
.map_err(|_| UploadError::Request)?;
.map_err(|e| UploadError::Request(Arc::from(e.to_string())))?;
let mut client_res = HttpResponse::build(res.status());
@ -221,40 +221,18 @@ impl Connection {
#[derive(Clone, Debug, thiserror::Error)]
pub(crate) enum UploadError {
#[error("There was an error uploading the image")]
Request,
#[error("There was an error making the upstream request: {0}")]
Request(Arc<str>),
#[error("There was an error parsing the image response")]
#[error("There was an error parsing the upstream response")]
Json,
#[error("Request returned bad HTTP status")]
Status,
}
impl ResponseError for UploadError {
fn status_code(&self) -> StatusCode {
StatusCode::INTERNAL_SERVER_ERROR
}
fn error_response(&self) -> HttpResponse {
HttpResponse::build(self.status_code())
.content_type(mime::TEXT_PLAIN.essence_str())
.body(self.to_string())
}
}
#[derive(Clone, Debug, thiserror::Error)]
#[error("The requested size is invalid, {0}")]
struct SizeError(u16);
impl ResponseError for SizeError {
fn status_code(&self) -> StatusCode {
StatusCode::BAD_REQUEST
}
fn error_response(&self) -> HttpResponse {
HttpResponse::build(self.status_code())
.content_type(mime::TEXT_PLAIN.essence_str())
.body(self.to_string())
}
#[error("Request failed with {0}: {1}")]
UploadFailure(String, String),
#[error("Requested size {0} is invalid")]
Size(u16),
}

View file

@ -15,9 +15,9 @@ use std::{
io::Cursor,
net::SocketAddr,
path::{Path, PathBuf},
sync::Arc,
time::SystemTime,
};
use tracing_error::SpanTrace;
use url::Url;
use uuid::Uuid;
@ -357,10 +357,12 @@ impl<T, E> ResultExt<T> for Result<T, E>
where
Error: From<E>,
{
#[track_caller]
fn stateful(self, state: &web::Data<State>) -> Result<T, StateError> {
self.map_err(Error::from).map_err(|error| StateError {
state: state.clone(),
error,
debug: Arc::from(format!("{error:?}")),
display: Arc::from(format!("{error}")),
})
}
}
@ -389,7 +391,7 @@ async fn static_files(filename: web::Path<String>, state: web::Data<State>) -> H
to_404(&state)
}
#[tracing::instrument(name = "Not found")]
#[tracing::instrument(name = "Not found", skip_all)]
async fn not_found(state: web::Data<State>) -> Result<HttpResponse, StateError> {
rendered(
|cursor| self::templates::not_found_html(cursor, &state),
@ -400,20 +402,19 @@ async fn not_found(state: web::Data<State>) -> Result<HttpResponse, StateError>
struct StateError {
state: web::Data<State>,
error: Error,
debug: Arc<str>,
display: Arc<str>,
}
impl std::fmt::Debug for StateError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
f.debug_struct("StateError")
.field("error", &self.error)
.finish()
f.write_str(&self.debug)
}
}
impl std::fmt::Display for StateError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "{}", self.error)
f.write_str(&self.display)
}
}
@ -424,33 +425,36 @@ impl ResponseError for StateError {
fn error_response(&self) -> HttpResponse {
match rendered(
|cursor| self::templates::error_html(cursor, &self.error.kind.to_string(), &self.state),
|cursor| self::templates::error_html(cursor, &self.display, &self.state),
HttpResponse::build(self.status_code()),
) {
Ok(res) => res,
Err(_) => HttpResponse::build(self.status_code())
.content_type(mime::TEXT_PLAIN.essence_str())
.body(self.error.kind.to_string()),
.body(self.display.to_string()),
}
}
}
#[derive(Debug)]
struct Error {
context: SpanTrace,
kind: ErrorKind,
context: color_eyre::Report,
}
impl std::fmt::Debug for Error {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.context.fmt(f)
}
}
impl std::fmt::Display for Error {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "{}", self.kind)?;
std::fmt::Display::fmt(&self.context, f)
self.context.fmt(f)
}
}
impl std::error::Error for Error {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
self.kind.source()
self.context.source()
}
}
@ -458,23 +462,23 @@ impl<T> From<T> for Error
where
ErrorKind: From<T>,
{
#[track_caller]
fn from(error: T) -> Self {
Error {
context: SpanTrace::capture(),
kind: error.into(),
context: color_eyre::Report::from(ErrorKind::from(error)),
}
}
}
#[derive(Debug, thiserror::Error)]
enum ErrorKind {
#[error("{0}")]
#[error("Error in IO")]
Render(#[from] std::io::Error),
#[error("{0}")]
#[error("Error in store")]
Store(#[from] self::store::Error),
#[error("{0}")]
#[error("Error in pict-rs connection")]
Upload(#[from] self::connection::UploadError),
#[error("{0}")]
@ -654,7 +658,7 @@ impl MoveEntryPath {
}
}
#[tracing::instrument(name = "Upload image", skip(req, pl))]
#[tracing::instrument(name = "Upload image", skip(req, pl, token, conn, state))]
async fn upload(
req: HttpRequest,
pl: web::Payload,
@ -753,13 +757,14 @@ struct ImagePath {
filename: String,
}
#[tracing::instrument(name = "Serve image", skip(req))]
#[tracing::instrument(name = "Serve image", skip(req, conn, state))]
async fn image(
req: HttpRequest,
path: web::Path<ImagePath>,
conn: web::Data<Connection>,
) -> Result<HttpResponse, actix_web::Error> {
conn.image(&path.filename, &req).await
state: web::Data<State>,
) -> Result<HttpResponse, StateError> {
conn.image(&path.filename, &req).await.stateful(&state)
}
#[derive(Debug, serde::Deserialize)]
@ -773,18 +778,20 @@ struct ThumbnailQuery {
size: u16,
}
#[tracing::instrument(name = "Serve thumbnail", skip(req))]
#[tracing::instrument(name = "Serve thumbnail", skip(req, conn, state))]
async fn thumbnail(
req: HttpRequest,
path: web::Path<ThumbnailPath>,
query: web::Query<ThumbnailQuery>,
conn: web::Data<Connection>,
) -> Result<HttpResponse, actix_web::Error> {
state: web::Data<State>,
) -> Result<HttpResponse, StateError> {
conn.thumbnail(query.size, &query.src, path.extension, &req)
.await
.stateful(&state)
}
#[tracing::instrument(name = "Index")]
#[tracing::instrument(name = "Index", skip_all)]
async fn index(state: web::Data<State>) -> Result<HttpResponse, StateError> {
rendered(
|cursor| self::templates::index_html(cursor, &state),
@ -793,16 +800,16 @@ async fn index(state: web::Data<State>) -> Result<HttpResponse, StateError> {
.stateful(&state)
}
#[tracing::instrument(name = "Collection", skip(req))]
#[tracing::instrument(name = "Collection", skip(req, token, connection, state))]
async fn collection(
req: HttpRequest,
path: web::Path<CollectionPath>,
token: Option<ValidToken>,
state: web::Data<State>,
connection: web::Data<Connection>,
req: HttpRequest,
state: web::Data<State>,
) -> Result<HttpResponse, StateError> {
match token {
Some(token) => edit_collection(path, token, connection, state.clone(), req)
Some(token) => edit_collection(req, path, token, connection, state.clone())
.await
.stateful(&state),
None => view_collection(path, connection, state.clone())
@ -903,7 +910,7 @@ async fn ensure_dimensions(
Ok(entries)
}
#[tracing::instrument(name = "View Collection")]
#[tracing::instrument(name = "View Collection", skip(connection, state))]
async fn view_collection(
path: web::Path<CollectionPath>,
connection: web::Data<Connection>,
@ -935,13 +942,13 @@ async fn view_collection(
)
}
#[tracing::instrument(name = "Edit Collection", skip(req))]
#[tracing::instrument(name = "Edit Collection", skip(req, connection, state))]
async fn edit_collection(
req: HttpRequest,
path: web::Path<CollectionPath>,
token: ValidToken,
connection: web::Data<Connection>,
state: web::Data<State>,
req: HttpRequest,
) -> Result<HttpResponse, Error> {
let qr = qr(&req, &path, &state);
@ -972,7 +979,7 @@ async fn edit_collection(
)
}
#[tracing::instrument(name = "Create Collection")]
#[tracing::instrument(name = "Create Collection", skip_all)]
async fn create_collection(
collection: web::Form<Collection>,
state: web::Data<State>,
@ -1002,7 +1009,7 @@ async fn create_collection(
))
}
#[tracing::instrument(name = "Update Collection")]
#[tracing::instrument(name = "Update Collection", skip(token, state))]
async fn update_collection(
path: web::Path<CollectionPath>,
form: web::Form<Collection>,
@ -1040,7 +1047,7 @@ async fn move_entry(
))
}
#[tracing::instrument(name = "Update Entry")]
#[tracing::instrument(name = "Update Entry", skip(token, state))]
async fn update_entry(
entry_path: web::Path<EntryPath>,
entry: web::Form<Entry>,
@ -1068,7 +1075,7 @@ struct ConfirmQuery {
confirmed: Option<bool>,
}
#[tracing::instrument(name = "Delete Entry")]
#[tracing::instrument(name = "Delete Entry", skip(token, conn, state))]
async fn delete_entry(
entry_path: web::Path<EntryPath>,
query: web::Query<ConfirmQuery>,
@ -1161,7 +1168,7 @@ fn to_svg_string(qr: &qrcodegen::QrCode, border: i32) -> String {
result
}
#[tracing::instrument(name = "Delete Collection")]
#[tracing::instrument(name = "Delete Collection", skip(token, conn, state))]
async fn delete_collection(
path: web::Path<CollectionPath>,
query: web::Query<ConfirmQuery>,
@ -1219,11 +1226,7 @@ async fn delete_collection(
let mut results = Vec::new();
for rx in future_vec {
results.push(
rx.await
.map_err(|_| ErrorKind::UploadString("Canceled".to_string()))
.stateful(&state)?,
);
results.push(rx.await.map_err(|_| ErrorKind::Canceled).stateful(&state)?);
}
// Only bail before deleting collection if all images failed deletion

View file

@ -12,13 +12,12 @@ use tracing_awc::Tracing;
use tracing_error::ErrorLayer;
use tracing_log::LogTracer;
use tracing_subscriber::{
filter::Targets, fmt::format::FmtSpan, layer::SubscriberExt, registry::LookupSpan, Layer,
Registry,
filter::Targets, layer::SubscriberExt, registry::LookupSpan, Layer, Registry,
};
use url::Url;
#[actix_rt::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
async fn main() -> color_eyre::Result<()> {
let config = pict_rs_aggregator::Config::parse();
init_logger(
@ -36,11 +35,13 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let bind_address = config.bind_address();
let state = pict_rs_aggregator::state(config, "", db)?;
tracing::info!("Launching on {bind_address}");
HttpServer::new(move || {
let client = Client::builder()
.wrap(Tracing)
.timeout(Duration::from_secs(30))
.add_default_header(("User-Agent", "pict_rs_aggregator-v0.5.0-alpha.1"))
.add_default_header(("User-Agent", "pict_rs_aggregator-v0.5.0-beta.1"))
.disable_redirects()
.finish();
@ -58,7 +59,9 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
fn init_logger(
opentelemetry_url: Option<&Url>,
console_event_buffer_size: Option<usize>,
) -> Result<(), Box<dyn std::error::Error>> {
) -> color_eyre::Result<()> {
color_eyre::install()?;
LogTracer::init()?;
opentelemetry::global::set_text_map_propagator(TraceContextPropagator::new());
@ -67,9 +70,7 @@ fn init_logger(
.unwrap_or_else(|_| "info".into())
.parse()?;
let format_layer = tracing_subscriber::fmt::layer()
.with_span_events(FmtSpan::NEW | FmtSpan::CLOSE)
.with_filter(targets.clone());
let format_layer = tracing_subscriber::fmt::layer().with_filter(targets.clone());
let subscriber = Registry::default()
.with(format_layer)
@ -94,7 +95,7 @@ fn init_subscriber<S>(
subscriber: S,
targets: Targets,
opentelemetry_url: Option<&Url>,
) -> Result<(), Box<dyn std::error::Error>>
) -> color_eyre::Result<()>
where
S: SubscriberExt + Send + Sync,
for<'a> S: LookupSpan<'a>,

View file

@ -54,6 +54,7 @@ pub(crate) struct Details {
#[derive(serde::Deserialize)]
pub(crate) struct Images {
msg: String,
code: Option<String>,
files: Option<Vec<Image>>,
}
@ -62,6 +63,10 @@ impl Images {
&self.msg
}
pub(crate) fn code(&self) -> Option<&str> {
self.code.as_deref()
}
pub(crate) fn files(&self) -> impl Iterator<Item = &Image> {
self.files.iter().flat_map(|v| v.iter())
}

View file

@ -390,19 +390,19 @@ impl Store {
#[derive(Debug, thiserror::Error)]
pub(crate) enum Error {
#[error("{0}")]
#[error("Error in stored json")]
Json(#[from] serde_json::Error),
#[error("{0}")]
#[error("Error parsing UUID")]
Uuid(#[from] uuid::Error),
#[error("{0}")]
#[error("Error in sled")]
Sled(#[from] sled::Error),
#[error("{0}")]
#[error("Error in sled transaction")]
Transaction(#[from] sled::transaction::TransactionError),
#[error("{0}")]
#[error("Error hashing or validating token")]
Bcrypt(#[from] bcrypt::BcryptError),
#[error("Panic in blocking operation")]

View file

@ -61,8 +61,7 @@ text_input_html, statics::file_upload_js};
<div class="button-group button-space">
@if let Some(upload_id) = entry.upload_id() {
<input type="hidden" name="upload_id" value="@upload_id" />
@:button_link_html("Refresh", &state.edit_collection_path(collection_id, Some(*id), token),
ButtonKind::Submit)
@:button_link_html("Refresh", "javascript:window.location.reload(true);", ButtonKind::Submit)
}
@if let Some((filename, delete_token)) = entry.file_parts() {
<input type="hidden" name="filename" value="@filename" />