Skip to content
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.

Sign up for GitHub

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

[Rust][Client][Reqwest] Better http error handling #6481

Merged
merged 10 commits into from
Jun 13, 2020
Merged

[Rust][Client][Reqwest] Better http error handling #6481

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9e3565e
Stronger typing for http errors with Rust client. (#5609).
bcourtine May 29, 2020
948d653
Merge branch 'master' into 5609-rust-client-better-error-typing (supp…
bcourtine May 29, 2020
ed3e8fb
Error structure can be parametrized (but is still hardcoded with `ser…
bcourtine May 29, 2020
3ad86b3
Each API method has is own enum of functionnal errors.
bcourtine May 29, 2020
755fc69
Fix the missing "Debug" derivation for API error enums.
bcourtine May 29, 2020
ff3d510
Generate models for error deserialization.
bcourtine May 30, 2020
250d0ed
Handle several 2xx success models.
bcourtine Jun 9, 2020
a890621
Expose new API objects, required to use the API (params, success, err…
bcourtine Jun 9, 2020
733fd9d
Merge remote-tracking branch 'upstream/master' into 5609-rust-client-…
bcourtine Jun 9, 2020
a72be48
Merge branch 'master' of https://github.com/openapitools/openapi-gene…
wing328 Jun 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 15 additions & 12 deletions modules/openapi-generator/src/main/resources/rust/reqwest/api.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,18 +263,21 @@ impl {{{classname}}} for {{{classname}}}Client {
{{/hasBodyParam}}

let req = req_builder.build()?;
{{^returnType}}
client.execute(req){{#supportAsync}}.await{{/supportAsync}}?.error_for_status()?;
Ok(())
{{/returnType}}
{{#returnType}}
{{#supportAsync}}
Ok(client.execute(req).await?.error_for_status()?.json::<{{{.}}}>().await?)
{{/supportAsync}}
{{^supportAsync}}
Ok(client.execute(req)?.error_for_status()?.json()?)
{{/supportAsync}}
{{/returnType}}
let mut resp = client.execute(req){{#supportAsync}}.await{{/supportAsync}}?;
if resp.status().is_success() {
{{^returnType}}
Ok(())
{{/returnType}}
{{#returnType}}
Ok(resp.json{{#supportAsync}}::<{{{.}}}>().await{{/supportAsync}}{{^supportAsync}}(){{/supportAsync}}?)
{{/returnType}}
} else {
let status = resp.status();
let content = resp.text(){{#supportAsync}}.await{{/supportAsync}}?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

{{/operation}}
Expand Down
8 changes: 8 additions & 0 deletions modules/openapi-generator/src/main/resources/rust/reqwest/api_mod.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
use reqwest;
use serde_json;

#[derive(Debug, Clone)]
pub struct ResponseErrorContent {
pub status: reqwest::StatusCode,
pub content: String,
pub entity: Option<serde_json::Value>,
}

#[derive(Debug)]
pub enum Error {
Reqwest(reqwest::Error),
Serde(serde_json::Error),
Io(std::io::Error),
ResponseError(ResponseErrorContent),
}

impl From<reqwest::Error> for Error {
Expand Down
11 changes: 10 additions & 1 deletion samples/client/petstore/rust/reqwest/fileResponseTest/src/apis/default_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,16 @@ impl DefaultApi for DefaultApiClient {
}

let req = req_builder.build()?;
Ok(client.execute(req)?.error_for_status()?.json()?)
let mut resp = client.execute(req)?;
if resp.status().is_success() {
Ok(resp.json()?)
} else {
let status = resp.status();
let content = resp.text()?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
Copy link

@seunlanlege seunlanlege May 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey 🙋‍♂️ great attempt on this issue, but this isn't complete yet.

The openapi spec allows for defining a schema for every possible response status: eg

"responses": {
    "200": {},
    "202": {},
    "400": {},
    "default": {}
}

so instead of just checking for a success status, you'll need to match over the response status.

and deserialize based on the defined schema for that status as well as the fallback(default) schema.

I imagine this means that the return type of methods would become an enum,

pub enum OperationIdResponse {
    Status200(Status200Schema),
    Status202(Status202Schema),
    Status400(Status400Schema),
    Unspecified(DefaultSchema)
}

// .. snip
match response.status().as_u16() {
    200 => Ok(OperationIdResponse::Status200(response.json().await?))
    202 => Ok(OperationIdResponse::Status202(response.json().await?))
    400 => Ok(OperationIdResponse::Status400(response.json().await?))
    _ => Ok(OperationIdResponse::Unspecified(response.json().await?))
}
// .. snip

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @seunlanlege,

Thanks for your comment. I know this is just a first attempt which is incomplete.

I had the same idea of an enum as response, but I don't like it. I think we should not return Ok when the return status is not ok…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am working on something like:

#[derive(Debug, Clone)]
pub struct ResponseErrorContent<T> {
    pub status: reqwest::StatusCode,
    pub content: String,
    pub entity: Option<T>,
}

#[derive(Debug)]
pub enum Error<T> {
    Reqwest(reqwest::Error),
    Serde(serde_json::Error),
    Io(std::io::Error),
    ResponseError(ResponseErrorContent<T>),
}

where T could be an enum of error types corresponding to error codes.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also bear in mind that there could be more than one success response in the 2xx range, So success types would potentially be an enum

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With an enum, multiple success codes can be handled, but this case is not frequent. So I don't know if it is worth complicating the generated code for this.

Copy link

@seunlanlege seunlanlege May 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiple success codes can be handled, but this case is not frequent.

unfortunately it's actually very common 😅

jira/sendgrid's openapi specifications have multiple 2xx success schemas

}

}
8 changes: 8 additions & 0 deletions samples/client/petstore/rust/reqwest/fileResponseTest/src/apis/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
use reqwest;
use serde_json;

#[derive(Debug, Clone)]
pub struct ResponseErrorContent {
pub status: reqwest::StatusCode,
pub content: String,
pub entity: Option<serde_json::Value>,
}

#[derive(Debug)]
pub enum Error {
Reqwest(reqwest::Error),
Serde(serde_json::Error),
Io(std::io::Error),
ResponseError(ResponseErrorContent),
}

impl From<reqwest::Error> for Error {
Expand Down
8 changes: 8 additions & 0 deletions samples/client/petstore/rust/reqwest/petstore-async/src/apis/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
use reqwest;
use serde_json;

#[derive(Debug, Clone)]
pub struct ResponseErrorContent {
pub status: reqwest::StatusCode,
pub content: String,
pub entity: Option<serde_json::Value>,
}

#[derive(Debug)]
pub enum Error {
Reqwest(reqwest::Error),
Serde(serde_json::Error),
Io(std::io::Error),
ResponseError(ResponseErrorContent),
}

impl From<reqwest::Error> for Error {
Expand Down
92 changes: 80 additions & 12 deletions samples/client/petstore/rust/reqwest/petstore-async/src/apis/pet_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,16 @@ use super::{Error, configuration};
req_builder = req_builder.json(&body);

let req = req_builder.build()?;
client.execute(req).await?.error_for_status()?;
Ok(())
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(())
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn delete_pet(configuration: &configuration::Configuration, pet_id: i64, api_key: Option<&str>) -> Result<(), Error> {
Expand All @@ -54,8 +62,16 @@ use super::{Error, configuration};
};

let req = req_builder.build()?;
client.execute(req).await?.error_for_status()?;
Ok(())
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(())
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn find_pets_by_status(configuration: &configuration::Configuration, status: Vec<String>) -> Result<Vec<crate::models::Pet>, Error> {
Expand All @@ -73,7 +89,16 @@ use super::{Error, configuration};
};

let req = req_builder.build()?;
Ok(client.execute(req).await?.error_for_status()?.json::<Vec<crate::models::Pet>>().await?)
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(resp.json::<Vec<crate::models::Pet>>().await?)
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn find_pets_by_tags(configuration: &configuration::Configuration, tags: Vec<String>) -> Result<Vec<crate::models::Pet>, Error> {
Expand All @@ -91,7 +116,16 @@ use super::{Error, configuration};
};

let req = req_builder.build()?;
Ok(client.execute(req).await?.error_for_status()?.json::<Vec<crate::models::Pet>>().await?)
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(resp.json::<Vec<crate::models::Pet>>().await?)
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn get_pet_by_id(configuration: &configuration::Configuration, pet_id: i64) -> Result<crate::models::Pet, Error> {
Expand All @@ -113,7 +147,16 @@ use super::{Error, configuration};
};

let req = req_builder.build()?;
Ok(client.execute(req).await?.error_for_status()?.json::<crate::models::Pet>().await?)
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(resp.json::<crate::models::Pet>().await?)
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn update_pet(configuration: &configuration::Configuration, body: crate::models::Pet) -> Result<(), Error> {
Expand All @@ -131,8 +174,16 @@ use super::{Error, configuration};
req_builder = req_builder.json(&body);

let req = req_builder.build()?;
client.execute(req).await?.error_for_status()?;
Ok(())
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(())
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn update_pet_with_form(configuration: &configuration::Configuration, pet_id: i64, name: Option<&str>, status: Option<&str>) -> Result<(), Error> {
Expand All @@ -157,8 +208,16 @@ use super::{Error, configuration};
req_builder = req_builder.form(&form_params);

let req = req_builder.build()?;
client.execute(req).await?.error_for_status()?;
Ok(())
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(())
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn upload_file(configuration: &configuration::Configuration, pet_id: i64, additional_metadata: Option<&str>, file: Option<std::path::PathBuf>) -> Result<crate::models::ApiResponse, Error> {
Expand All @@ -181,6 +240,15 @@ use super::{Error, configuration};
req_builder = req_builder.multipart(form);

let req = req_builder.build()?;
Ok(client.execute(req).await?.error_for_status()?.json::<crate::models::ApiResponse>().await?)
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(resp.json::<crate::models::ApiResponse>().await?)
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

45 changes: 40 additions & 5 deletions samples/client/petstore/rust/reqwest/petstore-async/src/apis/store_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,16 @@ use super::{Error, configuration};
}

let req = req_builder.build()?;
client.execute(req).await?.error_for_status()?;
Ok(())
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(())
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn get_inventory(configuration: &configuration::Configuration, ) -> Result<::std::collections::HashMap<String, i32>, Error> {
Expand All @@ -52,7 +60,16 @@ use super::{Error, configuration};
};

let req = req_builder.build()?;
Ok(client.execute(req).await?.error_for_status()?.json::<::std::collections::HashMap<String, i32>>().await?)
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(resp.json::<::std::collections::HashMap<String, i32>>().await?)
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn get_order_by_id(configuration: &configuration::Configuration, order_id: i64) -> Result<crate::models::Order, Error> {
Expand All @@ -66,7 +83,16 @@ use super::{Error, configuration};
}

let req = req_builder.build()?;
Ok(client.execute(req).await?.error_for_status()?.json::<crate::models::Order>().await?)
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(resp.json::<crate::models::Order>().await?)
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

pub async fn place_order(configuration: &configuration::Configuration, body: crate::models::Order) -> Result<crate::models::Order, Error> {
Expand All @@ -81,6 +107,15 @@ use super::{Error, configuration};
req_builder = req_builder.json(&body);

let req = req_builder.build()?;
Ok(client.execute(req).await?.error_for_status()?.json::<crate::models::Order>().await?)
let mut resp = client.execute(req).await?;
if resp.status().is_success() {
Ok(resp.json::<crate::models::Order>().await?)
} else {
let status = resp.status();
let content = resp.text().await?;
let entity: Option<serde_json::Value> = serde_json::from_str(&content).ok();
let error = crate::apis::ResponseErrorContent { status, content, entity };
Err(Error::ResponseError(error))
}
}

Loading