editoast: forward core error type

editoast/openapi.yaml

@@ -539,7 +539,6 @@ components:
         type:
           type: string
       required:
-      - status
       - type
       - context
       - message

editoast/src/core/mocking.rs

@@ -19,6 +19,13 @@ impl From<MockingClient> for CoreClient {
     }
 }
 
+#[derive(Debug)]
+pub struct MockingError {
+    pub bytes: Vec<u8>,
+    pub status: StatusCode,
+    pub url: String,
+}
+
 impl MockingClient {
     pub fn new() -> Self {
         Default::default()
@@ -35,7 +42,7 @@ impl MockingClient {
         method: reqwest::Method,
         req_path: P,
         body: Option<&B>,
-    ) -> Option<R::Response> {
+    ) -> Result<Option<R::Response>, MockingError> {
         let req_path = req_path.as_ref().to_string();
         let stub = 'find_stub: {
             for stub in &self.stubs {
@@ -71,16 +78,33 @@ impl MockingClient {
             (None, Some(expected)) => panic!("missing request body: '{expected}'"),
             _ => (),
         }
-        stub.response
+        let response = stub
+            .response
             .as_ref()
             .and_then(|r| r.body.as_ref())
             .map(|b| {
+                b.as_bytes()
+                    .expect("mocked response body should not be empty when specified")
+            });
+        match stub.response {
+            None => Ok(None),
+            Some(StubResponse { code, .. }) if code.is_success() => Ok(Some(
                 R::from_bytes(
-                    b.as_bytes()
-                        .expect("mocked response body should not be empty when specified"),
+                    response.expect("mocked response body should not be empty when specified"),
                 )
-                .expect("mocked response body should deserialize faultlessly")
-            })
+                .expect("mocked response body should deserialize faultlessly"),
+            )),
+            Some(StubResponse { code, .. }) => {
+                let err = response
+                    .expect("mocked response body should not be empty when specified")
+                    .to_vec();
+                Err(MockingError {
+                    bytes: err,
+                    status: code,
+                    url: req_path,
+                })
+            }
+        }
     }
 }
 

editoast/src/core/mod.rs

@@ -9,9 +9,10 @@ pub mod simulation;
 pub mod stdcm;
 pub mod version;
 
-use std::marker::PhantomData;
+use std::{collections::HashMap, fmt::Display, marker::PhantomData};
 
 use crate::error::{InternalError, Result};
+use actix_http::StatusCode;
 use async_trait::async_trait;
 use colored::{ColoredString, Colorize};
 use editoast_derive::EditoastError;
@@ -20,8 +21,12 @@ use log::info;
 use reqwest::Url;
 use serde::{de::DeserializeOwned, Serialize};
 use serde_derive::Deserialize;
+use serde_json::Value;
 use thiserror::Error;
 
+#[cfg(test)]
+use crate::core::mocking::MockingError;
+
 const MAX_RETRIES: u8 = 5;
 
 fn colored_method(method: &reqwest::Method) -> ColoredString {
@@ -60,6 +65,23 @@ impl CoreClient {
         Self::Direct(client)
     }
 
+    fn handle_error(
+        &self,
+        bytes: &[u8],
+        status: reqwest::StatusCode,
+        url: String,
+    ) -> InternalError {
+        // 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) = <Json<StandardCoreError>>::from_bytes(bytes) {
+            core_error.context.insert("url".to_owned(), url.into());
+            let mut internal_error: InternalError = core_error.into();
+            internal_error.set_status(status);
+            return internal_error;
+        }
+
+        CoreError::UnparsableErrorOutput.into()
+    }
+
     async fn fetch<B: Serialize, R: CoreResponse>(
         &self,
         method: reqwest::Method,
@@ -107,39 +129,18 @@ impl CoreClient {
                 }
 
                 log::error!(target: "editoast::coreclient", "{method_s} {path} {status}", status = status.to_string().bold().red());
-
-                // We try to deserialize the response as an InternalError in order to retain the context of the core error
-                if let Ok(mut internal_error) = <Json<InternalError>>::from_bytes(bytes.as_ref()) {
-                    internal_error.set_status(status);
-                    return Err(internal_error);
-                }
-
-                // 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>>::from_bytes(bytes.as_ref()).map_err(|err| {
-                        if let Ok(utf8_raw_error) = String::from_utf8(bytes.as_ref().to_vec()) {
-                            CoreError::GenericCoreError {
-                                status: Some(status.as_u16()),
-                                url: url.clone(),
-                                raw_error: utf8_raw_error,
-                            }
-                            .into()
-                        } else {
-                            err
-                        }
-                    })?;
-                Err(CoreError::Forward {
-                    status: status.as_u16(),
-                    core_error,
-                    url,
-                }
-                .into())
+                Err(self.handle_error(bytes.as_ref(), status, url))
             }
             #[cfg(test)]
-            CoreClient::Mocked(client) => client
-                .fetch_mocked::<_, B, R>(method, path, body)
-                .ok_or(CoreError::NoResponseContent.into()),
+            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(self.handle_error(&bytes, status, url))
+                    }
+                }
+            }
         }
     }
 }
