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

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

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 1 commit
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
11 changes: 4 additions & 7 deletions influxdb/src/client/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,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 +301,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 {
StatusCode::OK => Ok(()),
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
code => Err(Error::ApiError(code)),
}
}

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

use reqwest::StatusCode;
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
use thiserror::Error;

#[derive(Debug, Eq, PartialEq, Error)]
#[derive(Debug, Error)]
pub enum Error {
#[error("query is invalid: {error}")]
/// Error happens when a query is invalid
Expand All @@ -24,15 +25,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),
}
24 changes: 18 additions & 6 deletions influxdb/tests/integration_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,9 @@ 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) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be an AuthorizationError: {}",
write_result.unwrap_err()
Expand All @@ -151,7 +153,9 @@ 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::AuthorizationError) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be an AuthorizationError: {}",
read_result.unwrap_err()
Expand All @@ -164,7 +168,9 @@ 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) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be an AuthenticationError: {}",
read_result.unwrap_err()
Expand Down Expand Up @@ -208,7 +214,9 @@ 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) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be an AuthorizationError: {}",
write_result.unwrap_err()
Expand All @@ -220,7 +228,9 @@ async fn test_non_authed_write_and_read() {

assert_result_err(&read_result);
match read_result {
Err(Error::AuthorizationError) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be an AuthorizationError: {}",
read_result.unwrap_err()
Expand Down Expand Up @@ -297,7 +307,9 @@ 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) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be a AuthorizationError: {}",
read_result.unwrap_err()
Expand Down
16 changes: 12 additions & 4 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,7 +57,9 @@ 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) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be an AuthorizationError: {}",
write_result.unwrap_err()
Expand All @@ -68,7 +70,9 @@ 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::AuthorizationError) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be an AuthorizationError: {}",
read_result.unwrap_err()
Expand All @@ -95,7 +99,9 @@ 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) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be an AuthorizationError: {}",
write_result.unwrap_err()
Expand All @@ -106,7 +112,9 @@ async fn test_non_authed_write_and_read() {
let read_result = non_authed_client.query(&read_query).await;
assert_result_err(&read_result);
match read_result {
Err(Error::AuthorizationError) => {}
Err(Error::ApiError(code)) => {
dbg!(code);
0xDmtri marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(
"Should be an AuthorizationError: {}",
read_result.unwrap_err()
Expand Down
Loading