Skip to content

Commit 1a6f5ac

Browse files
committed
handle core errors properly
1 parent 1aacf1b commit 1a6f5ac

File tree

5 files changed

+68
-25
lines changed

5 files changed

+68
-25
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package fr.sncf.osrd.reporting.exceptions;
22

3+
import com.squareup.moshi.Json;
4+
35
public enum ErrorCause {
6+
@Json(name = "Internal")
47
INTERNAL,
8+
@Json(name = "User")
59
USER
610
}

core/osrd-reporting/src/main/java/fr/sncf/osrd/reporting/exceptions/OSRDError.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public final class OSRDError extends RuntimeException {
3737
public Map<String, Object> context = new HashMap<>();
3838

3939
public final transient ErrorType osrdErrorType;
40-
public final transient ErrorCause cause;
40+
public final ErrorCause cause;
4141

4242
/**
4343
* Constructs a new OSRDError with the specified error type.

core/src/main/java/fr/sncf/osrd/cli/WorkerCommand.kt

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,24 +169,30 @@ class WorkerCommand : CliCommand {
169169
)
170170
val span = tracer.spanBuilder(path).setParent(context).startSpan()
171171

172-
val payload = try {
172+
var payload: ByteArray
173+
var status: ByteArray
174+
try {
173175
span.makeCurrent().use { scope ->
174176
val response = endpoint.act(MQRequest(path, body))
175-
response
177+
payload = response
176178
.body()
177-
.readAllBytes() // TODO: check the response code too to catch error
179+
.readAllBytes() // TODO: check the response code too to catch
180+
val httpHeader = response.head().first()
181+
val statusCode = httpHeader.split(" ")[1]
182+
status = (if (statusCode[0] == '2') "ok" else "core_error").encodeToByteArray()
178183
}
179184
} catch (t: Throwable) {
180185
span.recordException(t)
181-
"ERROR, exception received".toByteArray() // TODO: have a valid payload for uncaught exceptions
186+
payload = "ERROR, exception received".toByteArray() // TODO: have a valid payload for uncaught exceptions
187+
status = "core_error".encodeToByteArray()
182188
} finally {
183189
span.end()
184190
}
185191

186192
if (replyTo != null) {
187193
val properties = AMQP.BasicProperties().builder()
188194
.correlationId(correlationId)
189-
.headers(mapOf("x-status" to "ok"))
195+
.headers(mapOf("x-status" to status))
190196
.build()
191197
channel.basicPublish("", replyTo, properties, payload)
192198
}

editoast/src/core/mod.rs

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,17 +97,16 @@ impl CoreClient {
9797
Ok(Self::MessageQueue(client))
9898
}
9999

100-
fn handle_error(
101-
&self,
102-
bytes: &[u8],
103-
status: reqwest::StatusCode,
104-
url: String,
105-
) -> InternalError {
100+
fn handle_error(&self, bytes: &[u8], url: String) -> InternalError {
106101
// We try to deserialize the response as an StandardCoreError in order to retain the context of the core error
107102
if let Ok(mut core_error) = <Json<StandardCoreError>>::from_bytes(bytes) {
103+
let status: u16 = match core_error.cause {
104+
CoreErrorCause::Internal => 500,
105+
CoreErrorCause::User => 400,
106+
};
108107
core_error.context.insert("url".to_owned(), url.into());
109108
let mut internal_error: InternalError = core_error.into();
110-
internal_error.set_status(StatusCode::from_u16(status.as_u16()).unwrap());
109+
internal_error.set_status(StatusCode::from_u16(status).unwrap());
111110
return internal_error;
112111
}
113112

@@ -166,7 +165,7 @@ impl CoreClient {
166165
}
167166

168167
error!(target: "editoast::coreclient", "{method_s} {path} {status}", status = status.to_string().bold().red());
169-
Err(self.handle_error(bytes.as_ref(), status, url))
168+
Err(self.handle_error(bytes.as_ref(), url))
170169
}
171170
CoreClient::MessageQueue(client) => {
172171
// TODO: maybe implement retry?
@@ -175,19 +174,26 @@ impl CoreClient {
175174
// TODO: tracing: use correlation id
176175

177176
let response = client
178-
.call_with_response::<_, R>(infra_id.to_string(), path, &body, true, None, None)
177+
.call_with_response(infra_id.to_string(), path, &body, true, None, None)
179178
.await?;
180179

181-
Ok(response)
180+
if response.status == b"ok" {
181+
return Ok(R::from_bytes(&response.payload)?);
182+
}
183+
184+
if response.status == b"core_error" {
185+
return Err(self.handle_error(&response.payload, path.to_string()));
186+
}
187+
188+
todo!("TODO: handle protocol errors")
182189
}
183190
#[cfg(test)]
184191
CoreClient::Mocked(client) => {
185192
match client.fetch_mocked::<_, B, R>(method, path, body) {
186193
Ok(Some(response)) => Ok(response),
187194
Ok(None) => Err(CoreError::NoResponseContent.into()),
188195
Err(MockingError { bytes, status, url }) => Err(self.handle_error(
189-
&bytes,
190-
reqwest::StatusCode::from_u16(status.as_u16()).unwrap(),
196+
&bytes, //reqwest::StatusCode::from_u16(status.as_u16()).unwrap(),
191197
url,
192198
)),
193199
}
@@ -372,6 +378,13 @@ pub struct StandardCoreError {
372378
error_type: String,
373379
context: HashMap<String, Value>,
374380
message: String,
381+
cause: CoreErrorCause,
382+
}
383+
384+
#[derive(Debug, Deserialize)]
385+
pub enum CoreErrorCause {
386+
Internal,
387+
User,
375388
}
376389

377390
impl crate::error::EditoastError for StandardCoreError {

editoast/src/core/mq_client.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use serde_json::to_vec;
1313
use std::{fmt::Debug, sync::Arc};
1414
use thiserror::Error;
1515
use tokio::time::{timeout, Duration};
16+
use utoipa::openapi::response;
1617

1718
#[derive(Debug, Clone)]
1819
pub struct RabbitMQClient {
@@ -44,11 +45,19 @@ pub enum Error {
4445
#[error("Cannot deserialize response: {0}")]
4546
#[editoast_error(status = "500")]
4647
DeserialisationError(InternalError),
48+
#[error("Cannot parse response status")]
49+
#[editoast_error(status = "500")]
50+
StatusParsingError,
4751
#[error("Response timeout")]
4852
#[editoast_error(status = "500")]
4953
ResponseTimeout,
5054
}
5155

56+
pub struct MQResponse {
57+
pub payload: Vec<u8>,
58+
pub status: Vec<u8>,
59+
}
60+
5261
impl RabbitMQClient {
5362
pub async fn new(options: Options) -> Result<Self, Error> {
5463
let connection = Connection::connect(&options.uri, ConnectionProperties::default())
@@ -120,18 +129,17 @@ impl RabbitMQClient {
120129
Ok(())
121130
}
122131

123-
pub async fn call_with_response<T, TR>(
132+
pub async fn call_with_response<T>(
124133
&self,
125134
routing_key: String,
126135
path: &str,
127136
published_payload: &Option<T>,
128137
mandatory: bool,
129138
correlation_id: Option<String>,
130139
override_timeout: Option<u64>,
131-
) -> Result<TR::Response, Error>
140+
) -> Result<MQResponse, Error>
132141
where
133142
T: Serialize,
134-
TR: CoreResponse,
135143
{
136144
// Create a channel
137145
let channel = self
@@ -204,10 +212,22 @@ impl RabbitMQClient {
204212
.map_err(Error::Lapin)?;
205213

206214
// Deserialize the response
207-
let response =
208-
TR::from_bytes(&delivery.data).map_err(|e| Error::DeserialisationError(e))?;
209-
210-
Ok(response)
215+
// let payload =
216+
// TR::from_bytes(&delivery.data).map_err(|e| Error::DeserialisationError(e))?;
217+
218+
let status = delivery
219+
.properties
220+
.headers()
221+
.as_ref()
222+
.and_then(|f| f.inner().get("x-status"))
223+
.and_then(|s| s.as_byte_array())
224+
.map(|s| Ok(s.as_slice().to_owned()))
225+
.unwrap_or(Err(Error::StatusParsingError))?;
226+
227+
Ok(MQResponse {
228+
payload: delivery.data,
229+
status,
230+
})
211231
} else {
212232
Err(Error::ResponseTimeout)
213233
}

0 commit comments

Comments
 (0)