Skip to content

Better error handling, pattern matching instead of conditional flow #137

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

Closed
wants to merge 12 commits into from
Closed

Better error handling, pattern matching instead of conditional flow #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5c9b6bd
better errors
0xDmtri Mar 5, 2024
14728a2
change reqwest::StatusCode to http::StatusCode
0xDmtri Mar 10, 2024
986a013
handle all 2xx responses as success
0xDmtri Mar 10, 2024
58742bc
remove dbg from tests
0xDmtri Mar 10, 2024
8fbcf6c
better errors
0xDmtri Mar 5, 2024
d583ef3
change reqwest::StatusCode to http::StatusCode
0xDmtri Mar 10, 2024
f2391dd
handle all 2xx responses as success
0xDmtri Mar 10, 2024
1603bfa
remove dbg from tests
0xDmtri Mar 10, 2024
e7fd233
better errors
0xDmtri Mar 10, 2024
456499b
bump
0xDmtri Mar 10, 2024
3544ded
bump
0xDmtri Mar 10, 2024
49b2922
bump
0xDmtri Mar 10, 2024
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
12 changes: 4 additions & 8 deletions influxdb/src/client/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
//! ```

use futures_util::TryFutureExt;
use http::StatusCode;
#[cfg(feature = "reqwest")]
use reqwest::{Client as HttpClient, RequestBuilder, Response as HttpResponse};
use std::collections::{BTreeMap, HashMap};
Expand Down Expand Up @@ -281,7 +280,7 @@ impl Client {
})?;

