Skip to content

Commit 4edf106

Browse files
flomonstermultun
authored andcommitted
editoast: retry core request when connection closed
This fix is linked to this hyper issue hyperium/hyper#2136 It can't be reproduced locally and is a frequent occurrence in the CI. Note: It might be a better fix than this one.
1 parent 834d287 commit 4edf106

File tree

1 file changed

+59
-30
lines changed

1 file changed

+59
-30
lines changed

editoast/src/core/mod.rs

+59-30
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@ use crate::error::Result;
1414
use async_trait::async_trait;
1515
use editoast_derive::EditoastError;
1616
pub use http_client::{HttpClient, HttpClientBuilder};
17+
use log::info;
1718
use reqwest::Url;
1819
use serde::{de::DeserializeOwned, Serialize};
1920
use serde_derive::Deserialize;
2021
use thiserror::Error;
2122

23+
const MAX_RETRIES: u8 = 5;
24+
2225
#[derive(Debug, Clone)]
2326
pub enum CoreClient {
2427
Direct(HttpClient),
@@ -50,11 +53,27 @@ impl CoreClient {
5053
) -> Result<R::Response> {
5154
match self {
5255
CoreClient::Direct(client) => {
53-
let mut request = client.request(method, path);
54-
if let Some(body) = body {
55-
request = request.json(body);
56-
}
57-
let response = request.send().await.map_err(Into::<CoreError>::into)?;
56+
let mut i_try = 0;
57+
let response = loop {
58+
let mut request = client.request(method.clone(), path);
59+
if let Some(body) = body {
60+
request = request.json(body);
61+
}
62+
match request.send().await.map_err(Into::<CoreError>::into) {
63+
// This error occurs quite often in the CI.
64+
// It's linked to this issue https://github.com/hyperium/hyper/issues/2136.
65+
// This is why we retry the request here
66+
Err(CoreError::ConnectionClosedBeforeMessageCompleted)
67+
if i_try < MAX_RETRIES =>
68+
{
69+
i_try += 1;
70+
info!("Core request '{}: {}': Connection closed before message completed. Retry [{}/{}]", method, path, i_try, MAX_RETRIES);
71+
continue;
72+
}
73+
response => break response?,
74+
}
75+
};
76+
5877
let url = response.url().to_string();
5978
let status = response.status();
6079
let bytes =
@@ -65,33 +84,31 @@ impl CoreClient {
6584
msg: err.to_string(),
6685
})?;
6786
if status.is_success() {
68-
R::from_bytes(bytes.as_ref())
69-
} else {
70-
// We try to deserialize the response as the standard Core error format
71-
// If that fails we try to return a generic error containing the raw error
72-
let core_error =
73-
<Json<CoreErrorPayload> as CoreResponse>::from_bytes(bytes.as_ref())
74-
.map_err(|err| {
75-
if let Ok(utf8_raw_error) =
76-
String::from_utf8(bytes.as_ref().to_vec())
77-
{
78-
CoreError::GenericCoreError {
79-
status: status.as_str().to_owned(),
80-
url: url.clone(),
81-
raw_error: utf8_raw_error,
82-
}
83-
.into()
84-
} else {
85-
err
86-
}
87-
})?;
88-
Err(CoreError::Forward {
89-
status: status.as_u16(),
90-
core_error,
91-
url,
87+
return R::from_bytes(bytes.as_ref());
88+
}
89+
// We try to deserialize the response as the standard Core error format
90+
// If that fails we try to return a generic error containing the raw error
91+
let core_error = <Json<CoreErrorPayload> as CoreResponse>::from_bytes(
92+
bytes.as_ref(),
93+
)
94+
.map_err(|err| {
95+
if let Ok(utf8_raw_error) = String::from_utf8(bytes.as_ref().to_vec()) {
96+
CoreError::GenericCoreError {
97+
status: status.as_str().to_owned(),
98+
url: url.clone(),
99+
raw_error: utf8_raw_error,
100+
}
101+
.into()
102+
} else {
103+
err
92104
}
93-
.into())
105+
})?;
106+
Err(CoreError::Forward {
107+
status: status.as_u16(),
108+
core_error,
109+
url,
94110
}
111+
.into())
95112
}
96113
#[cfg(test)]
97114
CoreClient::Mocked(client) => client
@@ -264,13 +281,25 @@ enum CoreError {
264281
url: String,
265282
raw_error: String,
266283
},
284+
#[error("Core connection closed before message completed. Should retry.")]
285+
#[editoast_error(status = 500)]
286+
ConnectionClosedBeforeMessageCompleted,
267287
#[cfg(test)]
268288
#[error("The mocked response had no body configured - check out StubResponseBuilder::body if this is unexpected")]
269289
NoResponseContent,
270290
}
271291

272292
impl From<reqwest::Error> for CoreError {
273293
fn from(value: reqwest::Error) -> Self {
294+
// Since we should retry the request it's useful to have its own kind of error.
295+
if value
296+
.to_string()
297+
.contains("connection closed before message completed")
298+
{
299+
return Self::ConnectionClosedBeforeMessageCompleted;
300+
}
301+
302+
// Convert the reqwest error
274303
Self::GenericCoreError {
275304
status: value
276305
.status()

0 commit comments

Comments
 (0)