From a240397e865f762b6fd24e4483e3211641cc119f Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 4 Feb 2025 14:16:52 +0900 Subject: [PATCH 1/5] fix: improve function config parse error message --- pkg/config/config.go | 24 ++++++++++++++- pkg/config/config_test.go | 64 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 5d7616575f..b01b59c4eb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -396,6 +396,28 @@ func (c *config) loadFromFile(filename string, fsys fs.FS) error { return c.loadFromReader(v, f) } +// Used to improve config parsing errors into a more contextual and actionable +// error message +func wrapTomlError(err error) error { + if err == nil { + return nil + } + // Check for array-style functions config and unknown functions fields errors error + if strings.Contains(err.Error(), "'functions[") && strings.Contains(err.Error(), "expected a map") || + strings.Contains(err.Error(), "Unknown config field: [functions") { + return errors.Errorf(`Invalid functions config format. Functions should be configured as: + +[functions.] +field = value + +Example: +[functions.hello] +verify_jwt = true +`) + } + return err +} + func (c *config) loadFromReader(v *viper.Viper, r io.Reader) error { if err := v.MergeConfig(r); err != nil { return errors.Errorf("failed to merge config: %w", err) @@ -430,7 +452,7 @@ func (c *config) loadFromReader(v *viper.Viper, r io.Reader) error { dc.ZeroFields = true dc.DecodeHook = c.newDecodeHook(LoadEnvHook) }); err != nil { - return errors.Errorf("failed to parse config: %w", err) + return errors.Errorf("failed to parse config: %w", wrapTomlError(err)) } return nil } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index e13e67e59a..d483649d13 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -431,3 +431,67 @@ func TestLoadFunctionImportMap(t *testing.T) { assert.Equal(t, "supabase/custom_import_map.json", config.Functions["hello"].ImportMap) }) } + +func TestLoadFunctionErrorMessageParsing(t *testing.T) { + t.Run("returns error for array-style function config", func(t *testing.T) { + config := NewConfig() + fsys := fs.MapFS{ + "supabase/config.toml": &fs.MapFile{Data: []byte(` + project_id = "bvikqvbczudanvggcord" + [[functions]] + name = "hello" + verify_jwt = true + `)}, + } + // Run test + err := config.Load("", fsys) + // Check error contains both decode errors + assert.Error(t, err) + assert.Contains(t, err.Error(), "Invalid functions config format. Functions should be configured as:\n\n[functions.]\nfield = value\n\nExample:\n[functions.hello]\nverify_jwt = true\n") + }) + t.Run("returns error with function slug for invalid non-existent field", func(t *testing.T) { + config := NewConfig() + fsys := fs.MapFS{ + "supabase/config.toml": &fs.MapFile{Data: []byte(` + project_id = "bvikqvbczudanvggcord" + [functions.hello] + unknown_field = true + `)}, + } + // Run test + err := config.Load("", fsys) + // Check error contains both decode errors + assert.Error(t, err) + assert.Contains(t, err.Error(), "'functions[hello]' has invalid keys: unknown_field") + }) + t.Run("returns error with function slug for invalid field value", func(t *testing.T) { + config := NewConfig() + fsys := fs.MapFS{ + "supabase/config.toml": &fs.MapFile{Data: []byte(` + project_id = "bvikqvbczudanvggcord" + [functions.hello] + verify_jwt = "not-a-bool" + `)}, + } + // Run test + err := config.Load("", fsys) + // Check error contains both decode errors + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot parse 'functions[hello].verify_jwt' as bool: strconv.ParseBool: parsing \"not-a-bool\"") + }) + t.Run("returns error for unkown function fields", func(t *testing.T) { + config := NewConfig() + fsys := fs.MapFS{ + "supabase/config.toml": &fs.MapFile{Data: []byte(` + project_id = "bvikqvbczudanvggcord" + [functions] + name = "hello" + verify_jwt = true + `)}, + } + // Run test + err := config.Load("", fsys) + assert.Error(t, err) + assert.Contains(t, err.Error(), "Invalid functions config format. Functions should be configured as:\n\n[functions.]\nfield = value\n\nExample:\n[functions.hello]\nverify_jwt = true\n") + }) +} From 42200178ad7a893bba18566b81c8de92f8e9fd39 Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 4 Feb 2025 15:32:13 +0900 Subject: [PATCH 2/5] chore: fix lint --- pkg/config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index d483649d13..c8629c4c7a 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -479,7 +479,7 @@ func TestLoadFunctionErrorMessageParsing(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "cannot parse 'functions[hello].verify_jwt' as bool: strconv.ParseBool: parsing \"not-a-bool\"") }) - t.Run("returns error for unkown function fields", func(t *testing.T) { + t.Run("returns error for unknown function fields", func(t *testing.T) { config := NewConfig() fsys := fs.MapFS{ "supabase/config.toml": &fs.MapFile{Data: []byte(` From c256ee3697efb3d38e67668059e12322a8f49c6e Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 4 Feb 2025 17:19:57 +0900 Subject: [PATCH 3/5] fix: apply PR comment using decode hooks --- pkg/config/config.go | 41 +-------------------- pkg/config/decode_hooks.go | 75 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 39 deletions(-) create mode 100644 pkg/config/decode_hooks.go diff --git a/pkg/config/config.go b/pkg/config/config.go index b01b59c4eb..91df55066d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -16,7 +16,6 @@ import ( "os" "path" "path/filepath" - "reflect" "regexp" "sort" "strconv" @@ -352,7 +351,6 @@ var ( initConfigTemplate = template.Must(template.New("initConfig").Parse(initConfigEmbed)) invalidProjectId = regexp.MustCompile("[^a-zA-Z0-9_.-]+") - envPattern = regexp.MustCompile(`^env\((.*)\)$`) refPattern = regexp.MustCompile(`^[a-z]{20}$`) ) @@ -396,28 +394,6 @@ func (c *config) loadFromFile(filename string, fsys fs.FS) error { return c.loadFromReader(v, f) } -// Used to improve config parsing errors into a more contextual and actionable -// error message -func wrapTomlError(err error) error { - if err == nil { - return nil - } - // Check for array-style functions config and unknown functions fields errors error - if strings.Contains(err.Error(), "'functions[") && strings.Contains(err.Error(), "expected a map") || - strings.Contains(err.Error(), "Unknown config field: [functions") { - return errors.Errorf(`Invalid functions config format. Functions should be configured as: - -[functions.] -field = value - -Example: -[functions.hello] -verify_jwt = true -`) - } - return err -} - func (c *config) loadFromReader(v *viper.Viper, r io.Reader) error { if err := v.MergeConfig(r); err != nil { return errors.Errorf("failed to merge config: %w", err) @@ -450,9 +426,9 @@ func (c *config) loadFromReader(v *viper.Viper, r io.Reader) error { dc.TagName = "toml" dc.Squash = true dc.ZeroFields = true - dc.DecodeHook = c.newDecodeHook(LoadEnvHook) + dc.DecodeHook = c.newDecodeHook(LoadEnvHook, ValidateFunctionsHookFunc) }); err != nil { - return errors.Errorf("failed to parse config: %w", wrapTomlError(err)) + return errors.Errorf("failed to parse config: %w", err) } return nil } @@ -798,19 +774,6 @@ func assertEnvLoaded(s string) error { return nil } -func LoadEnvHook(f reflect.Kind, t reflect.Kind, data interface{}) (interface{}, error) { - if f != reflect.String { - return data, nil - } - value := data.(string) - if matches := envPattern.FindStringSubmatch(value); len(matches) > 1 { - if env := os.Getenv(matches[1]); len(env) > 0 { - value = env - } - } - return value, nil -} - func truncateText(text string, maxLen int) string { if len(text) > maxLen { return text[:maxLen] diff --git a/pkg/config/decode_hooks.go b/pkg/config/decode_hooks.go new file mode 100644 index 0000000000..07203eddb3 --- /dev/null +++ b/pkg/config/decode_hooks.go @@ -0,0 +1,75 @@ +package config + +import ( + "os" + "reflect" + "regexp" + + "github.com/go-errors/errors" +) + +var envPattern = regexp.MustCompile(`^env\((.*)\)$`) + +// LoadEnvHook is a mapstructure decode hook that loads environment variables +// from strings formatted as env(VAR_NAME). +func LoadEnvHook(f reflect.Kind, t reflect.Kind, data interface{}) (interface{}, error) { + if f != reflect.String { + return data, nil + } + value := data.(string) + if matches := envPattern.FindStringSubmatch(value); len(matches) > 1 { + if env := os.Getenv(matches[1]); len(env) > 0 { + value = env + } + } + return value, nil +} + +// ValidateFunctionsHookFunc is a mapstructure decode hook that validates the functions config format. +func ValidateFunctionsHookFunc(f reflect.Type, t reflect.Type, data interface{}) (interface{}, error) { + // Only handle FunctionConfig type + if t != reflect.TypeOf(FunctionConfig{}) { + return data, nil + } + + // Check if source is not a map + if f.Kind() != reflect.Map { + return nil, errors.New(`Invalid functions config format. Functions should be configured as: + +[functions.] +field = value + +Example: +[functions.hello] +verify_jwt = true +`) + } + + // Check if any fields are defined directly under [functions] instead of [functions.] + if m, ok := data.(map[string]interface{}); ok { + for _, value := range m { + // Skip nil values and empty function configs as they're valid + if value == nil { + continue + } + // If it's already a function type, it's valid + if _, isFunction := value.(function); isFunction { + continue + } + // If the value is not a map, it means it's defined directly under [functions] + if _, isMap := value.(map[string]interface{}); !isMap { + return nil, errors.New(`Invalid functions config format. Functions should be configured as: + +[functions.] +field = value + +Example: +[functions.hello] +verify_jwt = true +`) + } + } + } + + return data, nil +} From aeb66697ccab2f864930058517cb31806465e7d1 Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 4 Feb 2025 17:43:12 +0900 Subject: [PATCH 4/5] chore: apply PR comments --- pkg/config/config.go | 2 +- pkg/config/decode_hooks.go | 33 +++++++++++++-------------------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 91df55066d..eff7a06cd0 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -426,7 +426,7 @@ func (c *config) loadFromReader(v *viper.Viper, r io.Reader) error { dc.TagName = "toml" dc.Squash = true dc.ZeroFields = true - dc.DecodeHook = c.newDecodeHook(LoadEnvHook, ValidateFunctionsHookFunc) + dc.DecodeHook = c.newDecodeHook(LoadEnvHook, ValidateFunctionsHook) }); err != nil { return errors.Errorf("failed to parse config: %w", err) } diff --git a/pkg/config/decode_hooks.go b/pkg/config/decode_hooks.go index 07203eddb3..4b667e0741 100644 --- a/pkg/config/decode_hooks.go +++ b/pkg/config/decode_hooks.go @@ -10,6 +10,15 @@ import ( var envPattern = regexp.MustCompile(`^env\((.*)\)$`) +const invalidFunctionsConfigFormat = `Invalid functions config format. Functions should be configured as: + +[functions.] +field = value + +Example: +[functions.hello] +verify_jwt = true` + // LoadEnvHook is a mapstructure decode hook that loads environment variables // from strings formatted as env(VAR_NAME). func LoadEnvHook(f reflect.Kind, t reflect.Kind, data interface{}) (interface{}, error) { @@ -25,8 +34,8 @@ func LoadEnvHook(f reflect.Kind, t reflect.Kind, data interface{}) (interface{}, return value, nil } -// ValidateFunctionsHookFunc is a mapstructure decode hook that validates the functions config format. -func ValidateFunctionsHookFunc(f reflect.Type, t reflect.Type, data interface{}) (interface{}, error) { +// ValidateFunctionsHook is a mapstructure decode hook that validates the functions config format. +func ValidateFunctionsHook(f reflect.Type, t reflect.Type, data interface{}) (interface{}, error) { // Only handle FunctionConfig type if t != reflect.TypeOf(FunctionConfig{}) { return data, nil @@ -34,15 +43,7 @@ func ValidateFunctionsHookFunc(f reflect.Type, t reflect.Type, data interface{}) // Check if source is not a map if f.Kind() != reflect.Map { - return nil, errors.New(`Invalid functions config format. Functions should be configured as: - -[functions.] -field = value - -Example: -[functions.hello] -verify_jwt = true -`) + return nil, errors.New(invalidFunctionsConfigFormat) } // Check if any fields are defined directly under [functions] instead of [functions.] @@ -58,15 +59,7 @@ verify_jwt = true } // If the value is not a map, it means it's defined directly under [functions] if _, isMap := value.(map[string]interface{}); !isMap { - return nil, errors.New(`Invalid functions config format. Functions should be configured as: - -[functions.] -field = value - -Example: -[functions.hello] -verify_jwt = true -`) + return nil, errors.New(invalidFunctionsConfigFormat) } } } From ef9074e84e10719ced69249632c920f89f33bb3b Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 4 Feb 2025 17:57:21 +0900 Subject: [PATCH 5/5] chore: fix tests re-use error message --- pkg/config/config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index c8629c4c7a..51ec9db9db 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -447,7 +447,7 @@ func TestLoadFunctionErrorMessageParsing(t *testing.T) { err := config.Load("", fsys) // Check error contains both decode errors assert.Error(t, err) - assert.Contains(t, err.Error(), "Invalid functions config format. Functions should be configured as:\n\n[functions.]\nfield = value\n\nExample:\n[functions.hello]\nverify_jwt = true\n") + assert.Contains(t, err.Error(), invalidFunctionsConfigFormat) }) t.Run("returns error with function slug for invalid non-existent field", func(t *testing.T) { config := NewConfig() @@ -492,6 +492,6 @@ func TestLoadFunctionErrorMessageParsing(t *testing.T) { // Run test err := config.Load("", fsys) assert.Error(t, err) - assert.Contains(t, err.Error(), "Invalid functions config format. Functions should be configured as:\n\n[functions.]\nfield = value\n\nExample:\n[functions.hello]\nverify_jwt = true\n") + assert.Contains(t, err.Error(), invalidFunctionsConfigFormat) }) }