Skip to content

Commit

Permalink
Fix notify dto (#2227)
Browse files Browse the repository at this point in the history
# Description
Notification calls from the driver to solver engines are currently
failing due to:

> Failed to deserialize the JSON body into the target type: invalid
type: map, expected variant identifier at line 1 column 143

I broke this with #2192 where I missed adding the flatten directive on
the driver dto.

# Changes
- [x] Fix dto on both sides (we can no longer deny_unknow_fields due to
this [serde bug](serde-rs/serde#1600))
- [x] Warn if the notify call is not successful

## How to test
Run e.g. the partial fill colocation test case. See warning before
adjusting the dto and now no longer see it.
  • Loading branch information
fleupold authored Jan 2, 2024
1 parent 1156d32 commit c90b189
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 4 deletions.
1 change: 1 addition & 0 deletions crates/driver/src/infra/solver/dto/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ impl Notification {
pub struct Notification {
auction_id: Option<String>,
solution_id: Option<u64>,
#[serde(flatten)]
kind: Kind,
}

Expand Down
4 changes: 3 additions & 1 deletion crates/driver/src/infra/solver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,9 @@ impl Solver {
req = req.header("X-REQUEST-ID", id);
}
let future = async move {
let _ = util::http::send(SOLVER_RESPONSE_MAX_BYTES, req).await;
if let Err(error) = util::http::send(SOLVER_RESPONSE_MAX_BYTES, req).await {
tracing::warn!(?error, "failed to notify solver");
}
};
tokio::task::spawn(future.in_current_span());
}
Expand Down
12 changes: 11 additions & 1 deletion crates/driver/src/util/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@ pub async fn send(limit_bytes: usize, req: reqwest::RequestBuilder) -> Result<St
}
data.extend_from_slice(&chunk);
}
String::from_utf8(data).map_err(Into::into)
let body = String::from_utf8(data).map_err(Error::NotUtf8)?;
if res.status().is_success() {
Ok(body)
} else {
Err(Error::NotOk {
code: res.status().as_u16(),
body: body.clone(),
})
}
}

#[derive(Debug, Error)]
Expand All @@ -20,4 +28,6 @@ pub enum Error {
ResponseTooLarge { limit_bytes: usize },
#[error("the response could not be parsed as UTF-8: {0:?}")]
NotUtf8(#[from] std::string::FromUtf8Error),
#[error("the response status was not 2xx but {code:?}, body: {body:?}")]
NotOk { code: u16, body: String },
}
2 changes: 1 addition & 1 deletion crates/solvers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ thiserror = "1"
tokio = { version = "1", features = ["macros", "rt-multi-thread", "signal", "time"] }
toml = "0.7"
tower = "0.4"
tower-http = { version = "0.4", features = ["trace"] }
tower-http = { version = "0.4", features = ["limit", "trace"] }
web3 = "0.19"

# TODO Once solvers are ported and E2E tests set up, slowly migrate code and
Expand Down
2 changes: 1 addition & 1 deletion crates/solvers/src/api/routes/notify/dto/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl Notification {

#[serde_as]
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
#[serde(rename_all = "camelCase")]
pub struct Notification {
#[serde_as(as = "Option<DisplayFromStr>")]
auction_id: Option<i64>,
Expand Down

0 comments on commit c90b189

Please sign in to comment.