From 16c78f6a98b4543ba666e4f17e520141a50b17dd Mon Sep 17 00:00:00 2001 From: Sriram <153843+yesoreyeram@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:14:44 +0000 Subject: [PATCH] Postgres: Datasource health check error message improvements #96906 (#96990) * updated postgres health check error messages * addressed review comments / added tests --- .../sqleng/handler_checkhealth.go | 55 ++++++++++++++++- .../sqleng/handler_checkhealth_test.go | 60 +++++++++++++++++++ 2 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth_test.go diff --git a/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go b/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go index 0aef767f618..6dedac9bdf8 100644 --- a/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go +++ b/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go @@ -3,17 +3,22 @@ package sqleng import ( "context" "encoding/json" + "errors" + "fmt" + "net" + "strings" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/log" + "github.com/lib/pq" ) func (e *DataSourceHandler) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { err := e.Ping() if err != nil { logCheckHealthError(ctx, e.dsInfo, err, e.log) - if req.PluginContext.User.Role == "Admin" { - return &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: err.Error()}, nil + if strings.EqualFold(req.PluginContext.User.Role, "Admin") { + return ErrToHealthCheckResult(err) } errResponse := &backend.CheckHealthResult{ Status: backend.HealthStatusError, @@ -24,7 +29,51 @@ func (e *DataSourceHandler) CheckHealth(ctx context.Context, req *backend.CheckH return &backend.CheckHealthResult{Status: backend.HealthStatusOk, Message: "Database Connection OK"}, nil } -func logCheckHealthError(ctx context.Context, dsInfo DataSourceInfo, err error, logger log.Logger) { +// ErrToHealthCheckResult converts error into user friendly health check message +// This should be called with non nil error. If the err parameter is empty, we will send Internal Server Error +func ErrToHealthCheckResult(err error) (*backend.CheckHealthResult, error) { + if err == nil { + return &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: "Internal Server Error"}, nil + } + res := &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: err.Error()} + details := map[string]string{ + "verboseMessage": err.Error(), + "errorDetailsLink": "https://grafana.com/docs/grafana/latest/datasources/postgres", + } + var opErr *net.OpError + if errors.As(err, &opErr) { + res.Message = "Network error: Failed to connect to the server" + if opErr != nil && opErr.Err != nil { + res.Message += fmt.Sprintf(". Error message: %s", opErr.Err.Error()) + } + } + if errors.Is(err, pq.ErrSSLNotSupported) { + res.Message = "SSL error: Failed to connect to the server" + } + if strings.HasPrefix(err.Error(), "pq") { + res.Message = "Database error: Failed to connect to the postgres server" + if unwrappedErr := errors.Unwrap(err); unwrappedErr != nil { + details["verboseMessage"] = unwrappedErr.Error() + } + } + var pqErr *pq.Error + if errors.As(err, &pqErr) { + if pqErr != nil { + if pqErr.Code != "" { + res.Message += fmt.Sprintf(". Postgres error code: %s", pqErr.Code.Name()) + } + details["verboseMessage"] = pqErr.Message + } + } + detailBytes, marshalErr := json.Marshal(details) + if marshalErr != nil { + return res, nil + } + res.JSONDetails = detailBytes + return res, nil +} + +func logCheckHealthError(_ context.Context, dsInfo DataSourceInfo, err error, logger log.Logger) { configSummary := map[string]any{ "config_url_length": len(dsInfo.URL), "config_user_length": len(dsInfo.User), diff --git a/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth_test.go b/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth_test.go new file mode 100644 index 00000000000..b3f8736407d --- /dev/null +++ b/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth_test.go @@ -0,0 +1,60 @@ +package sqleng + +import ( + "errors" + "net" + "testing" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/lib/pq" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestErrToHealthCheckResult(t *testing.T) { + tests := []struct { + name string + err error + want *backend.CheckHealthResult + }{ + { + name: "without error", + want: &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: "Internal Server Error"}, + }, + { + name: "network error", + err: errors.Join(errors.New("foo"), &net.OpError{Op: "read", Net: "tcp", Err: errors.New("some op")}), + want: &backend.CheckHealthResult{ + Status: backend.HealthStatusError, + Message: "Network error: Failed to connect to the server. Error message: some op", + JSONDetails: []byte(`{"errorDetailsLink":"https://grafana.com/docs/grafana/latest/datasources/postgres","verboseMessage":"foo\nread tcp: some op"}`), + }, + }, + { + name: "db error", + err: errors.Join(errors.New("foo"), &pq.Error{Message: pq.ErrCouldNotDetectUsername.Error(), Code: pq.ErrorCode("28P01")}), + want: &backend.CheckHealthResult{ + Status: backend.HealthStatusError, + Message: "foo\npq: pq: Could not detect default username. Please provide one explicitly. Postgres error code: invalid_password", + JSONDetails: []byte(`{"errorDetailsLink":"https://grafana.com/docs/grafana/latest/datasources/postgres","verboseMessage":"pq: Could not detect default username. Please provide one explicitly"}`), + }, + }, + { + name: "regular error", + err: errors.New("internal server error"), + want: &backend.CheckHealthResult{ + Status: backend.HealthStatusError, + Message: "internal server error", + JSONDetails: []byte(`{"errorDetailsLink":"https://grafana.com/docs/grafana/latest/datasources/postgres","verboseMessage":"internal server error"}`), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ErrToHealthCheckResult(tt.err) + require.Nil(t, err) + assert.Equal(t, string(tt.want.JSONDetails), string(got.JSONDetails)) + require.Equal(t, tt.want, got) + }) + } +}