-
Notifications
You must be signed in to change notification settings - Fork 153
Migrate orderbook API from warp to axum #4080
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b505933
9613dfe
1380e8d
801bb90
9504557
a4fe3a6
8f617f3
ad90a95
ed165a2
8f2cccb
9d71def
43423a3
1f2486c
1b86466
8ebb11c
1e5857e
9af007d
b03fb86
a7a6280
3191b88
60a3e7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,13 @@ | ||
| use axum::{http::StatusCode, response::IntoResponse, routing::get}; | ||
| use axum::{ | ||
| http::StatusCode, | ||
| response::{IntoResponse, Response}, | ||
| routing::get, | ||
| }; | ||
|
|
||
| pub(in crate::infra::api) fn healthz(app: axum::Router<()>) -> axum::Router<()> { | ||
| app.route("/healthz", get(route)) | ||
| } | ||
|
|
||
| async fn route() -> impl IntoResponse { | ||
| StatusCode::OK | ||
| async fn route() -> Response { | ||
| StatusCode::OK.into_response() | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -53,8 +53,8 @@ async fn http_validation(web3: Web3) { | |||||
|
|
||||||
| assert_eq!( | ||||||
| response.status(), | ||||||
| StatusCode::NOT_FOUND, | ||||||
| "Expected 404 for invalid OrderUid ({description}): {uid}" | ||||||
| StatusCode::BAD_REQUEST, | ||||||
| "Expected 400 for invalid OrderUid ({description}): {uid}" | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -76,8 +76,8 @@ async fn http_validation(web3: Web3) { | |||||
|
|
||||||
| assert_eq!( | ||||||
| response.status(), | ||||||
| StatusCode::NOT_FOUND, | ||||||
| "Expected 404 for invalid Address ({description}): {addr}" | ||||||
| StatusCode::BAD_REQUEST, | ||||||
| "Expected 400 for invalid Address ({description}): {addr}" | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -90,8 +90,8 @@ async fn http_validation(web3: Web3) { | |||||
|
|
||||||
| assert_eq!( | ||||||
| response.status(), | ||||||
| StatusCode::NOT_FOUND, | ||||||
| "Expected 404 for invalid token Address ({description}): {addr}" | ||||||
| StatusCode::BAD_REQUEST, | ||||||
| "Expected 400 for invalid token Address ({description}): {addr}" | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -113,19 +113,24 @@ async fn http_validation(web3: Web3) { | |||||
|
|
||||||
| assert_eq!( | ||||||
| response.status(), | ||||||
| StatusCode::NOT_FOUND, | ||||||
| "Expected 404 for invalid tx hash ({description}): {hash}" | ||||||
| StatusCode::BAD_REQUEST, | ||||||
| "Expected 400 for invalid tx hash ({description}): {hash}" | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| // Test malformed auction IDs | ||||||
| let invalid_auction_ids: Vec<(&str, &str)> = vec![ | ||||||
| ("not-a-number", "non-numeric"), | ||||||
| ("-1", "negative number"), | ||||||
| ("99999999999999999999999", "u64 overflow"), | ||||||
| ]; | ||||||
|
|
||||||
| for (id, description) in invalid_auction_ids { | ||||||
| // Note: "-1" returns 404 because it doesn't match the u64 route pattern at all, | ||||||
| // while non-numeric strings return 400 as they match the path but fail | ||||||
| // deserialization | ||||||
| for (id, description, expected_status) in [ | ||||||
| ("not-a-number", "non-numeric", StatusCode::BAD_REQUEST), | ||||||
| ("-1", "negative number", StatusCode::NOT_FOUND), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure about it.
Suggested change
|
||||||
| ( | ||||||
| "99999999999999999999999", | ||||||
| "u64 overflow", | ||||||
| StatusCode::BAD_REQUEST, | ||||||
| ), | ||||||
| ] { | ||||||
| let response = client | ||||||
| .get(format!("{API_HOST}/api/v1/solver_competition/{id}")) | ||||||
| .send() | ||||||
|
|
@@ -134,8 +139,8 @@ async fn http_validation(web3: Web3) { | |||||
|
|
||||||
| assert_eq!( | ||||||
| response.status(), | ||||||
| StatusCode::NOT_FOUND, | ||||||
| "Expected 404 for invalid AuctionId ({description}): {id}" | ||||||
| expected_status, | ||||||
| "Expected {expected_status} for invalid AuctionId ({description}): {id}" | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -203,6 +208,7 @@ async fn http_validation(web3: Web3) { | |||||
| ); | ||||||
|
|
||||||
| // Missing required fields (empty object) | ||||||
| // Axum returns 422 (Unprocessable Entity) for JSON deserialization errors | ||||||
| let response = client | ||||||
| .post(format!("{API_HOST}/api/v1/orders")) | ||||||
| .header("Content-Type", "application/json") | ||||||
|
|
@@ -213,8 +219,8 @@ async fn http_validation(web3: Web3) { | |||||
|
|
||||||
| assert_eq!( | ||||||
| response.status(), | ||||||
| StatusCode::BAD_REQUEST, | ||||||
| "Missing required fields should return 400" | ||||||
| StatusCode::UNPROCESSABLE_ENTITY, | ||||||
| "Missing required fields should return 422" | ||||||
| ); | ||||||
|
|
||||||
| // Wrong field types | ||||||
|
|
@@ -236,8 +242,8 @@ async fn http_validation(web3: Web3) { | |||||
|
|
||||||
| assert_eq!( | ||||||
| response.status(), | ||||||
| StatusCode::BAD_REQUEST, | ||||||
| "Wrong field types should return 400" | ||||||
| StatusCode::UNPROCESSABLE_ENTITY, | ||||||
| "Wrong field types should return 422" | ||||||
| ); | ||||||
|
|
||||||
| // Invalid enum value | ||||||
|
|
@@ -256,8 +262,8 @@ async fn http_validation(web3: Web3) { | |||||
|
|
||||||
| assert_eq!( | ||||||
| response.status(), | ||||||
| StatusCode::BAD_REQUEST, | ||||||
| "Invalid enum value should return 400" | ||||||
| StatusCode::UNPROCESSABLE_ENTITY, | ||||||
| "Invalid enum value should return 422" | ||||||
| ); | ||||||
|
|
||||||
| // Test error response formats | ||||||
|
|
@@ -270,7 +276,7 @@ async fn http_validation(web3: Web3) { | |||||
| .await | ||||||
| .unwrap(); | ||||||
|
|
||||||
| assert_eq!(response.status(), StatusCode::BAD_REQUEST); | ||||||
| assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY); | ||||||
|
|
||||||
| let body_text = response.text().await.unwrap(); | ||||||
| assert!( | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| pub mod request_id; | ||
| pub mod trace_id_format; | ||
| pub mod tracing_axum; | ||
| pub mod tracing_warp; |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking
axumusing correct HTTP error codes makes this migration backwards incompatible. I'm not saying we should keep the wrong ones, but we might wanna ping the FE folks. 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pinged them, though I'm not expecting these error codes to be especially problematic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ok
https://cowservices.slack.com/archives/C036G0J90BU/p1769427475645459
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:chefskiss:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a public API. Not only FE uses it.