Skip to content

Commit 9d8d453

Browse files
0xDmtriEmpty2k12
0xDmtri
and
Empty2k12
authored
Better Errors (#150)
* bump * error with an api status code * remove redundant import * fix ApiError * fix ApiError by converting StatusCode to u16 * revert * u16 as StatusCode * update tests * improve tests readability * fix: ci tests & add docker-compose local test harness --------- Co-authored-by: Gero Gerke <[email protected]>
1 parent bfe457c commit 9d8d453

File tree

5 files changed

+65
-36
lines changed

5 files changed

+65
-36
lines changed

Diff for: ‎docker-compose.integrationtest.yaml

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
version: '3'
2+
services:
3+
influxdb:
4+
image: influxdb:1.8
5+
ports:
6+
- 8086:8086
7+
authed_influxdb:
8+
image: influxdb:1.8
9+
ports:
10+
- 9086:8086
11+
environment:
12+
INFLUXDB_HTTP_AUTH_ENABLED: true
13+
INFLUXDB_ADMIN_USER: admin
14+
INFLUXDB_ADMIN_PASSWORD: password
15+
INFLUXDB_USER: nopriv_user
16+
INFLUXDB_USER_PASSWORD: password
17+
influxdbv2:
18+
image: influxdb:2.6
19+
ports:
20+
- 2086:8086
21+
environment:
22+
DOCKER_INFLUXDB_INIT_MODE: setup
23+
DOCKER_INFLUXDB_INIT_USERNAME: admin
24+
DOCKER_INFLUXDB_INIT_PASSWORD: password
25+
DOCKER_INFLUXDB_INIT_ORG: testing
26+
DOCKER_INFLUXDB_INIT_BUCKET: mydb
27+
DOCKER_INFLUXDB_INIT_ADMIN_TOKEN: admintoken

Diff for: ‎influxdb/src/client/mod.rs

+4-8
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
//! ```
1717
1818
use futures_util::TryFutureExt;
19-
use http::StatusCode;
2019
#[cfg(feature = "reqwest")]
2120
use reqwest::{Client as HttpClient, RequestBuilder, Response as HttpResponse};
2221
use std::collections::{BTreeMap, HashMap};
@@ -281,7 +280,7 @@ impl Client {
281280
})?;
282281

