From 2e31ef3f293ef7390257eccbe3195a6c940e0e38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Niewrza=C5=82?= Date: Thu, 27 Jan 2022 11:30:45 +0100 Subject: [PATCH] satellite/metabase: better error message while move Before this change we were returning full DB error message. That can be very confusing for end user. This change is translating error message into more user frindly version and fixes also DRPC error status code. Fixes https://github.com/storj/team-metainfo/issues/76 Change-Id: I29b06ab4ba50a0d14db7a822a2906d95d65ab524 --- satellite/metabase/commit.go | 2 +- satellite/metabase/commit_test.go | 6 ++---- satellite/metabase/common.go | 3 +++ satellite/metabase/move_object.go | 7 ++++++- satellite/metabase/move_object_test.go | 22 ++++++++++++++++++++++ satellite/metainfo/endpoint.go | 2 ++ 6 files changed, 36 insertions(+), 6 deletions(-) diff --git a/satellite/metabase/commit.go b/satellite/metabase/commit.go index c6a60e55e..6082475bc 100644 --- a/satellite/metabase/commit.go +++ b/satellite/metabase/commit.go @@ -192,7 +192,7 @@ func (db *DB) BeginObjectExactVersion(ctx context.Context, opts BeginObjectExact ) if err != nil { if code := pgerrcode.FromError(err); code == pgxerrcode.UniqueViolation { - return Object{}, ErrConflict.New("object already exists") + return Object{}, Error.Wrap(ErrObjectAlreadyExists.New("")) } return Object{}, Error.New("unable to insert object: %w", err) } diff --git a/satellite/metabase/commit_test.go b/satellite/metabase/commit_test.go index d77194f34..d72d6d84e 100644 --- a/satellite/metabase/commit_test.go +++ b/satellite/metabase/commit_test.go @@ -494,8 +494,7 @@ func TestBeginObjectExactVersion(t *testing.T) { Encryption: metabasetest.DefaultEncryption, }, Version: -1, - ErrClass: &metabase.ErrConflict, - ErrText: "object already exists", + ErrClass: &metabase.ErrObjectAlreadyExists, }.Check(ctx, t, db) metabasetest.Verify{ @@ -545,8 +544,7 @@ func TestBeginObjectExactVersion(t *testing.T) { Encryption: metabasetest.DefaultEncryption, }, Version: -1, - ErrClass: &metabase.ErrConflict, - ErrText: "object already exists", + ErrClass: &metabase.ErrObjectAlreadyExists, }.Check(ctx, t, db) metabasetest.Verify{ diff --git a/satellite/metabase/common.go b/satellite/metabase/common.go index f59c1f0d9..63222cf9f 100644 --- a/satellite/metabase/common.go +++ b/satellite/metabase/common.go @@ -19,6 +19,9 @@ import ( // Error is the default error for metabase. var Error = errs.Class("metabase") +// ErrObjectAlreadyExists is used to indicate that object already exists. +var ErrObjectAlreadyExists = errs.Class("object already exists") + // Common constants for segment keys. const ( Delimiter = '/' diff --git a/satellite/metabase/move_object.go b/satellite/metabase/move_object.go index 75f577b19..9f186e8a7 100644 --- a/satellite/metabase/move_object.go +++ b/satellite/metabase/move_object.go @@ -8,9 +8,12 @@ import ( "database/sql" "errors" + pgxerrcode "github.com/jackc/pgerrcode" + "storj.io/common/storj" "storj.io/common/uuid" "storj.io/private/dbutil/pgutil" + "storj.io/private/dbutil/pgutil/pgerrcode" "storj.io/private/dbutil/txutil" "storj.io/private/tagsql" ) @@ -167,7 +170,9 @@ func (db *DB) FinishMoveObject(ctx context.Context, opts FinishMoveObject) (err var segmentsCount int row := db.db.QueryRowContext(ctx, updateObjectsQuery, []byte(opts.NewBucket), opts.NewEncryptedObjectKey, opts.NewEncryptedMetadataKey, opts.NewEncryptedMetadataKeyNonce, opts.ProjectID, []byte(opts.BucketName), opts.ObjectKey, opts.Version, opts.StreamID) if err = row.Scan(&segmentsCount); err != nil { - if errors.Is(err, sql.ErrNoRows) { + if code := pgerrcode.FromError(err); code == pgxerrcode.UniqueViolation { + return Error.Wrap(ErrObjectAlreadyExists.New("")) + } else if errors.Is(err, sql.ErrNoRows) { return storj.ErrObjectNotFound.New("object not found") } return Error.New("unable to update object: %w", err) diff --git a/satellite/metabase/move_object_test.go b/satellite/metabase/move_object_test.go index 308e9fe00..48cd87335 100644 --- a/satellite/metabase/move_object_test.go +++ b/satellite/metabase/move_object_test.go @@ -190,6 +190,28 @@ func TestFinishMoveObject(t *testing.T) { metabasetest.Verify{}.Check(ctx, t, db) }) + t.Run("object already exists", func(t *testing.T) { + defer metabasetest.DeleteAll{}.Check(ctx, t, db) + + moveObjStream := metabasetest.RandObjectStream() + metabasetest.CreateObject(ctx, t, db, moveObjStream, 0) + + conflictObjStream := metabasetest.RandObjectStream() + conflictObjStream.ProjectID = moveObjStream.ProjectID + metabasetest.CreateObject(ctx, t, db, conflictObjStream, 0) + + metabasetest.FinishMoveObject{ + Opts: metabase.FinishMoveObject{ + NewBucket: conflictObjStream.BucketName, + ObjectStream: moveObjStream, + NewEncryptedObjectKey: []byte(conflictObjStream.ObjectKey), + NewEncryptedMetadataKeyNonce: testrand.Nonce().Bytes(), + NewEncryptedMetadataKey: testrand.Bytes(265), + }, + ErrClass: &metabase.ErrObjectAlreadyExists, + }.Check(ctx, t, db) + }) + t.Run("object does not exist", func(t *testing.T) { defer metabasetest.DeleteAll{}.Check(ctx, t, db) diff --git a/satellite/metainfo/endpoint.go b/satellite/metainfo/endpoint.go index 0ed49c611..bee298980 100644 --- a/satellite/metainfo/endpoint.go +++ b/satellite/metainfo/endpoint.go @@ -278,6 +278,8 @@ func (endpoint *Endpoint) convertMetabaseErr(err error) error { return rpcstatus.Error(rpcstatus.NotFound, err.Error()) case metabase.ErrInvalidRequest.Has(err): return rpcstatus.Error(rpcstatus.InvalidArgument, err.Error()) + case metabase.ErrObjectAlreadyExists.Has(err): + return rpcstatus.Error(rpcstatus.AlreadyExists, err.Error()) default: endpoint.log.Error("internal", zap.Error(err)) return rpcstatus.Error(rpcstatus.Internal, err.Error())