Fix gorm wrong error type cast (flyteorg#48)

* Fix gorm wrong error type cast Signed-off-by: Haytham Abuelfutuh <[email protected]> * Fix unit tests Signed-off-by: Haytham Abuelfutuh <[email protected]> * Support unwrapping errors to detect connection problems Signed-off-by: Haytham Abuelfutuh <[email protected]> * cleanup Signed-off-by: Haytham Abuelfutuh <[email protected]>
eapolinario · Sep 2, 2021 · cf145bd · cf145bd
1 parent 901e298
commit cf145bd
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 16 deletions.
diff --git a/datacalog/go.sum b/datacalog/go.sum
@@ -434,7 +434,6 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
 github.com/lib/pq v1.1.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
-github.com/lib/pq v1.2.0 h1:LXpIM/LZ5xGFhOpXAQUIMM1HdyqzVYM13zNdjCEEcA0=
 github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
 github.com/lib/pq v1.3.0 h1:/qkRGz8zljWiDcFvgpwUpwIAPu3r07TDvs3Rws+o/pU=
 github.com/lib/pq v1.3.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
@@ -686,7 +685,6 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U
 golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
-golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad h1:DN0cp81fZ3njFcrLCytUHRSUkqBjfTo4Tx9RJTWs0EY=
 golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
 golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2 h1:It14KIkyBFYkHkwZ7k45minvA9aorojkyjGk9KJ5B/w=
 golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
@@ -767,7 +765,6 @@ golang.org/x/net v0.0.0-20201031054903-ff519b6c9102/go.mod h1:sp8m0HH+o8qH0wwXwY
 golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
 golang.org/x/net v0.0.0-20201209123823-ac852fbbde11/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
-golang.org/x/net v0.0.0-20210119194325-5f4716e94777 h1:003p0dJM77cxMSyCPFphvZf/Y5/NXf5fzg6ufd1/Oew=
 golang.org/x/net v0.0.0-20210119194325-5f4716e94777/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 h1:qWPm9rbaAMKs8Bq/9LRpbMqxWRVUAQwMI9fVrssnTfw=
 golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=

diff --git a/datacalog/pkg/manager/impl/artifact_manager.go b/datacalog/pkg/manager/impl/artifact_manager.go
@@ -161,7 +161,7 @@ func (m *artifactManager) GetArtifact(ctx context.Context, request *datacatalog.
 
 		if err != nil {
 			if errors.IsDoesNotExistError(err) {
-				logger.Warnf(ctx, "Artifact does not exist tag: %+v, err %v", request.GetTagName(), err)
+				logger.Infof(ctx, "Artifact does not exist tag: %+v, err %v", request.GetTagName(), err)
 				m.systemMetrics.doesNotExistCounter.Inc(ctx)
 			} else {
 				logger.Errorf(ctx, "Unable to retrieve Artifact by tag %v, err: %v", request.GetTagName(), err)

diff --git a/datacalog/pkg/repositories/errors/postgres.go b/datacalog/pkg/repositories/errors/postgres.go
@@ -1,11 +1,15 @@
 package errors
 
 import (
+	"errors"
 	"fmt"
+	"reflect"
+
+	"github.com/flyteorg/flytestdlib/logger"
 
 	"github.com/jackc/pgconn"
 
-	"github.com/flyteorg/datacatalog/pkg/errors"
+	catalogErrors "github.com/flyteorg/datacatalog/pkg/errors"
 	"google.golang.org/grpc/codes"
 	"gorm.io/gorm"
 )
@@ -22,32 +26,38 @@ type postgresErrorTransformer struct {
 const (
 	unexpectedType            = "unexpected error type for: %v"
 	uniqueConstraintViolation = "value with matching already exists (%s)"
-	defaultPgError            = "failed database operation with %s"
+	defaultPgError            = "failed database operation with code [%s] and msg [%s]"
 	unsupportedTableOperation = "cannot query with specified table attributes: %s"
 )
 
 func (p *postgresErrorTransformer) fromGormError(err error) error {
 	switch err.Error() {
 	case gorm.ErrRecordNotFound.Error():
-		return errors.NewDataCatalogErrorf(codes.NotFound, "entry not found")
+		return catalogErrors.NewDataCatalogErrorf(codes.NotFound, "entry not found")
 	default:
-		return errors.NewDataCatalogErrorf(codes.Internal, unexpectedType, err)
+		return catalogErrors.NewDataCatalogErrorf(codes.Internal, unexpectedType, err)
 	}
 }
 
 func (p *postgresErrorTransformer) ToDataCatalogError(err error) error {
-	cErr, ok := err.(ConnectError)
+	if unwrappedErr := errors.Unwrap(err); unwrappedErr != nil {
+		err = unwrappedErr
+	}
+
+	pqError, ok := err.(*pgconn.PgError)
 	if !ok {
+		logger.InfofNoCtx("Unable to cast to pgconn.PgError. Error type: [%v]",
+			reflect.TypeOf(err))
 		return p.fromGormError(err)
 	}
-	pqError := cErr.Unwrap().(*pgconn.PgError)
+
 	switch pqError.Code {
 	case uniqueConstraintViolationCode:
-		return errors.NewDataCatalogErrorf(codes.AlreadyExists, uniqueConstraintViolation, pqError.Message)
+		return catalogErrors.NewDataCatalogErrorf(codes.AlreadyExists, uniqueConstraintViolation, pqError.Message)
 	case undefinedTable:
-		return errors.NewDataCatalogErrorf(codes.InvalidArgument, unsupportedTableOperation, pqError.Message)
+		return catalogErrors.NewDataCatalogErrorf(codes.InvalidArgument, unsupportedTableOperation, pqError.Message)
 	default:
-		return errors.NewDataCatalogErrorf(codes.Unknown, fmt.Sprintf(defaultPgError, pqError.Message))
+		return catalogErrors.NewDataCatalogErrorf(codes.Unknown, fmt.Sprintf(defaultPgError, pqError.Code, pqError.Message))
 	}
 }
 

diff --git a/datacalog/pkg/repositories/gormimpl/artifact_test.go b/datacalog/pkg/repositories/gormimpl/artifact_test.go
@@ -275,7 +275,7 @@ func TestCreateArtifactAlreadyExists(t *testing.T) {
 	assert.Error(t, err)
 	dcErr, ok := err.(apiErrors.DataCatalogError)
 	assert.True(t, ok)
-	assert.Equal(t, dcErr.Code(), codes.AlreadyExists)
+	assert.Equal(t, dcErr.Code().String(), codes.AlreadyExists.String())
 }
 
 func TestListArtifactsWithPartition(t *testing.T) {

diff --git a/datacalog/pkg/repositories/gormimpl/dataset_test.go b/datacalog/pkg/repositories/gormimpl/dataset_test.go
@@ -246,7 +246,7 @@ func TestCreateDatasetAlreadyExists(t *testing.T) {
 	assert.Error(t, err)
 	dcErr, ok := err.(datacatalog_error.DataCatalogError)
 	assert.True(t, ok)
-	assert.Equal(t, dcErr.Code(), codes.AlreadyExists)
+	assert.Equal(t, dcErr.Code().String(), codes.AlreadyExists.String())
 }
 
 func TestListDatasets(t *testing.T) {

diff --git a/datacalog/pkg/repositories/gormimpl/tag_test.go b/datacalog/pkg/repositories/gormimpl/tag_test.go
@@ -157,5 +157,5 @@ func TestTagAlreadyExists(t *testing.T) {
 	assert.Error(t, err)
 	dcErr, ok := err.(datacatalog_error.DataCatalogError)
 	assert.True(t, ok)
-	assert.Equal(t, dcErr.Code(), codes.AlreadyExists)
+	assert.Equal(t, dcErr.Code().String(), codes.AlreadyExists.String())
 }