283282
// todo: improve error parsing without serde
284-
if s.contains("\"error\"") {
283+
if s.contains("\"error\"") || s.contains("\"Error\"") {
285284
return Err(Error::DatabaseError {
286285
error: format!("influxdb error: \"{}\"", s),
287286
});
@@ -301,13 +300,10 @@ impl Client {
301300

302301
pub(crate) fn check_status(res: &HttpResponse) -> Result<(), Error> {
303302
let status = res.status();
304-
if status == StatusCode::UNAUTHORIZED.as_u16() {
305-
Err(Error::AuthorizationError)
306-
} else if status == StatusCode::FORBIDDEN.as_u16() {
307-
Err(Error::AuthenticationError)
308-
} else {
309-
Ok(())
303+
if !status.is_success() {
304+
return Err(Error::ApiError(status.into()));
310305
}
306+
Ok(())
311307
}
312308

313309
#[cfg(test)]

Diff for: ‎influxdb/src/error.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,9 @@ pub enum Error {
2525
/// Error which has happened inside InfluxDB
2626
DatabaseError { error: String },
2727

28-
#[error("authentication error. No or incorrect credentials")]
29-
/// Error happens when no or incorrect credentials are used. `HTTP 401 Unauthorized`
30-
AuthenticationError,
31-
32-
#[error("authorization error. User not authorized")]
33-
/// Error happens when the supplied user is not authorized. `HTTP 403 Forbidden`
34-
AuthorizationError,
28+
#[error("API error with a status code: {0}")]
29+
/// Error happens when API returns non 2xx status code.
30+
ApiError(u16),
3531

3632
#[error("connection error: {error}")]
3733
/// Error happens when HTTP request fails

Diff for: ‎influxdb/tests/integration_tests.rs

+18-12
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ async fn test_authed_write_and_read() {
120120
#[async_std::test]
121121
#[cfg(not(tarpaulin_include))]
122122
async fn test_wrong_authed_write_and_read() {
123+
use http::StatusCode;
124+
123125
const TEST_NAME: &str = "test_wrong_authed_write_and_read";
124126

125127
run_test(
@@ -140,9 +142,9 @@ async fn test_wrong_authed_write_and_read() {
140142
let write_result = client.query(write_query).await;
141143
assert_result_err(&write_result);
142144
match write_result {
143-
Err(Error::AuthorizationError) => {}
145+
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
144146
_ => panic!(
145-
"Should be an AuthorizationError: {}",
147+
"Should be an ApiError(UNAUTHORIZED): {}",
146148
write_result.unwrap_err()
147149
),
148150
}
@@ -151,9 +153,9 @@ async fn test_wrong_authed_write_and_read() {
151153
let read_result = client.query(read_query).await;
152154
assert_result_err(&read_result);
153155
match read_result {
154-
Err(Error::AuthorizationError) => {}
156+
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
155157
_ => panic!(
156-
"Should be an AuthorizationError: {}",
158+
"Should be an ApiError(UNAUTHORIZED): {}",
157159
read_result.unwrap_err()
158160
),
159161
}
@@ -164,9 +166,9 @@ async fn test_wrong_authed_write_and_read() {
164166
let read_result = client.query(read_query).await;
165167
assert_result_err(&read_result);
166168
match read_result {
167-
Err(Error::AuthenticationError) => {}
169+
Err(Error::ApiError(code)) if code == StatusCode::FORBIDDEN.as_u16() => {}
168170
_ => panic!(
169-
"Should be an AuthenticationError: {}",
171+
"Should be an ApiError(UNAUTHENTICATED): {}",
170172
read_result.unwrap_err()
171173
),
172174
}
@@ -190,6 +192,8 @@ async fn test_wrong_authed_write_and_read() {
190192
#[async_std::test]
191193
#[cfg(not(tarpaulin_include))]
192194
async fn test_non_authed_write_and_read() {
195+
use http::StatusCode;
196+
193197
const TEST_NAME: &str = "test_non_authed_write_and_read";
194198

195199
run_test(
@@ -208,9 +212,9 @@ async fn test_non_authed_write_and_read() {
208212
let write_result = non_authed_client.query(write_query).await;
209213
assert_result_err(&write_result);
210214
match write_result {
211-
Err(Error::AuthorizationError) => {}
215+
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
212216
_ => panic!(
213-
"Should be an AuthorizationError: {}",
217+
"Should be an ApiError(UNAUTHORIZED): {}",
214218
write_result.unwrap_err()
215219
),
216220
}
@@ -220,9 +224,9 @@ async fn test_non_authed_write_and_read() {
220224

221225
assert_result_err(&read_result);
222226
match read_result {
223-
Err(Error::AuthorizationError) => {}
227+
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
224228
_ => panic!(
225-
"Should be an AuthorizationError: {}",
229+
"Should be an ApiError(UNAUTHORIZED): {}",
226230
read_result.unwrap_err()
227231
),
228232
}
@@ -280,6 +284,8 @@ async fn test_write_and_read_field() {
280284
#[cfg(feature = "serde")]
281285
#[cfg(not(tarpaulin_include))]
282286
async fn test_json_non_authed_read() {
287+
use http::StatusCode;
288+
283289
const TEST_NAME: &str = "test_json_non_authed_read";
284290

285291
run_test(
@@ -297,9 +303,9 @@ async fn test_json_non_authed_read() {
297303
let read_result = non_authed_client.json_query(read_query).await;
298304
assert_result_err(&read_result);
299305
match read_result {
300-
Err(Error::AuthorizationError) => {}
306+
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
301307
_ => panic!(
302-
"Should be a AuthorizationError: {}",
308+
"Should be an ApiError(UNAUTHORIZED): {}",
303309
read_result.unwrap_err()
304310
),
305311
}

Diff for: ‎influxdb/tests/integration_tests_v2.rs

+13-9
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ async fn test_authed_write_and_read() {
3333
},
3434
|| async move {
3535
let client = Client::new("http://127.0.0.1:2086", "mydb").with_token("admintoken");
36-
let read_query = ReadQuery::new("DELETE MEASUREMENT weather");
36+
let read_query = ReadQuery::new("DROP MEASUREMENT \"weather\"");
3737
let read_result = client.query(read_query).await;
3838
assert_result_ok(&read_result);
3939
assert!(!read_result.unwrap().contains("error"), "Teardown failed");
@@ -48,6 +48,8 @@ async fn test_authed_write_and_read() {
4848
#[async_std::test]
4949
#[cfg(not(tarpaulin))]
5050
async fn test_wrong_authed_write_and_read() {
51+
use http::StatusCode;
52+
5153
run_test(
5254
|| async move {
5355
let client = Client::new("http://127.0.0.1:2086", "mydb").with_token("falsetoken");
@@ -57,9 +59,9 @@ async fn test_wrong_authed_write_and_read() {
5759
let write_result = client.query(&write_query).await;
5860
assert_result_err(&write_result);
5961
match write_result {
60-
Err(Error::AuthorizationError) => {}
62+
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
6163
_ => panic!(
62-
"Should be an AuthorizationError: {}",
64+
"Should be an ApiError(UNAUTHORIZED): {}",
6365
write_result.unwrap_err()
6466
),
6567
}
@@ -68,9 +70,9 @@ async fn test_wrong_authed_write_and_read() {
6870
let read_result = client.query(&read_query).await;
6971
assert_result_err(&read_result);
7072
match read_result {
71-
Err(Error::AuthorizationError) => {}
73+
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
7274
_ => panic!(
73-
"Should be an AuthorizationError: {}",
75+
"Should be an ApiError(UNAUTHORIZED): {}",
7476
read_result.unwrap_err()
7577
),
7678
}
@@ -86,6 +88,8 @@ async fn test_wrong_authed_write_and_read() {
8688
#[async_std::test]
8789
#[cfg(not(tarpaulin))]
8890
async fn test_non_authed_write_and_read() {
91+
use http::StatusCode;
92+
8993
run_test(
9094
|| async move {
9195
let non_authed_client = Client::new("http://127.0.0.1:2086", "mydb");
@@ -95,9 +99,9 @@ async fn test_non_authed_write_and_read() {
9599
let write_result = non_authed_client.query(&write_query).await;
96100
assert_result_err(&write_result);
97101
match write_result {
98-
Err(Error::AuthorizationError) => {}
102+
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
99103
_ => panic!(
100-
"Should be an AuthorizationError: {}",
104+
"Should be an ApiError(UNAUTHORIZED): {}",
101105
write_result.unwrap_err()
102106
),
103107
}
@@ -106,9 +110,9 @@ async fn test_non_authed_write_and_read() {
106110
let read_result = non_authed_client.query(&read_query).await;
107111
assert_result_err(&read_result);
108112
match read_result {
109-
Err(Error::AuthorizationError) => {}
113+
Err(Error::ApiError(code)) if code == StatusCode::UNAUTHORIZED.as_u16() => {}
110114
_ => panic!(
111-
"Should be an AuthorizationError: {}",
115+
"Should be an ApiError(UNAUTHORIZED): {}",
112116
read_result.unwrap_err()
113117
),
114118
}

0 commit comments

Comments
 (0)