From 6eb83812e1ba7430cf9b4bdf13ec9cb56ac9682d Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 13 Apr 2022 22:20:46 +0530 Subject: [PATCH] Propagate the parse error for URLs (#19972) During config validation, we would omit the actual error. This makes debugging difficult. ```release-note NONE ``` --- config/database_test.go | 4 ++-- config/file_test.go | 6 +++--- model/config.go | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/config/database_test.go b/config/database_test.go index f6eaeaa289..e837a7f893 100644 --- a/config/database_test.go +++ b/config/database_test.go @@ -474,7 +474,7 @@ func TestDatabaseStoreSet(t *testing.T) { _, _, err = ds.Set(newCfg) if assert.Error(t, err) { - assert.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, ") + assert.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, parse \"invalid\": invalid URI for request") } assert.Equal(t, "", *ds.Get().ServiceSettings.SiteURL) @@ -842,7 +842,7 @@ func TestDatabaseStoreLoad(t *testing.T) { err = ds.Load() if assert.Error(t, err) { - assert.EqualError(t, err, "invalid config: Config.IsValid: model.config.is_valid.site_url.app_error, ") + assert.EqualError(t, err, "invalid config: Config.IsValid: model.config.is_valid.site_url.app_error, parse \"invalid\": invalid URI for request") } }) diff --git a/config/file_test.go b/config/file_test.go index 414c4b6497..3ff5cf12b4 100644 --- a/config/file_test.go +++ b/config/file_test.go @@ -495,7 +495,7 @@ func TestFileStoreSet(t *testing.T) { _, _, err := configStore.Set(newCfg) if assert.Error(t, err) { - assert.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, ") + assert.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, parse \"invalid\": invalid URI for request") } assert.Equal(t, "", *configStore.Get().ServiceSettings.SiteURL) @@ -819,7 +819,7 @@ func TestFileStoreLoad(t *testing.T) { err = fs.Load() if assert.Error(t, err) { - assert.EqualError(t, err, "invalid config: Config.IsValid: model.config.is_valid.site_url.app_error, ") + assert.EqualError(t, err, "invalid config: Config.IsValid: model.config.is_valid.site_url.app_error, parse \"invalid\": invalid URI for request") } }) @@ -833,7 +833,7 @@ func TestFileStoreLoad(t *testing.T) { newCfg := minimalConfig _, _, err := configStore.Set(newCfg) require.Error(t, err) - require.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, ") + require.EqualError(t, err, "new configuration is invalid: Config.IsValid: model.config.is_valid.site_url.app_error, parse \"invalid_url\": invalid URI for request") }) t.Run("fixes required", func(t *testing.T) { diff --git a/model/config.go b/model/config.go index cb7ba9dd8d..c3cf579c82 100644 --- a/model/config.go +++ b/model/config.go @@ -3567,13 +3567,13 @@ func (s *ServiceSettings) isValid() *AppError { if *s.SiteURL != "" { if _, err := url.ParseRequestURI(*s.SiteURL); err != nil { - return NewAppError("Config.IsValid", "model.config.is_valid.site_url.app_error", nil, "", http.StatusBadRequest) + return NewAppError("Config.IsValid", "model.config.is_valid.site_url.app_error", nil, err.Error(), http.StatusBadRequest) } } if *s.WebsocketURL != "" { if _, err := url.ParseRequestURI(*s.WebsocketURL); err != nil { - return NewAppError("Config.IsValid", "model.config.is_valid.websocket_url.app_error", nil, "", http.StatusBadRequest) + return NewAppError("Config.IsValid", "model.config.is_valid.websocket_url.app_error", nil, err.Error(), http.StatusBadRequest) } }