Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

editoast: enhance error logging #4815

Merged
merged 2 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions editoast/Cargo.lock

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

1 change: 1 addition & 0 deletions editoast/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ osm4routing = "0.5.8"
osmpbfreader = "0.16.0"
itertools = "0.11"
utoipa = { version = "3.5", features = ["actix_extras"] }
atty = "0.2.14"

[dev-dependencies]
async-std = { version = "1.12", features = ["attributes", "tokio1"] }
Expand Down
13 changes: 12 additions & 1 deletion editoast/src/client/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
mod postgres_config;
mod redis_config;

use clap::{Args, Parser, Subcommand};
use clap::{Args, Parser, Subcommand, ValueEnum};
use derivative::Derivative;
pub use postgres_config::PostgresConfig;
pub use redis_config::RedisConfig;
Expand All @@ -14,10 +14,21 @@ pub struct Client {
pub postgres_config: PostgresConfig,
#[command(flatten)]
pub redis_config: RedisConfig,
#[arg(long, env, value_enum, default_value_t = Color::Auto)]
pub color: Color,
#[command(subcommand)]
pub command: Commands,
}

#[derive(ValueEnum, Debug, Derivative, Clone)]
#[derivative(Default)]
pub enum Color {
Never,
Always,
#[derivative(Default)]
Auto,
}

#[derive(Subcommand, Debug)]
pub enum Commands {
Runserver(RunserverArgs),
Expand Down
20 changes: 20 additions & 0 deletions editoast/src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::marker::PhantomData;

use crate::error::Result;
use async_trait::async_trait;
use colored::{ColoredString, Colorize};
use editoast_derive::EditoastError;
pub use http_client::{HttpClient, HttpClientBuilder};
use log::info;
Expand All @@ -22,6 +23,19 @@ use thiserror::Error;

const MAX_RETRIES: u8 = 5;

fn colored_method(method: &reqwest::Method) -> ColoredString {
let m = method.as_str();
match *method {
reqwest::Method::GET => m.green(),
reqwest::Method::POST => m.yellow(),
reqwest::Method::PUT => m.blue(),
reqwest::Method::PATCH => m.magenta(),
reqwest::Method::DELETE => m.red(),
_ => m.normal(),
}
.bold()
}

#[derive(Debug, Clone)]
pub enum CoreClient {
Direct(HttpClient),
Expand Down Expand Up @@ -51,6 +65,9 @@ impl CoreClient {
path: &str,
body: Option<&B>,
) -> Result<R::Response> {
let method_s = colored_method(&method);
log::info!(target: "editoast::coreclient", "{method_s} {path}");
log::debug!(target: "editoast::coreclient", "Request content: {body}", body = body.and_then(|b| serde_json::to_string_pretty(b).ok()).unwrap_or_default());
match self {
CoreClient::Direct(client) => {
let mut i_try = 0;
Expand Down Expand Up @@ -84,8 +101,11 @@ impl CoreClient {
msg: err.to_string(),
})?;
if status.is_success() {
log::info!(target: "editoast::coreclient", "{method_s} {path} {status}", status = status.to_string().bold().green());
return R::from_bytes(bytes.as_ref());
}

log::error!(target: "editoast::coreclient", "{method_s} {path} {status}", status = status.to_string().bold().red());
// We try to deserialize the response as the standard Core error format
// If that fails we try to return a generic error containing the raw error
let core_error = <Json<CoreErrorPayload> as CoreResponse>::from_bytes(
Expand Down
8 changes: 8 additions & 0 deletions editoast/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use actix_web::{error::JsonPayloadError, http::StatusCode, HttpResponse, ResponseError};
use colored::Colorize;
use diesel::result::Error as DieselError;
use redis::RedisError;
use serde::{Deserialize, Serialize};
use serde_json::{json, Value};
use std::backtrace::Backtrace;
use std::collections::HashMap;
use std::result::Result as StdResult;
use std::{
Expand Down Expand Up @@ -77,6 +79,12 @@ impl ResponseError for InternalError {
}

fn error_response(&self) -> HttpResponse {
log::error!(
"[{}] {}: {}",
self.error_type.bold(),
self.message,
Backtrace::capture() // won't log unless RUST_BACKTRACE=1
);
HttpResponse::build(self.status).json(self)
}
}
Expand Down
8 changes: 7 additions & 1 deletion editoast/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use actix_web::{App, HttpServer};
use chashmap::CHashMap;
use clap::Parser;
use client::{
ClearArgs, Client, Commands, GenerateArgs, ImportProfileSetArgs, ImportRailjsonArgs,
ClearArgs, Client, Color, Commands, GenerateArgs, ImportProfileSetArgs, ImportRailjsonArgs,
PostgresConfig, RedisConfig, RunserverArgs,
};
use colored::*;
Expand Down Expand Up @@ -71,6 +71,12 @@ async fn run() -> Result<(), Box<dyn Error + Send + Sync>> {
let pg_config = client.postgres_config;
let redis_config = client.redis_config;

match client.color {
Color::Never => colored::control::set_override(false),
Color::Always => colored::control::set_override(true),
Color::Auto => colored::control::set_override(atty::is(atty::Stream::Stderr)),
}

match client.command {
Commands::Runserver(args) => runserver(args, pg_config, redis_config).await,
Commands::Generate(args) => generate(args, pg_config, redis_config).await,
Expand Down