From: Simon Tatham Date: Thu, 25 Jan 2024 18:09:41 +0000 (+0000) Subject: Reorganise the ClientError enum. X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ian/git?a=commitdiff_plain;h=57bea7704d067d9ac8ebb2a7e02a166937a66942;p=mastodonochrome.git Reorganise the ClientError enum. Now it has lots more cases, which should make it easier to format each one nicely into an entry for a TUI-displayed error log. Other changes: - when we keep the error message from the JSON error document, we _also_ keep the HTTP status code - lots of from_request calls were missing (possibly in the stuff I moved in here from the old separate login.rs). Reinstated. --- diff --git a/src/client.rs b/src/client.rs index 270add3..88afc89 100644 --- a/src/client.rs +++ b/src/client.rs @@ -122,10 +122,18 @@ pub struct Client { #[derive(Debug, PartialEq, Eq, Clone)] pub enum ClientError { - InternalError(String), // message - UrlParseError(String, String), // url, message - UrlError(String, String), // url, message - NoAccountSource, + Internal(String), // message + UrlParse(String, String), // url, parsing error message + UrlFetchNet(String, String), // url, error message + UrlFetchHTTP(String, reqwest::StatusCode), // url, status + + // url, status, error text from the JSON error document + UrlFetchHTTPRich(String, reqwest::StatusCode, String), + + LinkParse(String, String), // url, parsing error message + JSONParse(String, String), // url, parsing error message + UrlConsistency(String, String), // url, error message + Consistency(String), // just error message } impl super::TopLevelErrorCandidate for ClientError {} @@ -134,9 +142,9 @@ impl From for ClientError { fn from(err: reqwest::Error) -> Self { match err.url() { Some(url) => { - ClientError::UrlError(url.to_string(), err.to_string()) + ClientError::UrlFetchNet(url.to_string(), err.to_string()) } - None => ClientError::InternalError(err.to_string()), + None => ClientError::Internal(err.to_string()), } } } @@ -144,17 +152,19 @@ impl From for ClientError { impl ClientError { fn from_response(url: &str, rsp: reqwest::blocking::Response) -> Self { let rspstatus = rsp.status(); - let message = if let Ok(text) = rsp.text() { + if let Ok(text) = rsp.text() { if let Ok(err) = serde_json::from_str::(&text) { - err.error + ClientError::UrlFetchHTTPRich( + url.to_owned(), + rspstatus, + err.error, + ) } else { - rspstatus.to_string() + ClientError::UrlFetchHTTP(url.to_owned(), rspstatus) } } else { - rspstatus.to_string() - }; - - ClientError::UrlError(url.to_owned(), message) + ClientError::UrlFetchHTTP(url.to_owned(), rspstatus) + } } } @@ -164,19 +174,43 @@ impl std::fmt::Display for ClientError { f: &mut std::fmt::Formatter<'_>, ) -> Result<(), std::fmt::Error> { match self { - ClientError::InternalError(ref msg) => { + ClientError::Internal(ref msg) => { write!(f, "internal failure: {}", msg) } - ClientError::UrlParseError(ref url, ref msg) => { + ClientError::UrlParse(ref url, ref msg) => { write!(f, "Parse failure {} (retrieving URL: {})", msg, url) } - ClientError::UrlError(ref url, ref msg) => { + ClientError::UrlFetchNet(ref url, ref msg) => { write!(f, "{} (retrieving URL: {})", msg, url) } - ClientError::NoAccountSource => write!( - f, - "server did not send 'source' details for our account" - ), + ClientError::UrlFetchHTTP(ref url, status) => { + write!(f, "{} (retrieving URL: {})", status.to_string(), url) + } + ClientError::UrlFetchHTTPRich(ref url, status, ref msg) => { + write!( + f, + "{} (HTTP status {}; retrieving URL: {})", + msg, + status.to_string(), + url + ) + } + ClientError::JSONParse(ref url, ref msg) => { + write!(f, "{} (parsing JSON returned from URL: {})", msg, url) + } + ClientError::LinkParse(ref url, ref msg) => { + write!( + f, + "{} (parsing Link header returned from URL: {})", + msg, url + ) + } + ClientError::UrlConsistency(ref url, ref msg) => { + write!(f, "{} (after fetching URL: {})", msg, url) + } + ClientError::Consistency(ref msg) => { + write!(f, "{}", msg) + } } } } @@ -232,7 +266,7 @@ impl Req { let url = match parsed { Ok(url) => Ok(url), Err(e) => { - Err(ClientError::UrlParseError(urlstr.clone(), e.to_string())) + Err(ClientError::UrlParse(urlstr.clone(), e.to_string())) } }?; Ok((urlstr, url)) @@ -551,7 +585,7 @@ impl Client { req: Req, ) -> Result<(String, reqwest::blocking::RequestBuilder), ClientError> { if req.method != reqwest::Method::GET && !self.permit_write { - return Err(ClientError::InternalError( + return Err(ClientError::Internal( "Non-GET request attempted in readonly mode".to_string(), )); } @@ -592,7 +626,7 @@ impl Client { } else { match serde_json::from_str(&rsp.text()?) { Ok(ac) => Ok(ac), - Err(e) => Err(ClientError::UrlError(url, e.to_string())), + Err(e) => Err(ClientError::JSONParse(url, e.to_string())), } }?; self.instance = Some(inst.clone()); @@ -669,12 +703,12 @@ impl Client { match serde_json::from_str(&rsp.text()?) { Ok(ac) => Ok(ac), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::JSONParse(url.clone(), e.to_string())) } } }?; if id.is_some_and(|id| ac.id != id) { - return Err(ClientError::UrlError( + return Err(ClientError::UrlConsistency( url, format!("request returned wrong account id {}", &ac.id), )); @@ -707,12 +741,12 @@ impl Client { match serde_json::from_str(&rsp.text()?) { Ok(poll) => Ok(poll), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::JSONParse(url.clone(), e.to_string())) } } }?; if poll.id != id { - return Err(ClientError::UrlError( + return Err(ClientError::UrlConsistency( url, format!("request returned wrong poll id {}", &poll.id), )); @@ -735,7 +769,7 @@ impl Client { match serde_json::from_str(&rsp.text()?) { Ok(ac) => Ok(ac), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::JSONParse(url.clone(), e.to_string())) } } }?; @@ -744,7 +778,7 @@ impl Client { return Ok(rel); } } - Err(ClientError::UrlError( + Err(ClientError::UrlConsistency( url, format!("request did not return expected account id {}", id), )) @@ -782,12 +816,12 @@ impl Client { match serde_json::from_str(&rsp.text()?) { Ok(st) => Ok(st), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::JSONParse(url.clone(), e.to_string())) } } }?; if st.id != id { - return Err(ClientError::UrlError( + return Err(ClientError::UrlConsistency( url, format!("request returned wrong status id {}", &st.id), )); @@ -826,12 +860,12 @@ impl Client { match serde_json::from_str(&rsp.text()?) { Ok(st) => Ok(st), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::JSONParse(url.clone(), e.to_string())) } } }?; if not.id != id { - return Err(ClientError::UrlError( + return Err(ClientError::UrlConsistency( url, format!("request returned wrong notification id {}", ¬.id), )); @@ -945,7 +979,7 @@ impl Client { let (url, rsp) = self.api_request(req)?; let rspstatus = rsp.status(); if !rspstatus.is_success() { - return Err(ClientError::UrlError(url, rspstatus.to_string())); + return Err(ClientError::from_response(&url, rsp)); } // Keep the Link: headers after we consume the response, for @@ -971,7 +1005,7 @@ impl Client { let sts: Vec = match serde_json::from_str(&body) { Ok(sts) => Ok(sts), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::JSONParse(url.clone(), e.to_string())) } }?; for st in &sts { @@ -983,7 +1017,7 @@ impl Client { let mut nots: Vec = match serde_json::from_str(&body) { Ok(nots) => Ok(nots), - Err(e) => Err(ClientError::UrlError( + Err(e) => Err(ClientError::JSONParse( url.clone(), e.to_string(), )), @@ -1022,7 +1056,7 @@ impl Client { let acs: Vec = match serde_json::from_str(&body) { Ok(acs) => Ok(acs), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::JSONParse(url.clone(), e.to_string())) } }?; for ac in &acs { @@ -1065,13 +1099,13 @@ impl Client { let linkhdr_str = match linkhdr.to_str() { Ok(s) => Ok(s), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::LinkParse(url.clone(), e.to_string())) } }?; let links = match parse_link_header::parse(linkhdr_str) { Ok(links) => Ok(links), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::LinkParse(url.clone(), e.to_string())) } }?; for (rel, link) in links { @@ -1145,7 +1179,7 @@ impl Client { match rsp.headers().get(reqwest::header::LOCATION) { None => { - return Err(ClientError::UrlError( + return Err(ClientError::UrlConsistency( url, "received redirection without a Location header" .to_owned(), @@ -1156,7 +1190,7 @@ impl Client { let sval = match std::str::from_utf8(bval) { Ok(s) => s, Err(_) => { - return Err(ClientError::UrlError( + return Err(ClientError::UrlConsistency( url, "HTTP redirect URL was invalid UTF-8" .to_owned(), @@ -1166,7 +1200,7 @@ impl Client { let newurl = match rsp.url().join(sval) { Ok(u) => u, Err(e) => { - return Err(ClientError::UrlError( + return Err(ClientError::UrlConsistency( url, format!("processing redirection: {}", e), )) @@ -1186,7 +1220,7 @@ impl Client { } }; if !ok { - return Err(ClientError::UrlError( + return Err(ClientError::UrlConsistency( url, format!("redirection to suspicious URL {}", sval), )); @@ -1205,7 +1239,7 @@ impl Client { let rspstatus = rsp.status(); if !rspstatus.is_success() { - return Err(ClientError::UrlError(url, rspstatus.to_string())); + return Err(ClientError::from_response(&url, rsp)); } let id = id.clone(); @@ -1303,11 +1337,11 @@ impl Client { )?; let rspstatus = rsp.status(); let ac: Account = if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { match serde_json::from_str(&rsp.text()?) { Ok(ac) => Ok(ac), - Err(e) => Err(ClientError::UrlError(url, e.to_string())), + Err(e) => Err(ClientError::JSONParse(url, e.to_string())), } }?; self.cache_account(&ac); @@ -1334,7 +1368,7 @@ impl Client { let (url, rsp) = self.api_request(req)?; let rspstatus = rsp.status(); if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { Ok(()) } @@ -1350,11 +1384,11 @@ impl Client { let rspstatus = rsp.status(); // Cache the returned status so as to update its faved/boosted flags let st: Status = if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { match serde_json::from_str(&rsp.text()?) { Ok(st) => Ok(st), - Err(e) => Err(ClientError::UrlError(url, e.to_string())), + Err(e) => Err(ClientError::JSONParse(url, e.to_string())), } }?; self.cache_status(&st); @@ -1387,11 +1421,11 @@ impl Client { .api_request(Req::get(&format!("api/v1/statuses/{id}/context")))?; let rspstatus = rsp.status(); let ctx: Context = if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { match serde_json::from_str(&rsp.text()?) { Ok(st) => Ok(st), - Err(e) => Err(ClientError::UrlError(url, e.to_string())), + Err(e) => Err(ClientError::JSONParse(url, e.to_string())), } }?; for st in &ctx.ancestors { @@ -1417,11 +1451,11 @@ impl Client { let rspstatus = rsp.status(); // Cache the returned poll so as to update its faved/boosted flags let poll: Poll = if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { match serde_json::from_str(&rsp.text()?) { Ok(poll) => Ok(poll), - Err(e) => Err(ClientError::UrlError(url, e.to_string())), + Err(e) => Err(ClientError::JSONParse(url, e.to_string())), } }?; self.cache_poll(&poll); @@ -1450,7 +1484,7 @@ impl Client { let (url, rsp) = self.api_request(req)?; let rspstatus = rsp.status(); if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { Ok(()) } @@ -1479,7 +1513,7 @@ impl Client { let (url, rsp) = self.api_request(req)?; let rspstatus = rsp.status(); if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { Ok(()) } @@ -1515,12 +1549,12 @@ impl Client { match serde_json::from_str(&rsp.text()?) { Ok(ac) => Ok(ac), Err(e) => { - Err(ClientError::UrlError(url.clone(), e.to_string())) + Err(ClientError::JSONParse(url.clone(), e.to_string())) } } }?; if ac.id != id { - return Err(ClientError::UrlError( + return Err(ClientError::UrlConsistency( url, format!("request returned wrong account id {}", &ac.id), )); @@ -1563,11 +1597,11 @@ impl Client { let (url, rsp) = self.api_request(req)?; let rspstatus = rsp.status(); if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { let app: Application = match serde_json::from_str(&rsp.text()?) { Ok(app) => Ok(app), - Err(e) => Err(ClientError::UrlError(url, e.to_string())), + Err(e) => Err(ClientError::JSONParse(url, e.to_string())), }?; Ok(app) } @@ -1580,14 +1614,14 @@ impl Client { ) -> Result { let client_id = match &app.client_id { Some(id) => Ok(id), - None => Err(ClientError::InternalError( + None => Err(ClientError::Internal( "registering application did not return a client id" .to_owned(), )), }?; let client_secret = match &app.client_secret { Some(id) => Ok(id), - None => Err(ClientError::InternalError( + None => Err(ClientError::Internal( "registering application did not return a client secret" .to_owned(), )), @@ -1613,11 +1647,11 @@ impl Client { let (url, rsp) = self.api_request(req)?; let rspstatus = rsp.status(); if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { let tok: Token = match serde_json::from_str(&rsp.text()?) { Ok(tok) => Ok(tok), - Err(e) => Err(ClientError::UrlError(url, e.to_string())), + Err(e) => Err(ClientError::JSONParse(url, e.to_string())), }?; Ok(tok) } @@ -1630,11 +1664,11 @@ impl Client { let (url, rsp) = self.api_request(req)?; let rspstatus = rsp.status(); if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { let app: Application = match serde_json::from_str(&rsp.text()?) { Ok(app) => Ok(app), - Err(e) => Err(ClientError::UrlError(url, e.to_string())), + Err(e) => Err(ClientError::JSONParse(url, e.to_string())), }?; Ok(app) } @@ -1646,7 +1680,7 @@ impl Client { ) -> Result { let client_id = match &app.client_id { Some(id) => Ok(id), - None => Err(ClientError::InternalError( + None => Err(ClientError::Internal( "registering application did not return a client id" .to_owned(), )), @@ -1677,11 +1711,11 @@ impl Client { let (url, rsp) = self.api_request(req)?; let rspstatus = rsp.status(); if !rspstatus.is_success() { - Err(ClientError::UrlError(url, rspstatus.to_string())) + Err(ClientError::from_response(&url, rsp)) } else { let tok: Token = match serde_json::from_str(&rsp.text()?) { Ok(tok) => Ok(tok), - Err(e) => Err(ClientError::UrlError(url, e.to_string())), + Err(e) => Err(ClientError::JSONParse(url, e.to_string())), }?; Ok(tok) } @@ -1693,10 +1727,9 @@ impl Client { ) -> Result<(), ClientError> { let url = match Url::parse(urlstr) { Ok(url) => Ok(url), - Err(e) => Err(ClientError::UrlParseError( - urlstr.to_owned(), - e.to_string(), - )), + Err(e) => { + Err(ClientError::UrlParse(urlstr.to_owned(), e.to_string())) + } }?; let req = self.client.request(reqwest::Method::GET, url).build()?; let (rsp, log) = execute_and_log_request(&self.client, req)?; @@ -1705,10 +1738,7 @@ impl Client { if rspstatus.is_redirection() || rspstatus.is_success() { Ok(()) } else { - Err(ClientError::UrlError( - urlstr.to_owned(), - rspstatus.to_string(), - )) + Err(ClientError::from_response(urlstr, rsp)) } } } diff --git a/src/options.rs b/src/options.rs index dde5a2e..b58c90d 100644 --- a/src/options.rs +++ b/src/options.rs @@ -32,7 +32,12 @@ impl YourOptionsMenu { let ac = client.account_by_id(&client.our_account_id())?; let source = match ac.source { Some(source) => source, - None => return Err(ClientError::NoAccountSource), + None => { + return Err(ClientError::Consistency( + "server did not send 'source' details for our account" + .to_owned(), + )) + } }; let title = FileHeader::new(ColouredString::general(