diff --git a/editoast/openapi.yaml b/editoast/openapi.yaml index e6eb16f1d91..6c06c6ab08a 100644 --- a/editoast/openapi.yaml +++ b/editoast/openapi.yaml @@ -4749,155 +4749,6 @@ components: type: string enum: - editoast:cache_operation:ObjectNotFound - EditoastCoreErrorBrokenPipe: - type: object - required: - - type - - status - - message - properties: - context: - type: object - message: - type: string - status: - type: integer - enum: - - 500 - type: - type: string - enum: - - editoast:coreclient:BrokenPipe - EditoastCoreErrorConnectionClosedBeforeMessageCompleted: - type: object - required: - - type - - status - - message - properties: - context: - type: object - message: - type: string - status: - type: integer - enum: - - 500 - type: - type: string - enum: - - editoast:coreclient:ConnectionClosedBeforeMessageCompleted - EditoastCoreErrorConnectionResetByPeer: - type: object - required: - - type - - status - - message - properties: - context: - type: object - message: - type: string - status: - type: integer - enum: - - 500 - type: - type: string - enum: - - editoast:coreclient:ConnectionResetByPeer - EditoastCoreErrorCoreResponseFormatError: - type: object - required: - - type - - status - - message - properties: - context: - type: object - required: - - msg - properties: - msg: - type: string - message: - type: string - status: - type: integer - enum: - - 500 - type: - type: string - enum: - - editoast:coreclient:CoreResponseFormatError - EditoastCoreErrorGenericCoreError: - type: object - required: - - type - - status - - message - properties: - context: - type: object - required: - - raw_error - - status - - url - properties: - raw_error: - type: string - status: - type: object - url: - type: string - message: - type: string - status: - type: integer - enum: - - 400 - type: - type: string - enum: - - editoast:coreclient:GenericCoreError - EditoastCoreErrorMqClientError: - type: object - required: - - type - - status - - message - properties: - context: - type: object - message: - type: string - status: - type: integer - enum: - - 500 - type: - type: string - enum: - - editoast:coreclient:MqClientError - EditoastCoreErrorUnparsableErrorOutput: - type: object - required: - - type - - status - - message - properties: - context: - type: object - message: - type: string - status: - type: integer - enum: - - 400 - type: - type: string - enum: - - editoast:coreclient:UnparsableErrorOutput EditoastDatabaseAccessErrorDatabaseAccessError: type: object required: @@ -5106,13 +4957,6 @@ components: - $ref: '#/components/schemas/EditoastAutoFixesEditoastErrorMissingErrorObject' - $ref: '#/components/schemas/EditoastCacheOperationErrorDuplicateIdsProvided' - $ref: '#/components/schemas/EditoastCacheOperationErrorObjectNotFound' - - $ref: '#/components/schemas/EditoastCoreErrorBrokenPipe' - - $ref: '#/components/schemas/EditoastCoreErrorConnectionClosedBeforeMessageCompleted' - - $ref: '#/components/schemas/EditoastCoreErrorConnectionResetByPeer' - - $ref: '#/components/schemas/EditoastCoreErrorCoreResponseFormatError' - - $ref: '#/components/schemas/EditoastCoreErrorGenericCoreError' - - $ref: '#/components/schemas/EditoastCoreErrorMqClientError' - - $ref: '#/components/schemas/EditoastCoreErrorUnparsableErrorOutput' - $ref: '#/components/schemas/EditoastDatabaseAccessErrorDatabaseAccessError' - $ref: '#/components/schemas/EditoastDelimitedAreaErrorInvalidLocations' - $ref: '#/components/schemas/EditoastDocumentErrorsDatabase' diff --git a/editoast/src/core/mocking.rs b/editoast/src/core/mocking.rs index 65f87b04516..05f76d69e60 100644 --- a/editoast/src/core/mocking.rs +++ b/editoast/src/core/mocking.rs @@ -24,7 +24,6 @@ impl From for CoreClient { #[derive(Debug)] pub struct MockingError { pub bytes: Vec, - pub status: StatusCode, pub url: String, } @@ -96,13 +95,12 @@ impl MockingClient { ) .expect("mocked response body should deserialize faultlessly"), )), - Some(StubResponse { code, .. }) => { + Some(StubResponse { .. }) => { let err = response .expect("mocked response body should not be empty when specified") .to_vec(); Err(MockingError { bytes: err, - status: code, url: req_path, }) } diff --git a/editoast/src/core/mod.rs b/editoast/src/core/mod.rs index 18afa03d8a3..c1c94c7d82d 100644 --- a/editoast/src/core/mod.rs +++ b/editoast/src/core/mod.rs @@ -15,7 +15,6 @@ use std::fmt::Display; use std::marker::PhantomData; use axum::http::StatusCode; -use editoast_derive::EditoastError; use mq_client::MqClientError; use serde::de::DeserializeOwned; use serde::Deserialize; @@ -27,8 +26,6 @@ use tracing::trace; #[cfg(test)] use crate::core::mocking::MockingError; -use crate::error::InternalError; -use crate::error::Result; pub use mq_client::RabbitMQClient; @@ -47,38 +44,28 @@ pub enum CoreClient { } impl CoreClient { - pub async fn new_mq(options: mq_client::Options) -> Result { + pub async fn new_mq(options: mq_client::Options) -> Result { let client = RabbitMQClient::new(options) .await - .map_err(CoreError::MqClientError)?; + .map_err(Error::MqClientError)?; Ok(Self::MessageQueue(client)) } - fn handle_error(&self, bytes: &[u8], url: String) -> InternalError { + fn handle_error(&self, bytes: &[u8], url: String) -> Error { // We try to deserialize the response as an StandardCoreError in order to retain the context of the core error if let Ok(mut core_error) = >::from_bytes(bytes) { - let status: u16 = match core_error.cause { - CoreErrorCause::Internal => 500, - CoreErrorCause::User => 400, - }; core_error.context.insert("url".to_owned(), url.into()); - let mut internal_error: InternalError = core_error.into(); - internal_error.set_status(StatusCode::from_u16(status).unwrap()); - return internal_error; + return Error::StandardCoreError(core_error); } - - let error: InternalError = CoreError::UnparsableErrorOutput.into(); - let mut error = error.with_context("url", url); - error.set_status(StatusCode::INTERNAL_SERVER_ERROR); - error + Error::UnparsableErrorOutput } #[tracing::instrument(name = "ping_core", skip_all)] - pub async fn ping(&self) -> Result { + pub async fn ping(&self) -> Result { match self { CoreClient::MessageQueue(mq_client) => { - mq_client.ping().await.map_err(|_| CoreError::BrokenPipe) + mq_client.ping().await.map_err(|_| Error::BrokenPipe) } #[cfg(test)] CoreClient::Mocked(_) => Ok(true), @@ -97,7 +84,7 @@ impl CoreClient { path: &str, body: Option<&B>, infra_id: Option, - ) -> Result { + ) -> Result { trace!( target: "editoast::coreclient", body = body.and_then(|b| serde_json::to_string_pretty(b).ok()).unwrap_or_default(), @@ -112,7 +99,7 @@ impl CoreClient { let response = client .call_with_response(infra_id.to_string(), path, &body, true, None) .await - .map_err(CoreError::MqClientError)?; + .map_err(Error::MqClientError)?; if response.status == b"ok" { return R::from_bytes(&response.payload); @@ -128,12 +115,8 @@ impl CoreClient { CoreClient::Mocked(client) => { match client.fetch_mocked::<_, B, R>(method, path, body) { Ok(Some(response)) => Ok(response), - Ok(None) => Err(CoreError::NoResponseContent.into()), - Err(MockingError { bytes, status, url }) => Err({ - let mut err = self.handle_error(&bytes, url); - err.set_status(status); - err - }), + Ok(None) => Err(Error::NoResponseContent), + Err(MockingError { bytes, url }) => Err(self.handle_error(&bytes, url)), } } } @@ -199,12 +182,12 @@ where /// Sends this request using the given [CoreClient] and returns the response content on success /// - /// Raises a [CoreError] if the request is not a success. + /// Raises a [Error] if the request is not a success. /// /// TODO: provide a mechanism in this trait to allow the implementer to /// manage itself its expected errors. Maybe a bound error type defaulting /// to CoreError and a trait function handle_errors would suffice? - async fn fetch(&self, core: &CoreClient) -> Result { + async fn fetch(&self, core: &CoreClient) -> Result { core.fetch::( self.method(), self.url(), @@ -221,7 +204,7 @@ pub trait CoreResponse { type Response; /// Reads the content of `bytes` and produces the response object - fn from_bytes(bytes: &[u8]) -> Result; + fn from_bytes(bytes: &[u8]) -> Result; } /// Indicates that the response that deserializes to `T` is expected to have a Json body @@ -233,12 +216,9 @@ pub struct Bytes; impl CoreResponse for Json { type Response = T; - fn from_bytes(bytes: &[u8]) -> Result { - serde_json::from_slice(bytes).map_err(|err| { - CoreError::CoreResponseFormatError { - msg: err.to_string(), - } - .into() + fn from_bytes(bytes: &[u8]) -> Result { + serde_json::from_slice(bytes).map_err(|err| Error::CoreResponseFormatError { + msg: err.to_string(), }) } } @@ -246,7 +226,7 @@ impl CoreResponse for Json { impl CoreResponse for Bytes { type Response = Vec; - fn from_bytes(bytes: &[u8]) -> Result { + fn from_bytes(bytes: &[u8]) -> Result { Ok(Vec::from_iter(bytes.iter().cloned())) } } @@ -254,22 +234,19 @@ impl CoreResponse for Bytes { impl CoreResponse for () { type Response = (); - fn from_bytes(_: &[u8]) -> Result { + fn from_bytes(_: &[u8]) -> Result { Ok(()) } } #[allow(clippy::enum_variant_names)] -#[derive(Debug, Error, EditoastError)] -#[editoast_error(base_id = "coreclient")] -pub enum CoreError { +#[derive(Debug, Error)] +pub enum Error { #[error("Cannot parse Core response: {msg}")] - #[editoast_error(status = 500)] CoreResponseFormatError { msg: String }, /// A fallback error variant for when no meaningful error could be parsed /// from core's output. #[error("Core error {}: {raw_error}", status.unwrap_or(500))] - #[editoast_error(status = status.unwrap_or(500))] GenericCoreError { status: Option, url: String, @@ -279,25 +256,24 @@ pub enum CoreError { UnparsableErrorOutput, #[error("Core connection closed before message completed. Should retry.")] - #[editoast_error(status = 500)] ConnectionClosedBeforeMessageCompleted, #[error("Core connection reset by peer. Should retry.")] - #[editoast_error(status = 500)] ConnectionResetByPeer, #[error("Core connection broken. Should retry.")] - #[editoast_error(status = 500)] BrokenPipe, #[error(transparent)] - #[editoast_error(status = "500")] - MqClientError(MqClientError), + MqClientError(#[from] MqClientError), + + #[error(transparent)] + StandardCoreError(#[from] StandardCoreError), #[cfg(test)] #[error("The mocked response had no body configured - check out StubResponseBuilder::body if this is unexpected")] NoResponseContent, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, PartialEq)] pub struct StandardCoreError { #[serde(skip)] status: StatusCode, @@ -306,10 +282,10 @@ pub struct StandardCoreError { context: HashMap, message: String, #[serde(default = "CoreErrorCause::default")] - cause: CoreErrorCause, + pub(crate) cause: CoreErrorCause, } -#[derive(Debug, Deserialize, Default)] +#[derive(Debug, Deserialize, Default, PartialEq)] pub enum CoreErrorCause { #[default] Internal, @@ -338,7 +314,7 @@ impl Display for StandardCoreError { } } -impl From for CoreError { +impl From for Error { fn from(value: reqwest::Error) -> Self { // Since we should retry the request it's useful to have its own kind of error. if value @@ -371,6 +347,7 @@ impl From for CoreError { #[cfg(test)] mod tests { + use axum::http::StatusCode; use pretty_assertions::assert_eq; use reqwest::Method; @@ -380,7 +357,9 @@ mod tests { use crate::core::mocking::MockingClient; use crate::core::AsCoreRequest; use crate::core::Bytes; - use crate::error::InternalError; + use crate::core::StandardCoreError; + + use super::Error; #[rstest::rstest] async fn test_expected_empty_response() { @@ -456,10 +435,13 @@ mod tests { .response(StatusCode::NOT_FOUND) .body(error.to_string()) .finish(); - let mut error_with_status: InternalError = serde_json::from_value(error).unwrap(); - error_with_status.set_status(StatusCode::NOT_FOUND); + let expected_error: StandardCoreError = serde_json::from_value(error).unwrap(); let result = Req.fetch(&core.into()).await; - let expected_err: InternalError = error_with_status; - assert_eq!(result, Err(expected_err)); + let result = result.err().unwrap(); + if let Error::StandardCoreError(standard_core_error) = result { + assert_eq!(standard_core_error, expected_error); + } else { + panic!("Unexpected error type: {:?}", result); + } } } diff --git a/editoast/src/views/core_error_response.rs b/editoast/src/views/core_error_response.rs new file mode 100644 index 00000000000..41aca16a5ab --- /dev/null +++ b/editoast/src/views/core_error_response.rs @@ -0,0 +1,69 @@ +use axum::http::StatusCode; + +use crate::core::CoreErrorCause; +use crate::core::Error; +use crate::error::InternalError; + +impl From for InternalError { + fn from(core_error: Error) -> Self { + let status = match core_error { + Error::UnparsableErrorOutput => StatusCode::BAD_REQUEST, + Error::StandardCoreError(ref error) => match error.cause { + CoreErrorCause::Internal => StatusCode::INTERNAL_SERVER_ERROR, + CoreErrorCause::User => StatusCode::BAD_REQUEST, + }, + _ => StatusCode::INTERNAL_SERVER_ERROR, + }; + + let error_type = match core_error { + Error::CoreResponseFormatError { .. } => { + "editoast:coreclient:CoreResponseFormatError".to_string() + } + Error::GenericCoreError { .. } => "editoast:coreclient:GenericCoreError".to_string(), + Error::UnparsableErrorOutput => "editoast:coreclient:UnparsableErrorOutput".to_string(), + Error::ConnectionClosedBeforeMessageCompleted => { + "editoast:coreclient:ConnectionClosedBeforeMessageCompleted".to_string() + } + Error::ConnectionResetByPeer => "editoast:coreclient:ConnectionResetByPeer".to_string(), + Error::BrokenPipe => "editoast:coreclient:BrokenPipe".to_string(), + Error::MqClientError(_) => "editoast:coreclient:MqClientError".to_string(), + Error::StandardCoreError(_) => "editoast:coreclient:StandardCoreError".to_string(), + #[cfg(test)] + Error::NoResponseContent => "editoast:coreclient:NoResponseContent".to_string(), + }; + + let context = match core_error { + Error::CoreResponseFormatError { ref msg } => { + [("msg".to_string(), serde_json::to_value(msg).unwrap())].into() + } + Error::GenericCoreError { + status, + ref url, + ref raw_error, + } => [ + ("status".to_string(), serde_json::to_value(status).unwrap()), + ("url".to_string(), serde_json::to_value(url).unwrap()), + ( + "raw_error".to_string(), + serde_json::to_value(raw_error).unwrap(), + ), + ] + .into(), + Error::UnparsableErrorOutput => Default::default(), + Error::ConnectionClosedBeforeMessageCompleted => Default::default(), + Error::ConnectionResetByPeer => Default::default(), + Error::BrokenPipe => Default::default(), + Error::MqClientError(_) => Default::default(), + Error::StandardCoreError(_) => Default::default(), + #[cfg(test)] + Error::NoResponseContent => Default::default(), + }; + + Self { + status, + error_type, + context, + message: core_error.to_string(), + } + } +} diff --git a/editoast/src/views/mod.rs b/editoast/src/views/mod.rs index 430e440974b..180b6b07a34 100644 --- a/editoast/src/views/mod.rs +++ b/editoast/src/views/mod.rs @@ -1,4 +1,5 @@ mod authz; +mod core_error_response; mod documents; pub mod electrical_profiles; pub mod fonts; @@ -74,14 +75,13 @@ use utoipa::IntoParams; use utoipa::ToSchema; use crate::client::get_app_version; +use crate::core; use crate::core::mq_client; use crate::core::pathfinding::PathfindingInputError; use crate::core::pathfinding::PathfindingNotFound; use crate::core::version::CoreVersionRequest; use crate::core::AsCoreRequest; use crate::core::CoreClient; -use crate::core::CoreError; -use crate::core::{self}; use crate::error::InternalError; use crate::error::Result; use crate::error::{self}; @@ -292,7 +292,7 @@ async fn authentication_middleware( Ok(next.run(req).await) } -#[derive(Debug, Error, EditoastError)] +#[derive(Debug, thiserror::Error, EditoastError)] #[editoast_error(base_id = "authz")] pub enum AuthorizationError { #[error("Unauthorized — user must be authenticated")] @@ -321,9 +321,9 @@ pub enum AppHealthError { #[error(transparent)] Valkey(anyhow::Error), #[error(transparent)] - Core(#[from] CoreError), - #[error(transparent)] Openfga(anyhow::Error), + #[error(transparent)] + Core(#[from] core::Error), } #[utoipa::path( diff --git a/editoast/src/views/path/pathfinding.rs b/editoast/src/views/path/pathfinding.rs index c6cbd2586bc..78d021a3003 100644 --- a/editoast/src/views/path/pathfinding.rs +++ b/editoast/src/views/path/pathfinding.rs @@ -315,9 +315,9 @@ async fn pathfinding_blocks_batch( path.into() } // TODO: only make HTTP status code errors non-fatal - Err(core_error) => { - PathfindingResult::Failure(PathfindingFailure::InternalError { core_error }) - } + Err(core_error) => PathfindingResult::Failure(PathfindingFailure::InternalError { + core_error: core_error.into(), + }), }; hash_to_path_indexes[hash] .iter() diff --git a/editoast/src/views/timetable/stdcm.rs b/editoast/src/views/timetable/stdcm.rs index ce3ef24d605..746dd9af20b 100644 --- a/editoast/src/views/timetable/stdcm.rs +++ b/editoast/src/views/timetable/stdcm.rs @@ -32,6 +32,7 @@ use tracing_opentelemetry::OpenTelemetrySpanExt; use utoipa::IntoParams; use utoipa::ToSchema; +use crate::core; use crate::core::conflict_detection::Conflict; use crate::core::conflict_detection::TrainRequirements; use crate::core::pathfinding::InvalidPathItem; @@ -40,6 +41,7 @@ use crate::core::simulation::PhysicsConsistParameters; use crate::core::simulation::{RoutingRequirement, SimulationResponse, SpacingRequirement}; use crate::core::AsCoreRequest; use crate::core::CoreClient; +use crate::error::InternalError; use crate::error::Result; use crate::models::prelude::*; use crate::models::stdcm_log::{StdcmLog, StdcmResponseOrError}; @@ -292,7 +294,10 @@ async fn stdcm( .collect(), }; - let stdcm_response = stdcm_request.fetch(core_client.as_ref()).await; + let stdcm_response: Result = stdcm_request + .fetch(core_client.as_ref()) + .await + .map_err(Into::into); // 6. Log STDCM request and response if logging is enabled if config.enable_stdcm_logging { @@ -309,10 +314,11 @@ async fn stdcm( }, ); - let stdcm_response = match stdcm_response.clone() { - Ok(response) => StdcmResponseOrError::Response(response), - Err(error) => { - StdcmResponseOrError::RequestError(serde_json::to_value(error).unwrap_or( + let stdcm_response = match stdcm_response { + Ok(ref response) => StdcmResponseOrError::Response(response.clone()), + Err(ref error) => { + let error: InternalError = error.clone(); + StdcmResponseOrError::RequestError(serde_json::to_value(error.clone()).unwrap_or( serde_json::Value::String("Failed to serialize the error".into()), )) } diff --git a/editoast/src/views/train_schedule.rs b/editoast/src/views/train_schedule.rs index 565cce6e9e7..2a560150a59 100644 --- a/editoast/src/views/train_schedule.rs +++ b/editoast/src/views/train_schedule.rs @@ -43,6 +43,7 @@ use crate::core::simulation::SimulationResponse; use crate::core::simulation::SimulationScheduleItem; use crate::core::AsCoreRequest; use crate::core::CoreClient; +use crate::error::InternalError; use crate::error::Result; use crate::models::infra::Infra; use crate::models::prelude::*; @@ -527,11 +528,14 @@ pub async fn consist_train_simulation_batch( .iter() .for_each(|index| simulation_results[*index] = sim_res.clone()) } - Err(core_error) => train_indexes.iter().for_each(|index| { - simulation_results[*index] = SimulationResponse::SimulationFailed { - core_error: core_error.clone(), - } - }), + Err(core_error) => { + let error: InternalError = core_error.into(); + train_indexes.iter().for_each(|index| { + simulation_results[*index] = SimulationResponse::SimulationFailed { + core_error: error.clone(), + } + }) + } } } diff --git a/front/public/locales/en/errors.json b/front/public/locales/en/errors.json index c67412b5f62..d7f2e6ecc06 100644 --- a/front/public/locales/en/errors.json +++ b/front/public/locales/en/errors.json @@ -36,14 +36,7 @@ "ObjectNotFound": "{{obj_type}} {{obj_id}} could not be found everywhere in the infrastructure cache" }, "coreclient": { - "BrokenPipe": "Core connection broken pipe. Should retry.", - "CannotExtractResponseBody": "Cannot extract Core response body: {{msg}}", - "ConnectionClosedBeforeMessageCompleted": "Core connection closed before message completed. Should retry.", - "ConnectionResetByPeer": "Core connection reset by peer. Should retry.", - "CoreResponseFormatError": "Cannot parse Core response: {{msg}}", - "GenericCoreError": "Core error {{raw_error}}", - "UnparsableErrorOutput": "Core returned an error in an unknown format", - "MqClientError": "Core: MQ client error" + "CannotExtractResponseBody": "Cannot extract Core response body: {{msg}}" }, "DatabaseAccessError": "Database access fatal error", "delimited_area": { diff --git a/front/public/locales/fr/errors.json b/front/public/locales/fr/errors.json index e433ed94c18..1739625432d 100644 --- a/front/public/locales/fr/errors.json +++ b/front/public/locales/fr/errors.json @@ -36,14 +36,7 @@ "ObjectNotFound": "{{obj_type}} {{obj_id}} n'a pu être trouvé nulle part dans le cache de l'infrastructure" }, "coreclient": { - "BrokenPipe": "Core: connexion interrompue. Nouvelle tentative.", - "CannotExtractResponseBody": "Core: Impossible d'extraire le corps de la réponse : {{msg}}", - "ConnectionClosedBeforeMessageCompleted": "Core: connexion fermée avant la fin du message. Nouvelle tentative.", - "ConnectionResetByPeer": "Core: réinitialisation de la connexion. Nouvelle tentative.", - "CoreResponseFormatError": "Core: impossible d'analyser la réponse '{{msg}}'", - "GenericCoreError": "Core: erreur {{raw_error}}", - "UnparsableErrorOutput": "Core: a renvoyé une erreur dans un format inconnu", - "MqClientError": "Core: erreur du client MQ" + "CannotExtractResponseBody": "Core: Impossible d'extraire le corps de la réponse : {{msg}}" }, "delimited_area": { "InvalidLocations": "Certaines localisations sont invalides"