@@ -271,16 +272,6 @@ impl CoreResponse for () {
     }
 }
 
-/// The structure of a standard core error (cf. class OSRDError)
-#[derive(Debug, Serialize, Deserialize)]
-struct CoreErrorPayload {
-    #[serde(rename = "type")]
-    type_: String,
-    cause: Option<String>,
-    message: String,
-    trace: Option<serde_json::Value>,
-}
-
 #[allow(clippy::enum_variant_names)]
 #[derive(Debug, Error, EditoastError)]
 #[editoast_error(base_id = "coreclient")]
@@ -291,13 +282,6 @@ enum CoreError {
     #[error("Cannot parse Core response: {msg}")]
     #[editoast_error(status = 500)]
     CoreResponseFormatError { msg: String },
-    /// A standard core error was found in the response, so it is forwarded
-    #[error("{}", core_error.message)]
-    Forward {
-        status: u16,
-        core_error: CoreErrorPayload,
-        url: 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))]
@@ -307,6 +291,8 @@ enum CoreError {
         url: String,
         raw_error: String,
     },
+    #[error("Core returned an error in an unknown format")]
+    UnparsableErrorOutput,
 
     #[error("Core connection closed before message completed. Should retry.")]
     #[editoast_error(status = 500)]
@@ -316,6 +302,38 @@ enum CoreError {
     NoResponseContent,
 }
 
+#[derive(Debug, Deserialize)]
+pub struct StandardCoreError {
+    #[serde(skip)]
+    status: StatusCode,
+    #[serde(rename = "type")]
+    error_type: String,
+    context: HashMap<String, Value>,
+    message: String,
+}
+
+impl crate::error::EditoastError for StandardCoreError {
+    fn get_type(&self) -> &str {
+        &self.error_type
+    }
+
+    fn get_status(&self) -> StatusCode {
+        self.status
+    }
+
+    fn context(&self) -> HashMap<String, Value> {
+        self.context.clone()
+    }
+}
+
+impl std::error::Error for StandardCoreError {}
+
+impl Display for StandardCoreError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.message)
+    }
+}
+
 impl From<reqwest::Error> for CoreError {
     fn from(value: reqwest::Error) -> Self {
         // Since we should retry the request it's useful to have its own kind of error.
@@ -341,10 +359,15 @@ impl From<reqwest::Error> for CoreError {
 #[cfg(test)]
 mod test {
     use actix_http::StatusCode;
+    use pretty_assertions::assert_eq;
     use reqwest::Method;
     use serde_derive::Serialize;
+    use serde_json::json;
 
-    use crate::core::{mocking::MockingClient, AsCoreRequest, Bytes};
+    use crate::{
+        core::{mocking::MockingClient, AsCoreRequest, Bytes},
+        error::InternalError,
+    };
 
     #[rstest::rstest]
     async fn test_expected_empty_response() {
@@ -381,4 +404,37 @@ mod test {
         let bytes = Req.fetch(&core.into()).await.unwrap();
         assert_eq!(&String::from_utf8(bytes).unwrap(), "not JSON :)");
     }
+
+    #[rstest::rstest]
+    async fn test_core_osrd_error() {
+        #[derive(Serialize)]
+        struct Req;
+        impl AsCoreRequest<()> for Req {
+            const METHOD: Method = Method::GET;
+            const URL_PATH: &'static str = "/test";
+        }
+        let error = json!({
+            "context": {
+                "stack_trace": [
+                    "ThreadPoolExecutor.java:635",
+                    "Thread.java:833"
+                ],
+                "message": "conflict offset is already on a range transition",
+                "url": "/test"
+            },
+            "message": "assert check failed",
+            "type": "assert_error",
+        });
+        let mut core = MockingClient::default();
+        core.stub("/test")
+            .method(Method::GET)
+            .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 result = Req.fetch(&core.into()).await;
+        let expected_err: InternalError = error_with_status;
+        assert_eq!(result, Err(expected_err));
+    }
 }

editoast/src/error.rs

@@ -40,15 +40,19 @@ impl From<StatusCodeRemoteDef> for StatusCode {
     }
 }
 
-#[derive(Debug, Serialize, Deserialize, ToSchema)]
+fn default_status_code() -> StatusCode {
+    StatusCode::INTERNAL_SERVER_ERROR
+}
+
+#[derive(Debug, Serialize, Deserialize, ToSchema, PartialEq)]
 pub struct InternalError {
-    #[serde(with = "StatusCodeRemoteDef")]
+    #[serde(with = "StatusCodeRemoteDef", default = "default_status_code")]
     #[schema(value_type = u16, minimum = 100, maximum = 599)]
-    status: StatusCode,
+    pub status: StatusCode,
     #[serde(rename = "type")]
-    error_type: String,
-    context: HashMap<String, Value>,
-    message: String,
+    pub error_type: String,
+    pub context: HashMap<String, Value>,
+    pub message: String,
 }
 
 impl InternalError {

front/src/common/api/osrdEditoastApi.ts

@@ -1361,7 +1361,7 @@ export type InternalError = {
     [key: string]: any;
   };
   message: string;
-  status: number;
+  status?: number;
   type: string;
 };
 export type TrackRange = {