From ac8525c8e0a80cdd16be825f0fa4413b54751bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Vall=C3=A9s?= Date: Mon, 11 Dec 2023 12:40:27 +0100 Subject: [PATCH] feat: return end-user error message in gRPC endpoints The middleware that maps package errors to gRPC status codes is updated to return a human-friendly error message if it is present in the error. The package https://github.com/Southclaws/fault/fmsg is used to add such error messages. --- go.mod | 9 +++- go.sum | 13 ++++- pkg/middleware/interceptor.go | 75 +++++++++++++-------------- pkg/middleware/interceptor_test.go | 81 ++++++++++++++++++++++++++++++ 4 files changed, 133 insertions(+), 45 deletions(-) create mode 100644 pkg/middleware/interceptor_test.go diff --git a/go.mod b/go.mod index 117b88003..dabaa7dc7 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,8 @@ toolchain go1.21.3 require ( cloud.google.com/go/longrunning v0.5.2 + github.com/Southclaws/fault v0.8.0 + github.com/frankban/quicktest v1.14.6 github.com/gabriel-vasile/mimetype v1.4.3 github.com/go-redis/redis/v9 v9.0.0-beta.2 github.com/gofrs/uuid v4.4.0+incompatible @@ -77,6 +79,7 @@ require ( github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/snappy v0.0.4 // indirect github.com/google/flatbuffers v2.0.8+incompatible // indirect + github.com/google/go-cmp v0.6.0 // indirect github.com/google/s2a-go v0.1.7 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect github.com/googleapis/gax-go/v2 v2.12.0 // indirect @@ -85,6 +88,8 @@ require ( github.com/klauspost/asmfmt v1.3.2 // indirect github.com/klauspost/compress v1.15.9 // indirect github.com/klauspost/cpuid/v2 v2.0.9 // indirect + github.com/kr/pretty v0.3.1 // indirect + github.com/kr/text v0.2.0 // indirect github.com/lestrrat-go/jspointer v0.0.0-20181205001929-82fadba7561c // indirect github.com/lestrrat-go/jsref v0.0.0-20211028120858-c0bcbb5abf20 // indirect github.com/lestrrat-go/option v1.0.0 // indirect @@ -142,10 +147,10 @@ require ( github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/influxdata/line-protocol v0.0.0-20200327222509-2487e7298839 // indirect github.com/jackc/chunkreader/v2 v2.0.1 // indirect - github.com/jackc/pgconn v1.13.0 + github.com/jackc/pgconn v1.14.0 github.com/jackc/pgio v1.0.0 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect - github.com/jackc/pgproto3/v2 v2.3.1 // indirect + github.com/jackc/pgproto3/v2 v2.3.2 // indirect github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect github.com/jackc/pgtype v1.13.0 // indirect github.com/jackc/pgx/v4 v4.17.2 // indirect diff --git a/go.sum b/go.sum index dca96a2fb..c98b96ddd 100644 --- a/go.sum +++ b/go.sum @@ -474,6 +474,8 @@ github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbt github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/Shopify/logrus-bugsnag v0.0.0-20171204204709-577dee27f20d/go.mod h1:HI8ITrYtUY+O+ZhtlqUnD8+KwNPOyugEhfP9fdUIaEQ= +github.com/Southclaws/fault v0.8.0 h1:VOXsrlglSRBGrkM9aVFdo4P61DjiZSRPGYt6LdAHQtc= +github.com/Southclaws/fault v0.8.0/go.mod h1:VUVkAWutC59SL16s6FTqf3I6I2z77RmnaW5XRz4bLOE= github.com/advancedlogic/GoOse v0.0.0-20191112112754-e742535969c1 h1:d0Ct1dZwgwMO0Llf81Eu+Lyj6kwqXdqHP/WsSkEria0= github.com/advancedlogic/GoOse v0.0.0-20191112112754-e742535969c1/go.mod h1:f3HCSN1fBWjcpGtXyM119MJgeQl838v6so/PQOqvE1w= github.com/ajstarks/svgo v0.0.0-20180226025133-644b8db467af/go.mod h1:K08gAheRH3/J6wwsYMMT4xOr94bZjxIelGM0+d/wbFw= @@ -826,6 +828,8 @@ github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoD github.com/form3tech-oss/jwt-go v3.2.3+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= github.com/form3tech-oss/jwt-go v3.2.5+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= github.com/frankban/quicktest v1.11.3/go.mod h1:wRf/ReqHper53s+kmmSZizM8NamnL3IM0I9ntUbOk+k= +github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= +github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY= @@ -1212,8 +1216,9 @@ github.com/jackc/pgconn v1.8.0/go.mod h1:1C2Pb36bGIP9QHGBYCjnyhqu7Rv3sGshaQUvmfG github.com/jackc/pgconn v1.9.0/go.mod h1:YctiPyvzfU11JFxoXokUOOKQXQmDMoJL9vJzHH8/2JY= github.com/jackc/pgconn v1.9.1-0.20210724152538-d89c8390a530/go.mod h1:4z2w8XhRbP1hYxkpTuBjTS3ne3J48K83+u0zoyvg2pI= github.com/jackc/pgconn v1.11.0/go.mod h1:4z2w8XhRbP1hYxkpTuBjTS3ne3J48K83+u0zoyvg2pI= -github.com/jackc/pgconn v1.13.0 h1:3L1XMNV2Zvca/8BYhzcRFS70Lr0WlDg16Di6SFGAbys= github.com/jackc/pgconn v1.13.0/go.mod h1:AnowpAqO4CMIIJNZl2VJp+KrkAZciAkhEl0W0JIobpI= +github.com/jackc/pgconn v1.14.0 h1:vrbA9Ud87g6JdFWkHTJXppVce58qPIdP7N8y0Ml/A7Q= +github.com/jackc/pgconn v1.14.0/go.mod h1:9mBNlny0UvkgJdCDvdVHYSjI+8tD2rnKK69Wz8ti++E= github.com/jackc/pgerrcode v0.0.0-20201024163028-a0d42d470451/go.mod h1:a/s9Lp5W7n/DD0VrVoyJ00FbP2ytTPDVOivvn2bMlds= github.com/jackc/pgio v1.0.0 h1:g12B9UwVnzGhueNavwioyEEpAmqMe1E/BN9ES+8ovkE= github.com/jackc/pgio v1.0.0/go.mod h1:oP+2QK2wFfUWgr+gxjoBH9KGBb31Eio69xUb0w5bYf8= @@ -1233,8 +1238,9 @@ github.com/jackc/pgproto3/v2 v2.0.6/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwX github.com/jackc/pgproto3/v2 v2.0.7/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA= github.com/jackc/pgproto3/v2 v2.1.1/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA= github.com/jackc/pgproto3/v2 v2.2.0/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA= -github.com/jackc/pgproto3/v2 v2.3.1 h1:nwj7qwf0S+Q7ISFfBndqeLwSwxs+4DPsbRFjECT1Y4Y= github.com/jackc/pgproto3/v2 v2.3.1/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA= +github.com/jackc/pgproto3/v2 v2.3.2 h1:7eY55bdBeCz1F2fTzSz69QC+pG46jYq9/jtSPiJ5nn0= +github.com/jackc/pgproto3/v2 v2.3.2/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA= github.com/jackc/pgservicefile v0.0.0-20200307190119-3430c5407db8/go.mod h1:vsD4gTJCa9TptPL8sPkXrLZ+hDuNrZCnj29CQpr4X1E= github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b/go.mod h1:vsD4gTJCa9TptPL8sPkXrLZ+hDuNrZCnj29CQpr4X1E= github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a h1:bbPeKD0xmW/Y25WS6cokEszi5g+S0QxI/d45PkRi7Nk= @@ -1584,6 +1590,7 @@ github.com/pierrec/lz4/v4 v4.1.15/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFu github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4/go.mod h1:4OwLy04Bl9Ef3GJJCoec+30X3LQs/0/m4HFRt/2LUSA= github.com/pkg/browser v0.0.0-20210706143420-7d21f8c997e2/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI= github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI= +github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1-0.20171018195549-f15c970de5b7/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -1648,6 +1655,7 @@ github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6L github.com/rogpeppe/go-internal v1.1.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.2.2/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= +github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ= @@ -1934,6 +1942,7 @@ golang.org/x/crypto v0.0.0-20210817164053-32db794688a5/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20220214200702-86341886e292/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= +golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= golang.org/x/crypto v0.15.0 h1:frVn1TEaCEaZcn3Tmd7Y2b5KKPaZ+I32Q2OA3kYp5TA= golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72g= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= diff --git a/pkg/middleware/interceptor.go b/pkg/middleware/interceptor.go index 5b84588f0..390a44aab 100644 --- a/pkg/middleware/interceptor.go +++ b/pkg/middleware/interceptor.go @@ -12,6 +12,7 @@ import ( "gorm.io/gorm" + "github.com/Southclaws/fault/fmsg" grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" grpc_recovery "github.com/grpc-ecosystem/go-grpc-middleware/recovery" "github.com/instill-ai/pipeline-backend/pkg/acl" @@ -39,7 +40,7 @@ func UnaryAppendMetadataInterceptor(ctx context.Context, req interface{}, info * newCtx := metadata.NewIncomingContext(ctx, md) h, err := handler(newCtx, req) - return h, InjectErrCode(err) + return h, AsGRPCError(err) } // StreamAppendMetadataInterceptor - append metadatas for stream @@ -58,74 +59,66 @@ func StreamAppendMetadataInterceptor(srv interface{}, stream grpc.ServerStream, return err } -func InjectErrCode(err error) error { - +// AsGRPCError sets the gRPC status and error message according to the error +// type and metadata. +func AsGRPCError(err error) error { if err == nil { return nil } + var pgErr *pgconn.PgError - if errors.As(err, &pgErr) { - if pgErr.Code == "23505" { - return status.Error(codes.AlreadyExists, err.Error()) - } + var code codes.Code + msg := fmsg.GetIssue(err) + if msg == "" { + msg = err.Error() } - switch { + switch { case + errors.As(err, &pgErr) && pgErr.Code == "23505", errors.Is(err, gorm.ErrDuplicatedKey): - return status.Error(codes.AlreadyExists, err.Error()) - case - errors.Is(err, gorm.ErrRecordNotFound): - return status.Error(codes.NotFound, err.Error()) + code = codes.AlreadyExists case + errors.Is(err, gorm.ErrRecordNotFound), errors.Is(err, repository.ErrNoDataDeleted), - errors.Is(err, repository.ErrNoDataUpdated): - return status.Error(codes.NotFound, err.Error()) + errors.Is(err, repository.ErrNoDataUpdated), + errors.Is(err, service.ErrNotFound), + errors.Is(err, acl.ErrMembershipNotFound): + code = codes.NotFound case errors.Is(err, repository.ErrOwnerTypeNotMatch), - errors.Is(err, repository.ErrPageTokenDecode): - return status.Error(codes.InvalidArgument, err.Error()) + errors.Is(err, repository.ErrPageTokenDecode), + errors.Is(err, bcrypt.ErrMismatchedHashAndPassword), + errors.Is(err, handler.ErrCheckUpdateImmutableFields), + errors.Is(err, handler.ErrCheckOutputOnlyFields), + errors.Is(err, handler.ErrCheckRequiredFields), + errors.Is(err, handler.ErrFieldMask), + errors.Is(err, handler.ErrResourceID), + errors.Is(err, handler.ErrSematicVersion), + errors.Is(err, handler.ErrUpdateMask): + code = codes.InvalidArgument case errors.Is(err, service.ErrNoPermission), errors.Is(err, service.ErrCanNotTriggerNonLatestPipelineRelease): - return status.Error(codes.PermissionDenied, err.Error()) - - case - errors.Is(err, service.ErrNotFound): - return status.Error(codes.NotFound, err.Error()) + code = codes.PermissionDenied case errors.Is(err, service.ErrUnauthenticated): - return status.Error(codes.Unauthenticated, err.Error()) + + code = codes.Unauthenticated case errors.Is(err, service.ErrRateLimiting), errors.Is(err, service.ErrNamespacePrivatePipelineQuotaExceed), errors.Is(err, service.ErrNamespaceTriggerQuotaExceed): - return status.Error(codes.ResourceExhausted, err.Error()) - - case - errors.Is(err, acl.ErrMembershipNotFound): - return status.Error(codes.NotFound, err.Error()) - - case - errors.Is(err, bcrypt.ErrMismatchedHashAndPassword): - return status.Error(codes.InvalidArgument, err.Error()) - - case - errors.Is(err, handler.ErrCheckUpdateImmutableFields), - errors.Is(err, handler.ErrCheckOutputOnlyFields), - errors.Is(err, handler.ErrCheckRequiredFields), - errors.Is(err, handler.ErrFieldMask), - errors.Is(err, handler.ErrResourceID), - errors.Is(err, handler.ErrSematicVersion), - errors.Is(err, handler.ErrUpdateMask): - return status.Error(codes.InvalidArgument, err.Error()) + code = codes.ResourceExhausted default: return err } + + return status.Error(code, msg) } diff --git a/pkg/middleware/interceptor_test.go b/pkg/middleware/interceptor_test.go new file mode 100644 index 000000000..4249942ac --- /dev/null +++ b/pkg/middleware/interceptor_test.go @@ -0,0 +1,81 @@ +package middleware + +import ( + "fmt" + "testing" + + "github.com/Southclaws/fault" + "github.com/Southclaws/fault/fmsg" + qt "github.com/frankban/quicktest" + "github.com/jackc/pgconn" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "gorm.io/gorm" +) + +func TestAsGRPCError(t *testing.T) { + c := qt.New(t) + + c.Run("nil", func(c *qt.C) { + c.Assert(AsGRPCError(nil), qt.IsNil) + }) + + c.Run("unknown", func(c *qt.C) { + in := &pgconn.PgError{ + Severity: "FATAL", + Code: "08006", + Message: "connection_failure", + Detail: "connection_failure", + } + + got := AsGRPCError(in) + c.Assert(got, qt.IsNotNil) + c.Assert(got, qt.Equals, in) + }) + + testcases := []struct { + name string + in error + wantCode codes.Code + wantMessage string + }{ + { + name: "pq unique constraint", + in: &pgconn.PgError{ + Severity: "FATAL", + Code: "23505", + Message: "unique_violation", + Detail: "unique_violation", + ConstraintName: "idx_mytable_mycolumn", + }, + wantCode: codes.AlreadyExists, + }, + { + name: "with end-user message", + in: fault.Wrap( + gorm.ErrDuplicatedKey, + fmsg.WithDesc("already exists", "Resource already exists."), + ), + wantCode: codes.AlreadyExists, + wantMessage: "Resource already exists.", + }, + } + + for _, tc := range testcases { + c.Run(tc.name, func(c *qt.C) { + err := fmt.Errorf("new err: %w", tc.in) + got := AsGRPCError(err) + c.Assert(got, qt.IsNotNil) + + st, ok := status.FromError(got) + c.Assert(ok, qt.IsTrue) + c.Assert(st.Code(), qt.Equals, tc.wantCode) + if tc.wantMessage == "" { + c.Assert(st.Message(), qt.Equals, err.Error()) + return + } + + c.Assert(st.Message(), qt.Equals, tc.wantMessage) + }) + } +}