Commit 7140097f authored by Ben Boeckel's avatar Ben Boeckel Committed by Kitware Robot
Browse files

Merge topic 'more-error-types-from-gitlab'

a6014c9c

 api/error: handle error objects from the server
Acked-by: Kitware Robot's avatarKitware Robot <kwrobot@kitware.com>
Acked-by: Brad King's avatarBrad King <brad.king@kitware.com>
Merge-request: !245
parents b783e839 a6014c9c
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
* The `api::Client` trait has been changed to use the `http` crate types. * The `api::Client` trait has been changed to use the `http` crate types.
This allows for clients to not be tied to `reqwest` and for mocking and This allows for clients to not be tied to `reqwest` and for mocking and
testing of the endpoints themselves. testing of the endpoints themselves.
* GitLab errors now detect error objects returned from the API.
## Fixes ## Fixes
......
...@@ -208,20 +208,18 @@ mod tests { ...@@ -208,20 +208,18 @@ mod tests {
.status(StatusCode::NOT_FOUND) .status(StatusCode::NOT_FOUND)
.build() .build()
.unwrap(); .unwrap();
let client = SingleTestClient::new_json( let err_obj = json!({
endpoint,
&json!({
"bogus": "dummy error message", "bogus": "dummy error message",
}), });
); let client = SingleTestClient::new_json(endpoint, &err_obj);
let res: Result<DummyResult, _> = Dummy.query(&client); let res: Result<DummyResult, _> = Dummy.query(&client);
let err = res.unwrap_err(); let err = res.unwrap_err();
if let ApiError::Gitlab { if let ApiError::GitlabUnrecognized {
msg, obj,
} = err } = err
{ {
assert_eq!(msg, "<unknown error>"); assert_eq!(obj, err_obj);
} else { } else {
panic!("unexpected error: {}", err); panic!("unexpected error: {}", err);
} }
......
...@@ -70,6 +70,18 @@ where ...@@ -70,6 +70,18 @@ where
/// The error message from GitLab. /// The error message from GitLab.
msg: String, msg: String,
}, },
/// GitLab returned an error object.
#[error("gitlab server error: {:?}", obj)]
GitlabObject {
/// The error object from GitLab.
obj: serde_json::Value,
},
/// GitLab returned an HTTP error with JSON we did not recognize.
#[error("gitlab server error: {:?}", obj)]
GitlabUnrecognized {
/// The full object from GitLab.
obj: serde_json::Value,
},
/// Failed to parse an expected data type from JSON. /// Failed to parse an expected data type from JSON.
#[error("could not parse {} data from JSON: {}", typename, source)] #[error("could not parse {} data from JSON: {}", typename, source)]
DataType { DataType {
...@@ -105,15 +117,25 @@ where ...@@ -105,15 +117,25 @@ where
} }
pub(crate) fn from_gitlab(value: serde_json::Value) -> Self { pub(crate) fn from_gitlab(value: serde_json::Value) -> Self {
let msg = value let error_value = value
.pointer("/message") .pointer("/message")
.or_else(|| value.pointer("/error")) .or_else(|| value.pointer("/error"));
.and_then(|s| s.as_str())
.unwrap_or_else(|| "<unknown error>");
if let Some(error_value) = error_value {
if let Some(msg) = error_value.as_str() {
ApiError::Gitlab { ApiError::Gitlab {
msg: msg.into(), msg: msg.into(),
} }
} else {
ApiError::GitlabObject {
obj: error_value.clone(),
}
}
} else {
ApiError::GitlabUnrecognized {
obj: value,
}
}
} }
pub(crate) fn data_type<T>(source: serde_json::Error) -> Self { pub(crate) fn data_type<T>(source: serde_json::Error) -> Self {
...@@ -123,3 +145,86 @@ where ...@@ -123,3 +145,86 @@ where
} }
} }
} }
#[cfg(test)]
mod tests {
use serde_json::json;
use thiserror::Error;
use crate::api::ApiError;
#[derive(Debug, Error)]
#[error("my error")]
enum MyError {}
#[test]
fn gitlab_error_error() {
let obj = json!({
"error": "error contents",
});
let err: ApiError<MyError> = ApiError::from_gitlab(obj);
if let ApiError::Gitlab {
msg,
} = err
{
assert_eq!(msg, "error contents");
} else {
panic!("unexpected error: {}", err);
}
}
#[test]
fn gitlab_error_message_string() {
let obj = json!({
"message": "error contents",
});
let err: ApiError<MyError> = ApiError::from_gitlab(obj);
if let ApiError::Gitlab {
msg,
} = err
{
assert_eq!(msg, "error contents");
} else {
panic!("unexpected error: {}", err);
}
}
#[test]
fn gitlab_error_message_object() {
let err_obj = json!({
"blah": "foo",
});
let obj = json!({
"message": err_obj.clone(),
});
let err: ApiError<MyError> = ApiError::from_gitlab(obj);
if let ApiError::GitlabObject {
obj,
} = err
{
assert_eq!(obj, err_obj);
} else {
panic!("unexpected error: {}", err);
}
}
#[test]
fn gitlab_error_message_unrecognized() {
let err_obj = json!({
"some_weird_key": "an even weirder value",
});
let err: ApiError<MyError> = ApiError::from_gitlab(err_obj.clone());
if let ApiError::GitlabUnrecognized {
obj,
} = err
{
assert_eq!(obj, err_obj);
} else {
panic!("unexpected error: {}", err);
}
}
}
...@@ -163,19 +163,17 @@ mod tests { ...@@ -163,19 +163,17 @@ mod tests {
.status(StatusCode::NOT_FOUND) .status(StatusCode::NOT_FOUND)
.build() .build()
.unwrap(); .unwrap();
let client = SingleTestClient::new_json( let err_obj = json!({
endpoint,
&json!({
"bogus": "dummy error message", "bogus": "dummy error message",
}), });
); let client = SingleTestClient::new_json(endpoint, &err_obj);
let err = api::ignore(Dummy).query(&client).unwrap_err(); let err = api::ignore(Dummy).query(&client).unwrap_err();
if let ApiError::Gitlab { if let ApiError::GitlabUnrecognized {
msg, obj,
} = err } = err
{ {
assert_eq!(msg, "<unknown error>"); assert_eq!(obj, err_obj);
} else { } else {
panic!("unexpected error: {}", err); panic!("unexpected error: {}", err);
} }
......
...@@ -499,21 +499,19 @@ mod tests { ...@@ -499,21 +499,19 @@ mod tests {
.status(StatusCode::NOT_FOUND) .status(StatusCode::NOT_FOUND)
.build() .build()
.unwrap(); .unwrap();
let client = SingleTestClient::new_json( let err_obj = json!({
endpoint,
&json!({
"bogus": "dummy error message", "bogus": "dummy error message",
}), });
); let client = SingleTestClient::new_json(endpoint, &err_obj);
let endpoint = Dummy::default(); let endpoint = Dummy::default();
let res: Result<Vec<DummyResult>, _> = api::paged(endpoint, Pagination::All).query(&client); let res: Result<Vec<DummyResult>, _> = api::paged(endpoint, Pagination::All).query(&client);
let err = res.unwrap_err(); let err = res.unwrap_err();
if let ApiError::Gitlab { if let ApiError::GitlabUnrecognized {
msg, obj,
} = err } = err
{ {
assert_eq!(msg, "<unknown error>"); assert_eq!(obj, err_obj);
} else { } else {
panic!("unexpected error: {}", err); panic!("unexpected error: {}", err);
} }
......
...@@ -164,19 +164,17 @@ mod tests { ...@@ -164,19 +164,17 @@ mod tests {
.status(StatusCode::NOT_FOUND) .status(StatusCode::NOT_FOUND)
.build() .build()
.unwrap(); .unwrap();
let client = SingleTestClient::new_json( let err_obj = json!({
endpoint,
&json!({
"bogus": "dummy error message", "bogus": "dummy error message",
}), });
); let client = SingleTestClient::new_json(endpoint, &err_obj);
let err = api::raw(Dummy).query(&client).unwrap_err(); let err = api::raw(Dummy).query(&client).unwrap_err();
if let ApiError::Gitlab { if let ApiError::GitlabUnrecognized {
msg, obj,
} = err } = err
{ {
assert_eq!(msg, "<unknown error>"); assert_eq!(obj, err_obj);
} else { } else {
panic!("unexpected error: {}", err); panic!("unexpected error: {}", err);
} }
......
...@@ -245,20 +245,18 @@ mod tests { ...@@ -245,20 +245,18 @@ mod tests {
.status(StatusCode::NOT_FOUND) .status(StatusCode::NOT_FOUND)
.build() .build()
.unwrap(); .unwrap();
let client = SingleTestClient::new_json( let err_obj = json!({
endpoint,
&json!({
"bogus": "dummy error message", "bogus": "dummy error message",
}), });
); let client = SingleTestClient::new_json(endpoint, &err_obj);
let res: Result<DummyResult, _> = api::sudo(Dummy, "user").query(&client); let res: Result<DummyResult, _> = api::sudo(Dummy, "user").query(&client);
let err = res.unwrap_err(); let err = res.unwrap_err();
if let ApiError::Gitlab { if let ApiError::GitlabUnrecognized {
msg, obj,
} = err } = err
{ {
assert_eq!(msg, "<unknown error>"); assert_eq!(obj, err_obj);
} else { } else {
panic!("unexpected error: {}", err); panic!("unexpected error: {}", err);
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment