Skip to content

Commit ac86c2d

Browse files
HeliozoaHeliozoa
committed
obsolete-client errorkind
1 parent 7f483a5 commit ac86c2d

File tree

6 files changed

+52
-137
lines changed

6 files changed

+52
-137
lines changed

‎tmc-langs-cli/src/main.rs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,21 @@ fn main() {
6262
/// Goes through the error chain and checks for special error types that should be indicated by the Kind.
6363
fn solve_error_kind(e: &anyhow::Error) -> Kind {
6464
for cause in e.chain() {
65-
// check for authorization error
66-
if let Some(CoreError::HttpError(_, status_code, _)) = cause.downcast_ref::<CoreError>() {
67-
if status_code.as_u16() == 403 {
65+
// check for http errors
66+
if let Some(CoreError::HttpError {
67+
url: _,
68+
status,
69+
error: _,
70+
obsolete_client,
71+
}) = cause.downcast_ref::<CoreError>()
72+
{
73+
if *obsolete_client {
74+
return Kind::ObsoleteClient;
75+
}
76+
if status.as_u16() == 403 {
6877
return Kind::Forbidden;
6978
}
70-
}
71-
// check for not logged in
72-
if let Some(CoreError::HttpError(_, status_code, _)) = cause.downcast_ref::<CoreError>() {
73-
if status_code.as_u16() == 401 {
79+
if status.as_u16() == 401 {
7480
return Kind::NotLoggedIn;
7581
}
7682
}

‎tmc-langs-cli/src/output.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ pub enum Kind {
8282
NotLoggedIn,
8383
/// Failed to connect to the TMC server, likely due to no internet connection
8484
ConnectionError,
85+
/// Client out of date
86+
ObsoleteClient,
8587
}
8688

8789
#[derive(Debug, Serialize, JsonSchema)]

‎tmc-langs-core/src/error.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//! The core error type.
22
3-
use crate::response;
43
use reqwest::{Method, StatusCode};
54
use std::path::PathBuf;
65
use thiserror::Error;
@@ -19,8 +18,13 @@ pub enum CoreError {
1918
TempFile(#[source] std::io::Error),
2019

2120
// network
22-
#[error("HTTP error {1} for {0}: {2}")]
23-
HttpError(Url, StatusCode, String),
21+
#[error("HTTP error {status} for {url}: {error}. Obsolete client: {obsolete_client}")]
22+
HttpError {
23+
url: Url,
24+
status: StatusCode,
25+
error: String,
26+
obsolete_client: bool,
27+
},
2428
#[error("Connection error trying to {0} {1}")]
2529
ConnectionError(Method, Url, #[source] reqwest::Error),
2630
#[error("OAuth2 password exchange error")]
@@ -44,10 +48,6 @@ pub enum CoreError {
4448
#[error(transparent)]
4549
SystemTime(#[from] std::time::SystemTimeError),
4650
#[error(transparent)]
47-
Response(#[from] response::ResponseError),
48-
#[error(transparent)]
49-
ResponseErrors(#[from] response::ResponseErrors),
50-
#[error(transparent)]
5151
WalkDir(#[from] walkdir::Error),
5252
#[error("File IO error")]
5353
FileIo(#[from] FileIo),

‎tmc-langs-core/src/response.rs‎

Lines changed: 6 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//! Contains types which model the JSON responses from tmc-server
22
3-
use crate::CoreError;
43
use lazy_static::lazy_static;
54
use regex::Regex;
65
use schemars::JsonSchema;
@@ -13,42 +12,15 @@ use std::str::FromStr;
1312
use thiserror::Error;
1413
use tmc_langs_util::ValidationResult;
1514

16-
/// Models the responses from tmc-server, which can either
17-
/// be some successful response, a single error or a list of errors
18-
#[derive(Debug, Deserialize)]
19-
#[serde(untagged)]
20-
pub enum Response<T> {
21-
Ok(T),
22-
Errs(ResponseErrors),
23-
Err(ResponseError),
24-
}
25-
26-
impl<T> Response<T> {
27-
/// Convenience function to easily propagate error responses
28-
pub fn into_result(self) -> Result<T, CoreError> {
29-
match self {
30-
Self::Ok(t) => Ok(t),
31-
Self::Err(err) => Err(err.into()),
32-
Self::Errs(errs) => Err(errs.into()),
33-
}
34-
}
35-
}
36-
3715
/// Represents an error response from tmc-server
3816
#[derive(Debug, Error, Deserialize)]
39-
#[error("Response contained errors: {errors:#?}")]
17+
#[error("Response contained errors: {error:?}, {errors:#?}, obsolete client: {obsolete_client}")]
4018
#[serde(deny_unknown_fields)] // prevents responses with an errors field from being parsed as an error
41-
pub struct ResponseErrors {
42-
pub errors: Vec<String>,
43-
}
44-
45-
/// Represents an error response from tmc-server
46-
#[derive(Debug, Error, Deserialize)]
47-
#[error("Response contained an error: {error:#?}. Obsolete client: {}", obsolete_client.unwrap_or_default())]
48-
#[serde(deny_unknown_fields)] // prevents responses with an error field from being parsed as an error
49-
pub struct ResponseError {
50-
pub error: String,
51-
pub obsolete_client: Option<bool>,
19+
pub struct ErrorResponse {
20+
pub error: Option<String>,
21+
pub errors: Option<Vec<String>>,
22+
#[serde(default)]
23+
pub obsolete_client: bool,
5224
}
5325

5426
/// OAuth2 credentials
@@ -526,35 +498,4 @@ mod test {
526498
let range = serde_json::to_value(&range).unwrap();
527499
assert_eq!(range, Value::String("intrange[1..5]".to_string()));
528500
}
529-
530-
#[test]
531-
fn deserializes_struct_with_error_field() {
532-
let json = r#"{
533-
"api_version": 7,
534-
"all_tests_passed": false,
535-
"user_id": 123,
536-
"login": "log",
537-
"course": "cou",
538-
"exercise_name": "exe",
539-
"status": "error",
540-
"points": [],
541-
"validations": null,
542-
"valgrind": null,
543-
"submission_url": "sub",
544-
"solution_url": "sol",
545-
"submitted_at": "sat",
546-
"processing_time": null,
547-
"reviewed": false,
548-
"requests_review": false,
549-
"paste_url": null,
550-
"message_for_paste": null,
551-
"missing_review_points": [],
552-
"error": "error msg"
553-
}"#;
554-
let s: Response<SubmissionProcessingStatus> = serde_json::from_str(json).unwrap();
555-
if let Response::Ok(_) = s {
556-
} else {
557-
panic!("parse failed")
558-
}
559-
}
560501
}

‎tmc-langs-core/src/tmc_core.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -754,8 +754,7 @@ impl TmcCore {
754754

755755
let url = Url::parse(submission_url)
756756
.map_err(|e| CoreError::UrlParse(submission_url.to_string(), e))?;
757-
let res: Response<SubmissionProcessingStatus> = self.get_json_from_url(url)?;
758-
let res = res.into_result()?;
757+
let res: SubmissionProcessingStatus = self.get_json_from_url(url)?;
759758
Ok(res)
760759
}
761760
}

‎tmc-langs-core/src/tmc_core/api.rs‎

Lines changed: 23 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Contains and additional impl for TmcCore for calling the TMC Server API.
22
33
use crate::error::CoreError;
4-
use crate::response::Response;
4+
use crate::response::ErrorResponse;
55
use crate::{
66
Course, CourseData, CourseDataExercise, CourseDataExercisePoint, CourseDetails, CourseExercise,
77
ExerciseDetails, FeedbackAnswer, NewSubmission, Organization, Review, Submission,
@@ -23,54 +23,28 @@ use url::Url;
2323
/// Provides a wrapper for reqwest Response's json that deserializes into Response<T> and converts it into a result
2424
trait CoreExt {
2525
fn json_res<T: DeserializeOwned>(self) -> Result<T, CoreError>;
26-
fn check_error(self, url: Url) -> Result<Self, CoreError>
27-
where
28-
Self: Sized;
2926
}
3027

3128
impl CoreExt for ReqwestResponse {
32-
#[cfg(not(test))]
3329
fn json_res<T: DeserializeOwned>(self) -> Result<T, CoreError> {
34-
let res: Response<T> = self.json().map_err(CoreError::HttpJsonResponse)?;
35-
res.into_result()
36-
}
37-
38-
// logs received JSON for easier debugging in tests
39-
#[cfg(test)]
40-
fn json_res<T: DeserializeOwned>(self) -> Result<T, CoreError> {
41-
let res: Value = self.json().map_err(CoreError::HttpJsonResponse)?;
42-
log::debug!("JSON {}", res);
43-
let res: Response<T> = serde_json::from_value(res).unwrap();
44-
res.into_result()
45-
}
46-
47-
fn check_error(self, url: Url) -> Result<Self, CoreError> {
30+
let url = self.url().clone();
4831
let status = self.status();
4932
if status.is_success() {
50-
Ok(self)
33+
Ok(self.json().map_err(CoreError::HttpJsonResponse)?)
5134
} else {
52-
let text = self.text().unwrap_or_default();
53-
// todo: clean the parsing
54-
let parsed = serde_json::from_str::<Value>(&text)
55-
.ok()
56-
.and_then(|ok| {
57-
ok.as_object().and_then(|obj|
58-
// parses either the error string or errors string array
59-
if let Some(error) = obj.get("error").and_then(|e| e.as_str()) {
60-
Some(error.to_string())
61-
} else if let Some(errors) = obj.get("errors").and_then(|e| e.as_array()) {
62-
let errors = errors
63-
.iter()
64-
.filter_map(|e| e.as_str())
65-
.collect::<Vec<_>>()
66-
.join(". ");
67-
Some(errors)
68-
} else {
69-
None
70-
})
71-
})
72-
.unwrap_or(text);
73-
Err(CoreError::HttpError(url, status, parsed))
35+
let err: ErrorResponse = self.json().map_err(CoreError::HttpJsonResponse)?;
36+
let error = match (err.error, err.errors) {
37+
(Some(err), Some(errs)) => format!("{}, {}", err, errs.join(",")),
38+
(Some(err), None) => err,
39+
(None, Some(errs)) => errs.join(","),
40+
_ => "".to_string(),
41+
};
42+
Err(CoreError::HttpError {
43+
url,
44+
status,
45+
error,
46+
obsolete_client: err.obsolete_client,
47+
})
7448
}
7549
}
7650
}
@@ -111,8 +85,7 @@ impl TmcCore {
11185
.get(url.clone())
11286
.core_headers(self)
11387
.send()
114-
.map_err(|e| CoreError::ConnectionError(Method::GET, url.clone(), e))?
115-
.check_error(url)?
88+
.map_err(|e| CoreError::ConnectionError(Method::GET, url, e))?
11689
.json_res()
11790
}
11891

@@ -129,8 +102,7 @@ impl TmcCore {
129102
.get(url.clone())
130103
.core_headers(self)
131104
.send()
132-
.map_err(|e| CoreError::ConnectionError(Method::GET, url.clone(), e))?
133-
.check_error(url)?
105+
.map_err(|e| CoreError::ConnectionError(Method::GET, url, e))?
134106
.copy_to(&mut target_file)
135107
.map_err(|e| CoreError::HttpWriteResponse(target.to_path_buf(), e))?;
136108
Ok(())
@@ -144,8 +116,7 @@ impl TmcCore {
144116
.get(url.clone())
145117
.core_headers(self)
146118
.send()
147-
.map_err(|e| CoreError::ConnectionError(Method::GET, url.clone(), e))?
148-
.check_error(url)?
119+
.map_err(|e| CoreError::ConnectionError(Method::GET, url, e))?
149120
.copy_to(&mut target_file)
150121
.map_err(|e| CoreError::HttpWriteResponse(target.to_path_buf(), e))?;
151122
Ok(())
@@ -709,8 +680,7 @@ impl TmcCore {
709680
.multipart(form)
710681
.core_headers(self)
711682
.send()
712-
.map_err(|e| CoreError::ConnectionError(Method::POST, submission_url.clone(), e))?
713-
.check_error(submission_url)?
683+
.map_err(|e| CoreError::ConnectionError(Method::POST, submission_url, e))?
714684
.json_res()?;
715685
log::debug!("received {:?}", res);
716686
Ok(res)
@@ -762,8 +732,7 @@ impl TmcCore {
762732
.multipart(form)
763733
.core_headers(self)
764734
.send()
765-
.map_err(|e| CoreError::ConnectionError(Method::POST, feedback_url.clone(), e))?
766-
.check_error(feedback_url)?
735+
.map_err(|e| CoreError::ConnectionError(Method::POST, feedback_url, e))?
767736
.json_res()
768737
}
769738

@@ -788,8 +757,7 @@ impl TmcCore {
788757
.query(&[("review[points]", review_points)])
789758
.core_headers(self)
790759
.send()
791-
.map_err(|e| CoreError::ConnectionError(Method::POST, url.clone(), e))?
792-
.check_error(url)?
760+
.map_err(|e| CoreError::ConnectionError(Method::POST, url, e))?
793761
.json_res()?;
794762
log::trace!("received {:?}", res);
795763
Ok(())
@@ -815,8 +783,7 @@ impl TmcCore {
815783
.post(url.clone())
816784
.multipart(form)
817785
.send()
818-
.map_err(|e| CoreError::ConnectionError(Method::POST, url.clone(), e))?
819-
.check_error(url)?
786+
.map_err(|e| CoreError::ConnectionError(Method::POST, url, e))?
820787
.json_res()
821788
}
822789
}

0 commit comments

Comments
 (0)