refactor: fine tune config connection manner and errmsgs

README.md

@@ -37,6 +37,7 @@ default_connection = "default"
 
 [connections.default]
 endpoint = "https://<cell>.<provider>.scopedb.cloud"
+auth = "api_key"
 api_key = "your-api-key"
 headers = ["X-Tenant: acme"]
 ```

scopeql/src/client/connection.rs

@@ -46,9 +46,7 @@ impl Client {
     ) -> Result<Self, Error> {
         let authorization = match api_key.filter(|api_key| !api_key.is_empty()) {
             Some(api_key) => Some(HeaderValue::from_str(&format!("Bearer {api_key}")).map_err(
-                |err| {
-                    Error::new("failed to build authorization header".to_string()).set_source(err)
-                },
+                |err| Error::new("failed to build authorization header").set_source(err),
             )?),
             None => None,
         };
@@ -60,7 +58,7 @@ impl Client {
                 authorization,
                 extra_headers,
             }),
-            Err(err) => Err(Error::new("failed to parse endpoint".to_string()).set_source(err)),
+            Err(err) => Err(Error::new("failed to parse endpoint").set_source(err)),
         }
     }
 
@@ -144,7 +142,7 @@ impl Client {
     fn make_url(&self, path: &str) -> Result<Url, Error> {
         self.endpoint
             .join(path)
-            .map_err(|err| Error::new("failed to construct URL".to_string()).set_source(err))
+            .map_err(|err| Error::new("failed to construct URL").set_source(err))
     }
 
     fn request_headers(&self) -> HeaderMap {

scopeql/src/client/protocol.rs

@@ -32,7 +32,7 @@ pub enum Response<T> {
 
 impl<T: DeserializeOwned> Response<T> {
     pub async fn from_http_response(r: reqwest::Response) -> Result<Self, Error> {
-        let make_error = |err| Error::new("failed to make response".to_string()).set_source(err);
+        let make_error = |err| Error::new("failed to make response").set_source(err);
 
         let code = r.status();
         if code.is_success() {

scopeql/src/config.rs

@@ -26,6 +26,8 @@ use serde::Serialize;
 use serde::de::IntoDeserializer;
 use toml_edit::DocumentMut;
 
+use crate::Error;
+
 pub const DEFAULT_URL: &str = "http://127.0.0.1:6543";
 
 fn candidate_config_paths() -> Vec<PathBuf> {
@@ -185,6 +187,15 @@ impl Config {
     }
 }
 
+fn deserialize_toml(path: &Path, doc: DocumentMut) -> Result<Config, Error> {
+    Config::deserialize(doc.into_deserializer()).map_err(|err| {
+        Error::new(format!(
+            "failed to deserialize config on {}: {err}",
+            path.display()
+        ))
+    })
+}
+
 impl Default for Config {
     fn default() -> Self {
         Self {
@@ -246,20 +257,28 @@ impl ConnectionAuthSpec {
 }
 
 pub(crate) fn get_connections(name: Option<&str>) {
-    let (_path, doc) = load_document();
-    let config =
-        Config::deserialize(doc.into_deserializer()).expect("failed to deserialize config");
+    do_get_connections(name).unwrap_or_else(|err| {
+        eprintln!("Failed to get connections: {err}");
+        std::process::exit(1);
+    });
+}
+
+fn do_get_connections(name: Option<&str>) -> Result<(), Error> {
+    let (path, doc) = load_document()?;
+    let config = deserialize_toml(&path, doc)?;
 
     if config.connections.is_empty() {
         println!("No connections configured.");
-        return;
+        return Ok(());
     }
 
     let connections = if let Some(name) = name {
-        let conn = config.connections.get(name).unwrap_or_else(|| {
-            panic!("Connection '{name}' not found in config.");
-        });
-        vec![(name, conn)]
+        match config.connections.get(name) {
+            Some(conn) => vec![(name, conn)],
+            None => {
+                return Err(Error::new(format!("Connection '{name}' not found.")));
+            }
+        }
     } else {
         config
             .connections
@@ -293,33 +312,47 @@ pub(crate) fn get_connections(name: Option<&str>) {
     }
 
     println!("{table}");
+    Ok(())
 }
 
 pub(crate) fn use_connection(name: &str) {
-    let (path, mut doc) = load_document();
+    do_use_connection(name).unwrap_or_else(|err| {
+        eprintln!("Failed to switch connection: {err}");
+        std::process::exit(1);
+    });
+}
 
-    let config =
-        Config::deserialize(doc.clone().into_deserializer()).expect("failed to deserialize config");
+fn do_use_connection(name: &str) -> Result<(), Error> {
+    let (path, mut doc) = load_document()?;
+    let config = deserialize_toml(&path, doc.clone())?;
 
     if !config.connections.contains_key(name) {
-        eprintln!("Connection '{name}' not found.");
-        std::process::exit(1);
+        return Err(Error::new(format!("Connection '{name}' not found.")));
     }
 
     set_toml_path(&mut doc, &["default_connection"], toml_edit::value(name));
 
-    std::fs::write(&path, doc.to_string()).unwrap_or_else(|err| {
-        panic!("failed to write config file {}: {err}", path.display());
-    });
+    std::fs::write(&path, doc.to_string()).map_err(|err| {
+        Error::new(format!(
+            "failed to write config file {}: {err}",
+            path.display()
+        ))
+    })?;
 
     println!("Switched to connection '{name}'");
+    Ok(())
 }
 
 pub(crate) fn set_connection(name: String) {
-    let (path, mut doc) = load_document();
+    do_set_connection(name).unwrap_or_else(|err| {
+        eprintln!("Failed to set connection: {err}");
+        std::process::exit(1);
+    });
+}
 
-    let mut config =
-        Config::deserialize(doc.clone().into_deserializer()).expect("failed to deserialize config");
+fn do_set_connection(name: String) -> Result<(), Error> {
+    let (path, mut doc) = load_document()?;
+    let mut config = deserialize_toml(&path, doc.clone())?;
 
     let conn = if let Some(conn) = config.connections.get_mut(&name) {
         if Confirm::new()
@@ -355,11 +388,15 @@ pub(crate) fn set_connection(name: String) {
 
     write_connection(&mut doc, &name, &conn);
 
-    std::fs::write(&path, doc.to_string()).unwrap_or_else(|err| {
-        panic!("failed to write config file {}: {err}", path.display());
-    });
+    std::fs::write(&path, doc.to_string()).map_err(|err| {
+        Error::new(format!(
+            "failed to write config file {}: {err}",
+            path.display()
+        ))
+    })?;
 
     println!("Set connection '{name}' in {}", path.display());
+    Ok(())
 }
 
 fn prompt_existing_auth(current: &ConnectionAuthSpec) -> ConnectionAuthSpec {
@@ -424,16 +461,25 @@ fn prompt_api_key() -> String {
 }
 
 fn write_connection(doc: &mut DocumentMut, name: &str, conn: &ConnectionSpec) {
-    if !doc["connections"].is_table() {
-        doc["connections"] = toml_edit::Item::Table(toml_edit::Table::new());
-    }
-    if doc["connections"].get(name).is_none() {
-        doc["connections"][name] = toml_edit::Item::Table(toml_edit::Table::new());
+    let connections = doc
+        .as_table_mut()
+        .entry("connections")
+        .or_insert(toml_edit::Item::Table({
+            let mut t = toml_edit::Table::new();
+            t.set_implicit(true);
+            t
+        }));
+    if !connections.is_table() {
+        let mut t = toml_edit::Table::new();
+        t.set_implicit(true);
+        *connections = toml_edit::Item::Table(t);
     }
+    let connections = connections.as_table_mut().unwrap();
 
-    let table = doc["connections"][name]
-        .as_table_mut()
-        .expect("connection should be a TOML table");
+    let table = connections
+        .entry(name)
+        .or_insert(toml_edit::Item::Table(toml_edit::Table::new()));
+    let table = table.as_table_mut().unwrap();
 
     table["endpoint"] = toml_edit::value(&conn.endpoint);
     if conn.headers.is_empty() {
@@ -446,7 +492,7 @@ fn write_connection(doc: &mut DocumentMut, name: &str, conn: &ConnectionSpec) {
         table["headers"] = toml_edit::value(headers);
     }
 
-    match &conn.auth {
+    match conn.auth() {
         ConnectionAuthSpec::Direct => {
             table["auth"] = toml_edit::value("direct");
             table.remove("api_key");
@@ -459,52 +505,68 @@ fn write_connection(doc: &mut DocumentMut, name: &str, conn: &ConnectionSpec) {
 }
 
 pub(crate) fn delete_connection(name: &str) {
-    let (path, mut doc) = load_document();
+    do_delete_connection(name).unwrap_or_else(|err| {
+        eprintln!("Failed to delete connection: {err}");
+        std::process::exit(1);
+    });
+}
 
-    let config =
-        Config::deserialize(doc.clone().into_deserializer()).expect("failed to deserialize config");
+fn do_delete_connection(name: &str) -> Result<(), Error> {
+    let (path, mut doc) = load_document()?;
+    let config = deserialize_toml(&path, doc.clone())?;
 
     if !config.connections.contains_key(name) {
-        eprintln!("Connection '{name}' not found.");
-        std::process::exit(1);
+        return Err(Error::new(format!("Connection '{name}' not found.")));
     }
 
     if config.default_connection == name {
         let Some(other) = config.connections.keys().find(|k| *k != name).cloned() else {
-            eprintln!("Cannot delete the only connection.");
-            std::process::exit(1);
+            return Err(Error::new("Cannot delete the only connection."));
         };
         set_toml_path(&mut doc, &["default_connection"], toml_edit::value(&other));
         println!("Switched to connection '{other}'");
     }
 
     doc["connections"].as_table_mut().unwrap().remove(name);
 
-    std::fs::write(&path, doc.to_string()).unwrap_or_else(|err| {
-        panic!("failed to write config file {}: {err}", path.display());
-    });
+    std::fs::write(&path, doc.to_string()).map_err(|err| {
+        Error::new(format!(
+            "failed to write config file {}: {err}",
+            path.display()
+        ))
+    })?;
 
     println!("Deleted connection '{name}' from {}", path.display());
+    Ok(())
 }
 
-fn load_document() -> (PathBuf, DocumentMut) {
-    let path = candidate_config_paths()
-        .into_iter()
+fn load_document() -> Result<(PathBuf, DocumentMut), Error> {
+    let candidates = candidate_config_paths();
+    let path = candidates
+        .iter()
         .find(|path| path.exists())
-        .unwrap_or_else(|| {
-            eprintln!("No config file found. Run `scopeql config add-connection` to create one.");
-            std::process::exit(1);
-        });
-
-    let content = std::fs::read_to_string(&path).unwrap_or_else(|err| {
-        panic!("failed to read config file {}: {err}", path.display());
-    });
-
-    let doc = DocumentMut::from_str(&content).unwrap_or_else(|err| {
-        panic!("failed to parse config file {}: {err}", path.display());
-    });
-
-    (path, doc)
+        .ok_or_else(|| {
+            Error::new(
+                "no config file found; run `scopeql config set-connection <name>` to create one",
+            )
+        })?;
+
+    let content = std::fs::read_to_string(path).map_err(|err| {
+        Error::new(format!(
+            "failed to read config file {}: {err}",
+            path.display()
+        ))
+    })?;
+
+    let doc = DocumentMut::from_str(&content).map_err(|err| {
+        Error::new(format!(
+            "failed to parse config file {}: {err}",
+            path.display()
+        ))
+    })?;
+
+    log::info!("loaded config from {}", path.display());
+    Ok((path.clone(), doc))
 }
 
 #[cfg(test)]

scopeql/src/main.rs

@@ -134,9 +134,9 @@ struct Error {
 }
 
 impl Error {
-    fn new(message: String) -> Self {
+    fn new(message: impl Into<String>) -> Self {
         Self {
-            message,
+            message: message.into(),
             source: None,
         }
     }

scopeql/src/output.rs

@@ -61,7 +61,7 @@ fn format_table(
 
     let rows = result_set
         .into_values()
-        .or_raise(|| Error::new("failed to convert result rows".to_string()))?;
+        .or_raise(|| Error::new("failed to convert result rows"))?;
 
     const TABLE_STYLE_PRESET: &str = "||--+-++|    ++++++";
     let mut table = comfy_table::Table::new();
@@ -124,7 +124,7 @@ fn format_json(result_set: ResultSet) -> Result<String, Error> {
         .collect::<Vec<_>>();
     let rows = result_set
         .into_values()
-        .or_raise(|| Error::new("failed to convert result rows".to_string()))?;
+        .or_raise(|| Error::new("failed to convert result rows"))?;
 
     let json_rows = rows
         .into_iter()
@@ -150,7 +150,7 @@ fn format_csv(result_set: ResultSet) -> Result<String, Error> {
         .collect::<Vec<_>>();
     let rows = result_set
         .into_values()
-        .or_raise(|| Error::new("failed to convert result rows".to_string()))?;
+        .or_raise(|| Error::new("failed to convert result rows"))?;
 
     let mut output = String::new();
     for (index, field) in fields.iter().enumerate() {
@@ -184,7 +184,7 @@ fn format_jsonl(result_set: ResultSet) -> Result<String, Error> {
         .collect::<Vec<_>>();
     let rows = result_set
         .into_values()
-        .or_raise(|| Error::new("failed to convert result rows".to_string()))?;
+        .or_raise(|| Error::new("failed to convert result rows"))?;
 
     let mut output = String::new();
     for row in rows {