Add POST-requests handler to multichain-api-gateway #54
Conversation
| @@ -5,6 +5,7 @@ edition = "2021" | |||
|
|
|||
| [dependencies] | |||
| actix-web = "4" | |||
| awc = { version = "3.0.0", features = ["openssl"] } | |||
There was a problem hiding this comment.
Somehow default awc can't work with ssl
actix/actix-web#1045
multichain-api-gateway/src/lib.rs
Outdated
| App, HttpRequest, HttpServer, Responder, | ||
| }; | ||
| use awc::Client; |
There was a problem hiding this comment.
I decided to try replace "reqwest" client with actix-web client (awc) since we use actix-web server. It takes almost the same lines of code.
Let me know what you think; it's easy to rollback to "reqwest" if needed.
multichain-api-gateway/src/lib.rs
Outdated
| headers: &HeaderMap, | ||
| ) -> Vec<(Instance, String)> { | ||
| let client = Client::builder() | ||
| .timeout(std::time::Duration::from_secs(60)) |
There was a problem hiding this comment.
Default value for a timeout is 1 second.
This request takes 48 second on my pc
There was a problem hiding this comment.
I believe the timeout may greatly depend on the request. Is it possible to look at current Elixir implementation of those requests and check what the timeout is in their case and if there is any at all?
Just as an idea, it may be great to have that parameter configurable, however, I am still interested in current instances timeouts
There was a problem hiding this comment.
- Made it configurable from config
- Clarified what is the default timeout: it's 60 seconds for all methods
multichain-api-gateway/src/lib.rs
Outdated
| let str_response = match response { | ||
| Ok(mut response) => match response.body().await { | ||
| Ok(bytes) => match str::from_utf8(bytes.as_ref()) { | ||
| Ok(str) => str.to_string(), | ||
| Err(utf8_error) => utf8_error.to_string(), | ||
| }, | ||
| Err(payload_error) => payload_error.to_string(), | ||
| }, | ||
| Err(send_request_error) => send_request_error.to_string(), | ||
| }; |
There was a problem hiding this comment.
I think the whole block is looking kind of messy, but I don't know what to do with it, since:
response.body().awaitis needed to be called outside of anymap- chain computation via
and_thencan't be used because we have different result and error types on every step
There was a problem hiding this comment.
I suggest to create a new private method
async fn make_request(request: ClientRequest, body: Bytes) -> Result<String, Box<dyn Error>>
inside an ApiRequest and use ? to convert between errors there
multichain-api-gateway/src/lib.rs
Outdated
| Ok(str) => str.to_string(), | ||
| Err(utf8_error) => utf8_error.to_string(), |
There was a problem hiding this comment.
We can use unwrap_or_else here, but I'm not sure if it will add more readability
There was a problem hiding this comment.
Personally, in my opinion, it would be more readable. You would just have to map success result into String before unwrapping
| serde_json::from_str(&value) | ||
| .unwrap_or_else(|e| Value::String(format!("{}: {}\n", e, value))), |
There was a problem hiding this comment.
Before that, when the server returned an error, the error was replaced withexpected value at column 1 at row 1 (json parsing failed). Now we preserve both errors
multichain-api-gateway/src/lib.rs
Outdated
| headers: &HeaderMap, | ||
| ) -> Vec<(Instance, String)> { | ||
| let client = Client::builder() | ||
| .timeout(std::time::Duration::from_secs(60)) |
There was a problem hiding this comment.
I believe the timeout may greatly depend on the request. Is it possible to look at current Elixir implementation of those requests and check what the timeout is in their case and if there is any at all?
Just as an idea, it may be great to have that parameter configurable, however, I am still interested in current instances timeouts
multichain-api-gateway/src/lib.rs
Outdated
| Ok(str) => str.to_string(), | ||
| Err(utf8_error) => utf8_error.to_string(), |
There was a problem hiding this comment.
Personally, in my opinion, it would be more readable. You would just have to map success result into String before unwrapping
multichain-api-gateway/src/lib.rs
Outdated
| let str_response = match response { | ||
| Ok(mut response) => match response.body().await { | ||
| Ok(bytes) => match str::from_utf8(bytes.as_ref()) { | ||
| Ok(str) => str.to_string(), | ||
| Err(utf8_error) => utf8_error.to_string(), | ||
| }, | ||
| Err(payload_error) => payload_error.to_string(), | ||
| }, | ||
| Err(send_request_error) => send_request_error.to_string(), | ||
| }; |
There was a problem hiding this comment.
I suggest to create a new private method
async fn make_request(request: ClientRequest, body: Bytes) -> Result<String, Box<dyn Error>>
inside an ApiRequest and use ? to convert between errors there
This lets us make POST requests to our API (e.g. contract verify request).
It's done in the way that the incoming request body is passed through to the following calls.
Added 1 integration test that makes a call as described in an example here.