chiark / gitweb /
Reorganise the ClientError enum.
authorSimon Tatham <anakin@pobox.com>
Thu, 25 Jan 2024 18:09:41 +0000 (18:09 +0000)
committerSimon Tatham <anakin@pobox.com>
Thu, 25 Jan 2024 18:09:41 +0000 (18:09 +0000)
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.

src/client.rs
src/options.rs

index 270add3924f919bb0998e16e890f660a0b6b58d5..88afc894ceb4664911006522adab9ab4ffe6ba2e 100644 (file)
@@ -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<reqwest::Error> 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<reqwest::Error> 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::<ServerError>(&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 {}", &not.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<Status> = 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<Notification> =
                     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<Account> = 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<Token, ClientError> {
         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<String, ClientError> {
         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))
         }
     }
 }
index dde5a2ecbb1b72a63c0ca2bcae6042bf2033f964..b58c90d9311d4122e135c960a094251c43499b6b 100644 (file)
@@ -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(