// todo: improve error parsing without serde
if s.contains("\"error\"") {
if s.contains("\"error\"") || s.contains("\"Error\"") {
return Err(Error::DatabaseError {
error: format!("influxdb error: \"{}\"", s),
});
Expand All @@ -301,12 +300,9 @@ impl Client {

pub(crate) fn check_status(res: &HttpResponse) -> Result<(), Error> {
let status = res.status();
if status == StatusCode::UNAUTHORIZED.as_u16() {
Err(Error::AuthorizationError)
} else if status == StatusCode::FORBIDDEN.as_u16() {
Err(Error::AuthenticationError)
} else {
Ok(())
match status.is_success() {
true => Ok(()),
false => Err(Error::ApiError(status)),
}
}

Expand Down
15 changes: 7 additions & 8 deletions influxdb/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
//! Errors that might happen in the crate

#[cfg(feature = "reqwest")]
use reqwest::StatusCode;
#[cfg(feature = "surf")]
use surf::StatusCode;
use thiserror::Error;

#[derive(Debug, Eq, PartialEq, Error)]
Expand All @@ -25,15 +29,10 @@ pub enum Error {
/// Error which has happened inside InfluxDB
DatabaseError { error: String },

#[error("authentication error. No or incorrect credentials")]
/// Error happens when no or incorrect credentials are used. `HTTP 401 Unauthorized`
AuthenticationError,

#[error("authorization error. User not authorized")]
/// Error happens when the supplied user is not authorized. `HTTP 403 Forbidden`
AuthorizationError,

#[error("connection error: {error}")]
/// Error happens when HTTP request fails
ConnectionError { error: String },

#[error("server responded with an error code: {0}")]
ApiError(StatusCode),
}
84 changes: 54 additions & 30 deletions influxdb/tests/integration_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,22 +140,30 @@ async fn test_wrong_authed_write_and_read() {
let write_result = client.query(write_query).await;
assert_result_err(&write_result);
match write_result {
Err(Error::AuthorizationError) => {}
_ => panic!(
"Should be an AuthorizationError: {}",
write_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 401_u16 {
panic!(
"Should be an ApiError(401), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(401): {}", write_result.unwrap_err()),
}

let read_query = ReadQuery::new("SELECT * FROM weather");
let read_result = client.query(read_query).await;
assert_result_err(&read_result);
match read_result {
Err(Error::AuthorizationError) => {}
_ => panic!(
"Should be an AuthorizationError: {}",
read_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 401_u16 {
panic!(
"Should be an ApiError(401), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(401): {}", read_result.unwrap_err()),
}

let client = Client::new("http://127.0.0.1:9086", TEST_NAME)
Expand All @@ -164,11 +172,15 @@ async fn test_wrong_authed_write_and_read() {
let read_result = client.query(read_query).await;
assert_result_err(&read_result);
match read_result {
Err(Error::AuthenticationError) => {}
_ => panic!(
"Should be an AuthenticationError: {}",
read_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 403_u16 {
panic!(
"Should be an ApiError(403), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(403): {}", read_result.unwrap_err()),
}
},
|| async move {
Expand Down Expand Up @@ -208,23 +220,31 @@ async fn test_non_authed_write_and_read() {
let write_result = non_authed_client.query(write_query).await;
assert_result_err(&write_result);
match write_result {
Err(Error::AuthorizationError) => {}
_ => panic!(
"Should be an AuthorizationError: {}",
write_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 401_u16 {
panic!(
"Should be an ApiError(401), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(401): {}", write_result.unwrap_err()),
}

let read_query = ReadQuery::new("SELECT * FROM weather");
let read_result = non_authed_client.query(read_query).await;

assert_result_err(&read_result);
match read_result {
Err(Error::AuthorizationError) => {}
_ => panic!(
"Should be an AuthorizationError: {}",
read_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 401_u16 {
panic!(
"Should be an ApiError(401), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(401): {}", read_result.unwrap_err()),
}
},
|| async move {
Expand Down Expand Up @@ -297,11 +317,15 @@ async fn test_json_non_authed_read() {
let read_result = non_authed_client.json_query(read_query).await;
assert_result_err(&read_result);
match read_result {
Err(Error::AuthorizationError) => {}
_ => panic!(
"Should be a AuthorizationError: {}",
read_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 401_u16 {
panic!(
"Should be an ApiError(401), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(401): {}", read_result.unwrap_err()),
}
},
|| async move {
Expand Down
56 changes: 36 additions & 20 deletions influxdb/tests/integration_tests_v2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,22 +57,30 @@ async fn test_wrong_authed_write_and_read() {
let write_result = client.query(&write_query).await;
assert_result_err(&write_result);
match write_result {
Err(Error::AuthorizationError) => {}
_ => panic!(
"Should be an AuthorizationError: {}",
write_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 401_u16 {
panic!(
"Should be an ApiError(401), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(401): {}", write_result.unwrap_err()),
}

let read_query = ReadQuery::new("SELECT * FROM weather");
let read_result = client.query(&read_query).await;
assert_result_err(&read_result);
match read_result {
Err(Error::AuthorizationError) => {}
_ => panic!(
"Should be an AuthorizationError: {}",
read_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 401_u16 {
panic!(
"Should be an ApiError(401), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(401): {}", read_result.unwrap_err()),
}
},
|| async move {},
Expand All @@ -95,22 +103,30 @@ async fn test_non_authed_write_and_read() {
let write_result = non_authed_client.query(&write_query).await;
assert_result_err(&write_result);
match write_result {
Err(Error::AuthorizationError) => {}
_ => panic!(
"Should be an AuthorizationError: {}",
write_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 401_u16 {
panic!(
"Should be an ApiError(401), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(401): {}", write_result.unwrap_err()),
}

let read_query = ReadQuery::new("SELECT * FROM weather");
let read_result = non_authed_client.query(&read_query).await;
assert_result_err(&read_result);
match read_result {
Err(Error::AuthorizationError) => {}
_ => panic!(
"Should be an AuthorizationError: {}",
read_result.unwrap_err()
),
Err(Error::ApiError(code)) => {
if code.as_u16() != 401_u16 {
panic!(
"Should be an ApiError(401), but code received was: {}",
code
);
};
}
_ => panic!("Should be an ApiError(401): {}", read_result.unwrap_err()),
}
},
|| async move {},
Expand Down