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.

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 Errors #150

Merged
merged 11 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 27 additions & 0 deletions docker-compose.integrationtest.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
version: '3'
services:
influxdb:
image: influxdb:1.8
ports:
- 8086:8086
authed_influxdb:
image: influxdb:1.8
ports:
- 9086:8086
environment:
INFLUXDB_HTTP_AUTH_ENABLED: true
INFLUXDB_ADMIN_USER: admin
INFLUXDB_ADMIN_PASSWORD: password
INFLUXDB_USER: nopriv_user
INFLUXDB_USER_PASSWORD: password
influxdbv2:
image: influxdb:2.6
ports:
- 2086:8086
environment:
DOCKER_INFLUXDB_INIT_MODE: setup
DOCKER_INFLUXDB_INIT_USERNAME: admin
DOCKER_INFLUXDB_INIT_PASSWORD: password
DOCKER_INFLUXDB_INIT_ORG: testing
DOCKER_INFLUXDB_INIT_BUCKET: mydb
DOCKER_INFLUXDB_INIT_ADMIN_TOKEN: admintoken
12 changes: 4 additions & 8 deletions influxdb/src/client/mod.rs
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,13 +300,10 @@ 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(())
if !status.is_success() {
return Err(Error::ApiError(status.into()));
}
Ok(())
}

#[cfg(test)]
Expand Down
10 changes: 3 additions & 7 deletions influxdb/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,9 @@ 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("API error with a status code: {0}")]
/// Error happens when API returns non 2xx status code.
ApiError(u16),

#[error("connection error: {error}")]
/// Error happens when HTTP request fails
Expand Down
30 changes: 18 additions & 12 deletions influxdb/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ async fn test_authed_write_and_read() {
#[async_std::test]
#[cfg(not(tarpaulin_include))]
async fn test_wrong_authed_write_and_read() {
use http::StatusCode;

const TEST_NAME: &str = "test_wrong_authed_write_and_read";

run_test(
Expand All @@ -140,9 +142,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)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
_ => panic!(
"Should be an AuthorizationError: {}",
"Should be an ApiError(UNAUTHORIZED): {}",
write_result.unwrap_err()
),
}
Expand All @@ -151,9 +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)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
_ => panic!(
"Should be an AuthorizationError: {}",
"Should be an ApiError(UNAUTHORIZED): {}",
read_result.unwrap_err()
),
}
Expand All @@ -164,9 +166,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)) if code == StatusCode::FORBIDDEN.as_u16() => {}
_ => panic!(
"Should be an AuthenticationError: {}",
"Should be an ApiError(UNAUTHENTICATED): {}",
read_result.unwrap_err()
),
}
Expand All @@ -190,6 +192,8 @@ async fn test_wrong_authed_write_and_read() {
#[async_std::test]
#[cfg(not(tarpaulin_include))]
async fn test_non_authed_write_and_read() {
use http::StatusCode;

const TEST_NAME: &str = "test_non_authed_write_and_read";

run_test(
Expand All @@ -208,9 +212,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)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
_ => panic!(
"Should be an AuthorizationError: {}",
"Should be an ApiError(UNAUTHORIZED): {}",
write_result.unwrap_err()
),
}
Expand All @@ -220,9 +224,9 @@ async fn test_non_authed_write_and_read() {

assert_result_err(&read_result);
match read_result {
Err(Error::AuthorizationError) => {}
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
_ => panic!(
"Should be an AuthorizationError: {}",
"Should be an ApiError(UNAUTHORIZED): {}",
read_result.unwrap_err()
),
}
Expand Down Expand Up @@ -280,6 +284,8 @@ async fn test_write_and_read_field() {
#[cfg(feature = "serde")]
#[cfg(not(tarpaulin_include))]
async fn test_json_non_authed_read() {
use http::StatusCode;

const TEST_NAME: &str = "test_json_non_authed_read";

run_test(
Expand All @@ -297,9 +303,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)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
_ => panic!(
"Should be a AuthorizationError: {}",
"Should be an ApiError(UNAUTHORIZED): {}",
read_result.unwrap_err()
),
}
Expand Down
22 changes: 13 additions & 9 deletions influxdb/tests/integration_tests_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ async fn test_authed_write_and_read() {
},
|| async move {
let client = Client::new("http://127.0.0.1:2086", "mydb").with_token("admintoken");
let read_query = ReadQuery::new("DELETE MEASUREMENT weather");
let read_query = ReadQuery::new("DROP MEASUREMENT \"weather\"");
let read_result = client.query(read_query).await;
assert_result_ok(&read_result);
assert!(!read_result.unwrap().contains("error"), "Teardown failed");
Expand All @@ -48,6 +48,8 @@ async fn test_authed_write_and_read() {
#[async_std::test]
#[cfg(not(tarpaulin))]
async fn test_wrong_authed_write_and_read() {
use http::StatusCode;

run_test(
|| async move {
let client = Client::new("http://127.0.0.1:2086", "mydb").with_token("falsetoken");
Expand All @@ -57,9 +59,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)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
_ => panic!(
"Should be an AuthorizationError: {}",
"Should be an ApiError(UNAUTHORIZED): {}",
write_result.unwrap_err()
),
}
Expand All @@ -68,9 +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)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
_ => panic!(
"Should be an AuthorizationError: {}",
"Should be an ApiError(UNAUTHORIZED): {}",
read_result.unwrap_err()
),
}
Expand All @@ -86,6 +88,8 @@ async fn test_wrong_authed_write_and_read() {
#[async_std::test]
#[cfg(not(tarpaulin))]
async fn test_non_authed_write_and_read() {
use http::StatusCode;

run_test(
|| async move {
let non_authed_client = Client::new("http://127.0.0.1:2086", "mydb");
Expand All @@ -95,9 +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)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
_ => panic!(
"Should be an AuthorizationError: {}",
"Should be an ApiError(UNAUTHORIZED): {}",
write_result.unwrap_err()
),
}
Expand All @@ -106,9 +110,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)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
_ => panic!(
"Should be an AuthorizationError: {}",
"Should be an ApiError(UNAUTHORIZED): {}",
read_result.unwrap_err()
),
}
Expand Down