From 7b8c7b623c91b9172a0ea658764ae6e3669b020f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Farkas?= Date: Wed, 7 Feb 2024 08:39:24 +0100 Subject: [PATCH] sql: remove the usage of "errutil" (#81888) --- pkg/tsdb/mssql/mssql.go | 2 +- pkg/tsdb/mssql/mssql_test.go | 30 ++++++++++----------- pkg/tsdb/sqleng/sql_engine.go | 5 +--- pkg/tsdb/sqleng/sql_engine_test.go | 42 ++++++++++++++++-------------- 4 files changed, 40 insertions(+), 39 deletions(-) diff --git a/pkg/tsdb/mssql/mssql.go b/pkg/tsdb/mssql/mssql.go index 14fa101147b..9c8c4d3520d 100644 --- a/pkg/tsdb/mssql/mssql.go +++ b/pkg/tsdb/mssql/mssql.go @@ -282,7 +282,7 @@ func (t *mssqlQueryResultTransformer) TransformQueryError(logger log.Logger, err // ref https://github.com/denisenkom/go-mssqldb/blob/045585d74f9069afe2e115b6235eb043c8047043/tds.go#L904 if strings.HasPrefix(strings.ToLower(err.Error()), "unable to open tcp connection with host") { logger.Error("Query error", "error", err) - return sqleng.ErrConnectionFailed.Errorf("failed to connect to server - %s", t.userError) + return fmt.Errorf("failed to connect to server - %s", t.userError) } return err diff --git a/pkg/tsdb/mssql/mssql_test.go b/pkg/tsdb/mssql/mssql_test.go index cfdb0dea313..df9e2f3903c 100644 --- a/pkg/tsdb/mssql/mssql_test.go +++ b/pkg/tsdb/mssql/mssql_test.go @@ -1313,23 +1313,23 @@ func TestMSSQL(t *testing.T) { func TestTransformQueryError(t *testing.T) { transformer := &mssqlQueryResultTransformer{} - randomErr := fmt.Errorf("random error") - - tests := []struct { - err error - expectedErr error - }{ - {err: fmt.Errorf("Unable to open tcp connection with host 'localhost:5000': dial tcp: connection refused"), expectedErr: sqleng.ErrConnectionFailed}, - {err: fmt.Errorf("unable to open tcp connection with host 'localhost:5000': dial tcp: connection refused"), expectedErr: sqleng.ErrConnectionFailed}, - {err: randomErr, expectedErr: randomErr}, - } - logger := backend.NewLoggerWith("logger", "mssql.test") - for _, tc := range tests { - resultErr := transformer.TransformQueryError(logger, tc.err) - assert.ErrorIs(t, resultErr, tc.expectedErr) - } + t.Run("Should not return a connection error", func(t *testing.T) { + err := fmt.Errorf("Unable to open tcp connection with host 'localhost:5000': dial tcp: connection refused") + resultErr := transformer.TransformQueryError(logger, err) + errorText := resultErr.Error() + assert.NotEqual(t, err, resultErr) + assert.NotContains(t, errorText, "Unable to open tcp connection with host") + assert.Contains(t, errorText, "failed to connect to server") + }) + + t.Run("Should return a non-connection error unmodified", func(t *testing.T) { + err := fmt.Errorf("normal error") + resultErr := transformer.TransformQueryError(logger, err) + assert.Equal(t, err, resultErr) + assert.ErrorIs(t, err, resultErr) + }) } func TestGenerateConnectionString(t *testing.T) { diff --git a/pkg/tsdb/sqleng/sql_engine.go b/pkg/tsdb/sqleng/sql_engine.go index 30863093299..d8c0b3cba0d 100644 --- a/pkg/tsdb/sqleng/sql_engine.go +++ b/pkg/tsdb/sqleng/sql_engine.go @@ -21,14 +21,11 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend/gtime" "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/grafana/grafana/pkg/setting" - "github.com/grafana/grafana/pkg/util/errutil" ) // MetaKeyExecutedQueryString is the key where the executed query should get stored const MetaKeyExecutedQueryString = "executedQueryString" -var ErrConnectionFailed = errutil.Internal("sqleng.connectionError") - // SQLMacroEngine interpolates macros into sql. It takes in the Query to have access to query context and // timeRange to be able to generate queries that use from and to. type SQLMacroEngine interface { @@ -112,7 +109,7 @@ func (e *DataSourceHandler) TransformQueryError(logger log.Logger, err error) er var opErr *net.OpError if errors.As(err, &opErr) { logger.Error("Query error", "err", err) - return ErrConnectionFailed.Errorf("failed to connect to server - %s", e.userError) + return fmt.Errorf("failed to connect to server - %s", e.userError) } return e.queryResultTransformer.TransformQueryError(logger, err) diff --git a/pkg/tsdb/sqleng/sql_engine_test.go b/pkg/tsdb/sqleng/sql_engine_test.go index 9231e8eb4e8..ede504534a7 100644 --- a/pkg/tsdb/sqleng/sql_engine_test.go +++ b/pkg/tsdb/sqleng/sql_engine_test.go @@ -400,28 +400,32 @@ func TestSQLEngine(t *testing.T) { } }) - t.Run("Should handle connection errors", func(t *testing.T) { - randomErr := fmt.Errorf("random error") - - tests := []struct { - err error - expectedErr error - expectQueryResultTransformerWasCalled bool - }{ - {err: &net.OpError{Op: "Dial", Err: fmt.Errorf("inner-error")}, expectedErr: ErrConnectionFailed, expectQueryResultTransformerWasCalled: false}, - {err: randomErr, expectedErr: randomErr, expectQueryResultTransformerWasCalled: true}, + t.Run("Should not return raw connection errors", func(t *testing.T) { + err := net.OpError{Op: "Dial", Err: fmt.Errorf("inner-error")} + transformer := &testQueryResultTransformer{} + dp := DataSourceHandler{ + log: backend.NewLoggerWith("logger", "test"), + queryResultTransformer: transformer, } + resultErr := dp.TransformQueryError(dp.log, &err) + assert.False(t, transformer.transformQueryErrorWasCalled) + errorText := resultErr.Error() + assert.NotEqual(t, err, resultErr) + assert.NotContains(t, errorText, "inner-error") + assert.Contains(t, errorText, "failed to connect to server") + }) - for _, tc := range tests { - transformer := &testQueryResultTransformer{} - dp := DataSourceHandler{ - log: backend.NewLoggerWith("logger", "test"), - queryResultTransformer: transformer, - } - resultErr := dp.TransformQueryError(dp.log, tc.err) - assert.ErrorIs(t, resultErr, tc.expectedErr) - assert.Equal(t, tc.expectQueryResultTransformerWasCalled, transformer.transformQueryErrorWasCalled) + t.Run("Should return non-connection errors unmodified", func(t *testing.T) { + err := fmt.Errorf("normal error") + transformer := &testQueryResultTransformer{} + dp := DataSourceHandler{ + log: backend.NewLoggerWith("logger", "test"), + queryResultTransformer: transformer, } + resultErr := dp.TransformQueryError(dp.log, err) + assert.True(t, transformer.transformQueryErrorWasCalled) + assert.Equal(t, err, resultErr) + assert.ErrorIs(t, err, resultErr) }) }