Merge pull request #108 from supabase/fix/improve-error-msgs

improve error msgs

Author: imor

cli/Cargo.lock

@@ -11,6 +11,7 @@ anyhow = "1.0.69"
 clap = { version = "4.1.6", features = ["derive"] }
 dirs = "5.0.1"
 postgrest = "1.5.0"
+regex = "1.9"
 reqwest = { version = "0.11.14", features = ["json", "native-tls-vendored"] }
 rpassword = "7.2.0"
 serde = { version = "1.0.156", features = ["derive"] }

cli/Cargo.toml

@@ -24,6 +24,10 @@ pub async fn publish(
     let request = create_publish_package_request(&payload);
     client.publish_package(&jwt, &request).await?;
 
+    if payload.install_files.is_empty() {
+        return Err(anyhow::anyhow!("No valid script file (.sql) found."));
+    }
+
     let mut num_published = 0;
 
     for install_file in &payload.install_files {

cli/src/commands/publish.rs

@@ -146,7 +146,7 @@ impl Payload {
 
         if !util::is_valid_extension_name(&extension_name) {
             return Err(anyhow::anyhow!(
-                "invalid extension name detected {}",
+                "Invalid extension name detected: {}. It must begin with an alphabet, contain only alphanumeric characters or `_` and should be between 2 and 32 characters long.",
                 extension_name
             ));
         }
@@ -166,31 +166,46 @@ impl Payload {
             match &parts[..] {
                 [file_ext_name, ver] => {
                     // Make sure the file's extension name matches the control file
-                    if file_ext_name == &extension_name && util::is_valid_version(ver) {
-                        let ifile = InstallFile {
-                            filename: file_name.to_string(),
-                            version: ver.to_string(),
-                            body: fs::read_to_string(&path)
-                                .context(format!("Failed to read file {}", &file_name))?,
-                        };
-                        install_files.push(ifile);
+                    if file_ext_name != &extension_name {
+                        println!("Warning: file `{file_name}` will be skipped because its extension name(`{file_ext_name}`) doesn't match `{extension_name}`");
+                        continue;
+                    }
+                    if !util::is_valid_version(ver) {
+                        println!("Warning: file `{file_name}` will be skipped because its version (`{ver}`) is invalid. It should be have the format `major.minor.patch`.");
+                        continue;
                     }
+
+                    let ifile = InstallFile {
+                        filename: file_name.to_string(),
+                        version: ver.to_string(),
+                        body: fs::read_to_string(&path)
+                            .context(format!("Failed to read file {}", &file_name))?,
+                    };
+                    install_files.push(ifile);
                 }
                 [file_ext_name, from_ver, to_ver] => {
                     // Make sure the file's extension name matches the control file
-                    if file_ext_name == &extension_name
-                        && util::is_valid_version(from_ver)
-                        && util::is_valid_version(to_ver)
-                    {
-                        let ufile = UpgradeFile {
-                            filename: file_name.to_string(),
-                            from_version: from_ver.to_string(),
-                            to_version: to_ver.to_string(),
-                            body: fs::read_to_string(&path)
-                                .context(format!("Failed to read file {}", &file_name))?,
-                        };
-                        upgrade_files.push(ufile);
+                    if file_ext_name != &extension_name {
+                        println!("Warning: file `{file_name}` will be skipped because its extension name(`{file_ext_name}`) doesn't match `{extension_name}`");
+                        continue;
                     }
+                    if !util::is_valid_version(from_ver) {
+                        println!("Warning: file `{file_name}` will be skipped because its from version(`{from_ver}`) is invalid. It should be have the format `major.minor.patch`.");
+                        continue;
+                    }
+                    if !util::is_valid_version(to_ver) {
+                        println!("Warning: file `{file_name}` will be skipped because its from version(`{to_ver}`) is invalid. It should be have the format `major.minor.patch`.");
+                        continue;
+                    }
+
+                    let ufile = UpgradeFile {
+                        filename: file_name.to_string(),
+                        from_version: from_ver.to_string(),
+                        to_version: to_ver.to_string(),
+                        body: fs::read_to_string(&path)
+                            .context(format!("Failed to read file {}", &file_name))?,
+                    };
+                    upgrade_files.push(ufile);
                 }
                 _ => (),
             }
@@ -264,7 +279,9 @@ impl ControlFileRef {
                 return self.read_control_line_value(line);
             }
         }
-        Err(anyhow::anyhow!("default version is required"))
+        Err(anyhow::anyhow!(
+            "`default_version` in control file is required"
+        ))
     }
 
     fn read_control_line_value(&self, line: &str) -> anyhow::Result<String> {

cli/src/models.rs

@@ -1,4 +1,5 @@
 use anyhow::Context;
+use regex::Regex;
 use sqlx::postgres::PgConnection;
 use sqlx::Connection;
 use std::fs::{File, OpenOptions};
@@ -10,11 +11,28 @@ pub async fn get_connection(connection_str: &str) -> anyhow::Result<PgConnection
         .context("Failed to establish PostgreSQL connection")
 }
 
-pub fn is_valid_extension_name(_name: &str) -> bool {
-    true
+pub fn is_valid_extension_name(name: &str) -> bool {
+    let name_regex = Regex::new(r"^[A-z][A-z0-9\_]{2,32}$").expect("regex is valid");
+    name_regex.is_match(name)
 }
 
-pub fn is_valid_version(_version: &str) -> bool {
+pub fn is_valid_version(version: &str) -> bool {
+    let nums: Vec<&str> = version.split('.').collect();
+    if nums.len() != 3 {
+        println!("1");
+        return false;
+    }
+
+    for num_str in nums {
+        let num: i16 = match num_str.parse() {
+            Ok(n) => n,
+            _ => return false,
+        };
+        if num < 0 {
+            return false;
+        }
+    }
+
     true
 }