diff --git a/changelog/unreleased/services-should-never-return-transport-level-errors.md b/changelog/unreleased/services-should-never-return-transport-level-errors.md new file mode 100644 index 0000000000..85eecf5aa9 --- /dev/null +++ b/changelog/unreleased/services-should-never-return-transport-level-errors.md @@ -0,0 +1,5 @@ +Bugfix: services should never return transport level errors + +The CS3 API adopted the grpc error codes from the [google grpc status package](https://github.com/googleapis/googleapis/blob/master/google/rpc/status.proto). It also separates transport level errors from application level errors on purpose. This allows sending CS3 messages over protocols other than GRPC. To keep that seperation, the server side must always return `nil`, even though the code generation for go produces function signatures for rpcs with an `error` return property. That allows clients to clearly distinguish between transport level errors indicated by `err != nil` the error and application level errors by checking the status code. + +https://github.com/cs3org/reva/pull/2415 diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 4a944e692f..3f0620858d 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -46,9 +46,6 @@ import ( "github.com/cs3org/reva/pkg/utils" "github.com/golang-jwt/jwt" "github.com/pkg/errors" - - "google.golang.org/grpc/codes" - gstatus "google.golang.org/grpc/status" ) /* About caching @@ -125,15 +122,9 @@ func (s *svc) CreateHome(ctx context.Context, req *provider.CreateHomeRequest) ( res, err := s.CreateStorageSpace(ctx, createReq) if err != nil { return &provider.CreateHomeResponse{ - Status: status.NewInternal(ctx, "error calling CreateHome"), - }, nil - } - if res.Status.Code != rpc.Code_CODE_OK && res.Status.Code != rpc.Code_CODE_ALREADY_EXISTS { - return &provider.CreateHomeResponse{ - Status: res.Status, + Status: status.NewStatusFromErrType(ctx, "gateway could not call CreateStorageSpace", err), }, nil } - return &provider.CreateHomeResponse{ Opaque: res.Opaque, Status: res.Status, @@ -141,8 +132,6 @@ func (s *svc) CreateHome(ctx context.Context, req *provider.CreateHomeRequest) ( } func (s *svc) CreateStorageSpace(ctx context.Context, req *provider.CreateStorageSpaceRequest) (*provider.CreateStorageSpaceResponse, error) { - log := appctx.GetLogger(ctx) - // TODO change the CreateStorageSpaceRequest to contain a space instead of sending individual properties space := &provider.StorageSpace{ Owner: req.Owner, @@ -159,12 +148,16 @@ func (s *svc) CreateStorageSpace(ctx context.Context, req *provider.CreateStorag srClient, err := s.getStorageRegistryClient(ctx, s.c.StorageRegistryEndpoint) if err != nil { - return nil, errors.Wrap(err, "gateway: error getting storage registry client") + return &provider.CreateStorageSpaceResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could get storage registry client", err), + }, nil } spaceJSON, err := json.Marshal(space) if err != nil { - return nil, errors.Wrap(err, "gateway: marshaling space failed") + return &provider.CreateStorageSpaceResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not marshal space json", err), + }, nil } // The registry is responsible for choosing the right provider @@ -179,7 +172,9 @@ func (s *svc) CreateStorageSpace(ctx context.Context, req *provider.CreateStorag }, }) if err != nil { - return nil, err + return &provider.CreateStorageSpaceResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call GetStorageProviders", err), + }, nil } if res.Status.Code != rpc.Code_CODE_OK { return &provider.CreateStorageSpaceResponse{ @@ -189,20 +184,21 @@ func (s *svc) CreateStorageSpace(ctx context.Context, req *provider.CreateStorag if len(res.Providers) == 0 { return &provider.CreateStorageSpaceResponse{ - Status: status.NewNotFound(ctx, fmt.Sprintf("error finding provider for space %+v", space)), + Status: status.NewNotFound(ctx, fmt.Sprintf("gateway found no provider for space %+v", space)), }, nil } // just pick the first provider, we expect only one c, err := s.getStorageProviderClient(ctx, res.Providers[0]) if err != nil { - return nil, err + return &provider.CreateStorageSpaceResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not get storage provider client", err), + }, nil } createRes, err := c.CreateStorageSpace(ctx, req) if err != nil { - log.Err(err).Msg("gateway: error creating storage space on storage provider") return &provider.CreateStorageSpaceResponse{ - Status: status.NewInternal(ctx, "error calling CreateStorageSpace"), + Status: status.NewStatusFromErrType(ctx, "gateway could not call CreateStorageSpace", err), }, nil } @@ -210,7 +206,6 @@ func (s *svc) CreateStorageSpace(ctx context.Context, req *provider.CreateStorag } func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSpacesRequest) (*provider.ListStorageSpacesResponse, error) { - // TODO update CS3 api to forward the filters to the registry so it can filter the number of providers the gateway needs to query filters := map[string]string{} @@ -236,7 +231,9 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp c, err := s.getStorageRegistryClient(ctx, s.c.StorageRegistryEndpoint) if err != nil { - return nil, errors.Wrap(err, "gateway: error getting storage registry client") + return &provider.ListStorageSpacesResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not get storage registry client", err), + }, nil } listReq := ®istry.ListStorageProvidersRequest{Opaque: req.Opaque} @@ -246,7 +243,7 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp res, err := c.ListStorageProviders(ctx, listReq) if err != nil { return &provider.ListStorageSpacesResponse{ - Status: status.NewStatusFromErrType(ctx, "ListStorageSpaces filters: req "+req.String(), err), + Status: status.NewStatusFromErrType(ctx, "gateway could not call ListStorageSpaces", err), }, nil } if res.Status.Code != rpc.Code_CODE_OK { @@ -289,20 +286,19 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp } func (s *svc) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorageSpaceRequest) (*provider.UpdateStorageSpaceResponse, error) { - log := appctx.GetLogger(ctx) // TODO: needs to be fixed - c, _, err := s.find(ctx, &provider.Reference{ResourceId: req.StorageSpace.Root}) + ref := &provider.Reference{ResourceId: req.StorageSpace.Root} + c, _, err := s.find(ctx, ref) if err != nil { return &provider.UpdateStorageSpaceResponse{ - Status: status.NewStatusFromErrType(ctx, "error finding ID", err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find reference %+v", ref), err), }, nil } res, err := c.UpdateStorageSpace(ctx, req) if err != nil { - log.Err(err).Msg("gateway: error creating update space on storage provider") return &provider.UpdateStorageSpaceResponse{ - Status: status.NewInternal(ctx, "error calling UpdateStorageSpace"), + Status: status.NewStatusFromErrType(ctx, "gateway could not call UpdateStorageSpace", err), }, nil } s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), res.StorageSpace.Root) @@ -310,30 +306,29 @@ func (s *svc) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorag } func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) (*provider.DeleteStorageSpaceResponse, error) { - log := appctx.GetLogger(ctx) // TODO: needs to be fixed storageid, opaqeid, err := utils.SplitStorageSpaceID(req.Id.OpaqueId) if err != nil { return &provider.DeleteStorageSpaceResponse{ - Status: status.NewInvalidArg(ctx, "OpaqueID was empty"), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not split space id %s", req.GetId().GetOpaqueId()), err), }, nil } - c, _, err := s.find(ctx, &provider.Reference{ResourceId: &provider.ResourceId{ + ref := &provider.Reference{ResourceId: &provider.ResourceId{ StorageId: storageid, OpaqueId: opaqeid, - }}) + }} + c, _, err := s.find(ctx, ref) if err != nil { return &provider.DeleteStorageSpaceResponse{ - Status: status.NewStatusFromErrType(ctx, "error finding path", err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find reference %+v", ref), err), }, nil } res, err := c.DeleteStorageSpace(ctx, req) if err != nil { - log.Err(err).Msg("gateway: error deleting storage space on storage provider") return &provider.DeleteStorageSpaceResponse{ - Status: status.NewInternal(ctx, "error calling DeleteStorageSpace"), + Status: status.NewStatusFromErrType(ctx, "gateway could not call DeleteStorageSpace", err), }, nil } @@ -346,7 +341,9 @@ func (s *svc) GetHome(ctx context.Context, _ *provider.GetHomeRequest) (*provide srClient, err := s.getStorageRegistryClient(ctx, s.c.StorageRegistryEndpoint) if err != nil { - return nil, errors.Wrap(err, "gateway: error getting storage registry client") + return &provider.GetHomeResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not get storage registry client", err), + }, nil } spaceJSON, err := json.Marshal(&provider.StorageSpace{ @@ -354,7 +351,9 @@ func (s *svc) GetHome(ctx context.Context, _ *provider.GetHomeRequest) (*provide SpaceType: "personal", }) if err != nil { - return nil, errors.Wrap(err, "gateway: marshaling space failed") + return &provider.GetHomeResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not marshal space", err), + }, nil } // The registry is responsible for choosing the right provider @@ -370,7 +369,9 @@ func (s *svc) GetHome(ctx context.Context, _ *provider.GetHomeRequest) (*provide }, }) if err != nil { - return nil, err + return &provider.GetHomeResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call GetStorageProviders", err), + }, nil } if res.Status.Code != rpc.Code_CODE_OK { return &provider.GetHomeResponse{ @@ -405,16 +406,15 @@ func (s *svc) InitiateFileDownload(ctx context.Context, req *provider.InitiateFi c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &gateway.InitiateFileDownloadResponse{ - Status: status.NewStatusFromErrType(ctx, "error initiating download ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } storageRes, err := c.InitiateFileDownload(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &gateway.InitiateFileDownloadResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil - } - return nil, errors.Wrap(err, "gateway: error calling InitiateFileDownload") + return &gateway.InitiateFileDownloadResponse{ + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not call InitiateFileDownload, ref=%+v", req.Ref), err), + }, nil } protocols := make([]*gateway.FileDownloadProtocol, len(storageRes.Protocols)) @@ -430,7 +430,7 @@ func (s *svc) InitiateFileDownload(ctx context.Context, req *provider.InitiateFi u, err := url.Parse(protocols[p].DownloadEndpoint) if err != nil { return &gateway.InitiateFileDownloadResponse{ - Status: status.NewInternal(ctx, "wrong format for download endpoint"), + Status: status.NewStatusFromErrType(ctx, "wrong format for download endpoint", err), }, nil } @@ -439,7 +439,7 @@ func (s *svc) InitiateFileDownload(ctx context.Context, req *provider.InitiateFi token, err := s.sign(ctx, target) if err != nil { return &gateway.InitiateFileDownloadResponse{ - Status: status.NewInternal(ctx, "error creating signature for download"), + Status: status.NewStatusFromErrType(ctx, "error creating signature for download", err), }, nil } @@ -461,16 +461,15 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &gateway.InitiateFileUploadResponse{ - Status: status.NewStatusFromErrType(ctx, "initiateFileUpload ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } storageRes, err := c.InitiateFileUpload(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &gateway.InitiateFileUploadResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil - } - return nil, errors.Wrap(err, "gateway: error calling InitiateFileUpload") + return &gateway.InitiateFileUploadResponse{ + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not call InitiateFileUpload, ref=%+v", req.Ref), err), + }, nil } if storageRes.Status.Code != rpc.Code_CODE_OK { @@ -493,7 +492,7 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile u, err := url.Parse(protocols[p].UploadEndpoint) if err != nil { return &gateway.InitiateFileUploadResponse{ - Status: status.NewInternal(ctx, "wrong format for upload endpoint"), + Status: status.NewStatusFromErrType(ctx, "wrong format for upload endpoint", err), }, nil } @@ -502,7 +501,7 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile token, err := s.sign(ctx, target) if err != nil { return &gateway.InitiateFileUploadResponse{ - Status: status.NewInternal(ctx, "error creating signature for upload"), + Status: status.NewStatusFromErrType(ctx, "error creating signature for upload", err), }, nil } @@ -520,10 +519,11 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile } func (s *svc) GetPath(ctx context.Context, req *provider.GetPathRequest) (*provider.GetPathResponse, error) { - c, p, err := s.find(ctx, &provider.Reference{ResourceId: req.ResourceId}) + ref := &provider.Reference{ResourceId: req.ResourceId} + c, p, err := s.find(ctx, ref) if err != nil { return &provider.GetPathResponse{ - Status: status.NewStatusFromErrType(ctx, "getpath ref="+req.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find reference %+v", ref), err), }, nil } @@ -535,8 +535,9 @@ func (s *svc) GetPath(ctx context.Context, req *provider.GetPathRequest) (*provi res, err := c.GetPath(ctx, req) if err != nil { - err = errors.Wrap(err, "gateway: error getting path:"+req.String()) - return nil, err + return &provider.GetPathResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call GetPath", err), + }, nil } if res.Status.Code != rpc.Code_CODE_OK { @@ -557,13 +558,15 @@ func (s *svc) CreateContainer(ctx context.Context, req *provider.CreateContainer c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.CreateContainerResponse{ - Status: status.NewStatusFromErrType(ctx, "createContainer ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.CreateContainer(ctx, req) if err != nil { - return nil, errors.Wrap(err, "gateway: error calling CreateContainer") + return &provider.CreateContainerResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call CreateContainer", err), + }, nil } s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId) @@ -574,16 +577,15 @@ func (s *svc) TouchFile(ctx context.Context, req *provider.TouchFileRequest) (*p c, _, err := s.find(ctx, req.Ref) if err != nil { return &provider.TouchFileResponse{ - Status: status.NewStatusFromErrType(ctx, "TouchFile ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find reference %+v", req.Ref), err), }, nil } res, err := c.TouchFile(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &provider.TouchFileResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil - } - return nil, errors.Wrap(err, "gateway: error calling TouchFile") + return &provider.TouchFileResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call TouchFile", err), + }, nil } return res, nil @@ -596,18 +598,15 @@ func (s *svc) Delete(ctx context.Context, req *provider.DeleteRequest) (*provide c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.DeleteResponse{ - Status: status.NewStatusFromErrType(ctx, "delete ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.Delete(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &provider.DeleteResponse{ - Status: status.NewPermissionDenied(ctx, err, "permission denied"), - }, nil - } - return nil, errors.Wrap(err, "gateway: error calling Delete") + return &provider.DeleteResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call Delete", err), + }, nil } s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId) @@ -622,7 +621,7 @@ func (s *svc) Move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo c, sourceProviderInfo, req.Source, err = s.findAndUnwrap(ctx, req.Source) if err != nil { return &provider.MoveResponse{ - Status: status.NewStatusFromErrType(ctx, "Move ref="+req.Source.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Source), err), }, nil } @@ -635,22 +634,27 @@ func (s *svc) Move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo _, destinationProviderInfo, req.Destination, err = s.findAndUnwrap(ctx, req.Destination) if err != nil { return &provider.MoveResponse{ - Status: status.NewStatusFromErrType(ctx, "Move ref="+req.Destination.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Destination), err), }, nil } // if the storage id is the same the storage provider decides if the move is allowedy or not if sourceProviderInfo.Address != destinationProviderInfo.Address { - res := &provider.MoveResponse{ - Status: status.NewUnimplemented(ctx, nil, "gateway: cross storage move not supported, use copy and delete"), - } - return res, nil + return &provider.MoveResponse{ + Status: status.NewUnimplemented(ctx, nil, "gateway does not support cross storage move, use copy and delete"), + }, nil } } s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.Source.ResourceId) s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.Destination.ResourceId) - return c.Move(ctx, req) + res, err := c.Move(ctx, req) + if err != nil { + return &provider.MoveResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call Move", err), + }, nil + } + return res, nil } func (s *svc) SetArbitraryMetadata(ctx context.Context, req *provider.SetArbitraryMetadataRequest) (*provider.SetArbitraryMetadataResponse, error) { @@ -660,16 +664,15 @@ func (s *svc) SetArbitraryMetadata(ctx context.Context, req *provider.SetArbitra c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.SetArbitraryMetadataResponse{ - Status: status.NewStatusFromErrType(ctx, "SetArbitraryMetadata ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.SetArbitraryMetadata(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &provider.SetArbitraryMetadataResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil - } - return nil, errors.Wrap(err, "gateway: error calling SetArbitraryMetadata") + return &provider.SetArbitraryMetadataResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call SetArbitraryMetadata", err), + }, nil } s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId) @@ -683,16 +686,15 @@ func (s *svc) UnsetArbitraryMetadata(ctx context.Context, req *provider.UnsetArb c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.UnsetArbitraryMetadataResponse{ - Status: status.NewStatusFromErrType(ctx, "UnsetArbitraryMetadata ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.UnsetArbitraryMetadata(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &provider.UnsetArbitraryMetadataResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil - } - return nil, errors.Wrap(err, "gateway: error calling UnsetArbitraryMetadata") + return &provider.UnsetArbitraryMetadataResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call UnsetArbitraryMetadata", err), + }, nil } s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId) @@ -706,16 +708,15 @@ func (s *svc) SetLock(ctx context.Context, req *provider.SetLockRequest) (*provi c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.SetLockResponse{ - Status: status.NewStatusFromErrType(ctx, "SetLock ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.SetLock(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &provider.SetLockResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil - } - return nil, errors.Wrap(err, "gateway: error calling SetLock") + return &provider.SetLockResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call SetLock", err), + }, nil } return res, nil @@ -728,16 +729,15 @@ func (s *svc) GetLock(ctx context.Context, req *provider.GetLockRequest) (*provi c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.GetLockResponse{ - Status: status.NewStatusFromErrType(ctx, "GetLock ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.GetLock(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &provider.GetLockResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil - } - return nil, errors.Wrap(err, "gateway: error calling GetLock") + return &provider.GetLockResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call GetLock", err), + }, nil } return res, nil @@ -750,16 +750,15 @@ func (s *svc) RefreshLock(ctx context.Context, req *provider.RefreshLockRequest) c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.RefreshLockResponse{ - Status: status.NewStatusFromErrType(ctx, "RefreshLock ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.RefreshLock(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &provider.RefreshLockResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil - } - return nil, errors.Wrap(err, "gateway: error calling RefreshLock") + return &provider.RefreshLockResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call RefreshLock", err), + }, nil } return res, nil @@ -772,16 +771,15 @@ func (s *svc) Unlock(ctx context.Context, req *provider.UnlockRequest) (*provide c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.UnlockResponse{ - Status: status.NewStatusFromErrType(ctx, "Unlock ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.Unlock(ctx, req) if err != nil { - if gstatus.Code(err) == codes.PermissionDenied { - return &provider.UnlockResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil - } - return nil, errors.Wrap(err, "gateway: error calling Unlock") + return &provider.UnlockResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call Unlock", err), + }, nil } return res, nil @@ -805,7 +803,7 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St if err != nil { // we have no provider -> not found return &provider.StatResponse{ - Status: status.NewStatusFromErrType(ctx, "could not find provider", err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } @@ -850,15 +848,15 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St var currentInfo *provider.ResourceInfo statResp, err := c.Stat(ctx, &provider.StatRequest{Opaque: req.Opaque, Ref: providerRef, ArbitraryMetadataKeys: req.ArbitraryMetadataKeys}) if err != nil { - appctx.GetLogger(ctx).Error().Err(err).Msg("gateway: could not stat parent mount, skipping") + appctx.GetLogger(ctx).Error().Err(err).Str("service", "gateway").Msg("could not stat parent mount, skipping") continue } if statResp.Status.Code != rpc.Code_CODE_OK { - appctx.GetLogger(ctx).Debug().Interface("status", statResp.Status).Msg("gateway: stating parent mount was not ok, skipping") + appctx.GetLogger(ctx).Debug().Interface("status", statResp.Status).Str("service", "gateway").Msg("stating parent mount was not ok, skipping") continue } if statResp.Info == nil { - appctx.GetLogger(ctx).Error().Err(err).Msg("gateway: stat response for parent mount carried no info, skipping") + appctx.GetLogger(ctx).Error().Err(err).Str("service", "gateway").Msg("stat response for parent mount carried no info, skipping") continue } @@ -947,7 +945,7 @@ func (s *svc) ListContainer(ctx context.Context, req *provider.ListContainerRequ if err != nil { // we have no provider -> not found return &provider.ListContainerResponse{ - Status: status.NewStatusFromErrType(ctx, "could not find provider", err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } // list /foo, mount points at /foo/bar, /foo/bif, /foo/bar/bam @@ -972,7 +970,7 @@ func (s *svc) ListContainer(ctx context.Context, req *provider.ListContainerRequ // get client for storage provider c, err := s.getStorageProviderClient(ctx, providerInfos[i]) if err != nil { - appctx.GetLogger(ctx).Error().Err(err).Msg("gateway: could not get storage provider client, skipping") + appctx.GetLogger(ctx).Error().Err(err).Str("service", "gateway").Msg("could not get storage provider client, skipping") continue } @@ -1022,7 +1020,7 @@ func (s *svc) ListContainer(ctx context.Context, req *provider.ListContainerRequ ArbitraryMetadataKeys: req.ArbitraryMetadataKeys, }) if err != nil || rsp.Status.Code != rpc.Code_CODE_OK { - appctx.GetLogger(ctx).Error().Err(err).Msg("gateway: could not list provider, skipping") + appctx.GetLogger(ctx).Error().Err(err).Str("service", "gateway").Msg("could not list provider, skipping") continue } @@ -1057,15 +1055,15 @@ func (s *svc) ListContainer(ctx context.Context, req *provider.ListContainerRequ ArbitraryMetadataKeys: req.ArbitraryMetadataKeys, }) if err != nil { - appctx.GetLogger(ctx).Error().Err(err).Msg("gateway: could not stat parent mount for list, skipping") + appctx.GetLogger(ctx).Error().Err(err).Str("service", "gateway").Msg("could not stat parent mount for list, skipping") continue } if statResp.Status.Code != rpc.Code_CODE_OK { - appctx.GetLogger(ctx).Debug().Interface("status", statResp.Status).Msg("gateway: stating parent mount for list was not ok, skipping") + appctx.GetLogger(ctx).Debug().Interface("status", statResp.Status).Str("service", "gateway").Msg("stating parent mount for list was not ok, skipping") continue } if statResp.Info == nil { - appctx.GetLogger(ctx).Error().Err(err).Msg("gateway: stat response for list carried no info, skipping") + appctx.GetLogger(ctx).Error().Err(err).Str("service", "gateway").Msg("stat response for list carried no info, skipping") continue } @@ -1102,7 +1100,7 @@ func (s *svc) ListContainer(ctx context.Context, req *provider.ListContainerRequ } default: log := appctx.GetLogger(ctx) - log.Err(err).Msg("gateway: unhandled ListContainer case") + log.Err(err).Str("service", "gateway").Msg("unhandled ListContainer case") } } } @@ -1129,13 +1127,15 @@ func (s *svc) ListFileVersions(ctx context.Context, req *provider.ListFileVersio c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.ListFileVersionsResponse{ - Status: status.NewStatusFromErrType(ctx, "ListFileVersions ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.ListFileVersions(ctx, req) if err != nil { - return nil, errors.Wrap(err, "gateway: error calling ListFileVersions") + return &provider.ListFileVersionsResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call ListFileVersions", err), + }, nil } return res, nil @@ -1147,13 +1147,15 @@ func (s *svc) RestoreFileVersion(ctx context.Context, req *provider.RestoreFileV c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.RestoreFileVersionResponse{ - Status: status.NewStatusFromErrType(ctx, "RestoreFileVersion ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } res, err := c.RestoreFileVersion(ctx, req) if err != nil { - return nil, errors.Wrap(err, "gateway: error calling RestoreFileVersion") + return &provider.RestoreFileVersionResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call RestoreFileVersion", err), + }, nil } s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId) @@ -1169,7 +1171,7 @@ func (s *svc) ListRecycle(ctx context.Context, req *provider.ListRecycleRequest) providerInfos, err := s.findSpaces(ctx, req.Ref) if err != nil { return &provider.ListRecycleResponse{ - Status: status.NewStatusFromErrType(ctx, "ListRecycle ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } for i := range providerInfos { @@ -1178,7 +1180,7 @@ func (s *svc) ListRecycle(ctx context.Context, req *provider.ListRecycleRequest) c, err := s.getStorageProviderClient(ctx, providerInfos[i]) if err != nil { return &provider.ListRecycleResponse{ - Status: status.NewInternal(ctx, "gateway: could not get storage provider client"), + Status: status.NewStatusFromErrType(ctx, "gateway could not get storage provider client", err), }, nil } @@ -1220,7 +1222,9 @@ func (s *svc) ListRecycle(ctx context.Context, req *provider.ListRecycleRequest) Key: req.Key, }) if err != nil { - return nil, errors.Wrap(err, "gateway: error calling ListRecycle") + return &provider.ListRecycleResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call ListRecycle", err), + }, nil } if utils.IsAbsoluteReference(req.Ref) { @@ -1245,7 +1249,7 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc providerInfos, err := s.findSpaces(ctx, req.Ref) if err != nil { return &provider.RestoreRecycleItemResponse{ - Status: status.NewStatusFromErrType(ctx, "RestoreRecycleItem source ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } var srcProvider *registry.ProviderInfo @@ -1288,7 +1292,7 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc dstProviderInfos, err := s.findSpaces(ctx, req.RestoreRef) if err != nil { return &provider.RestoreRecycleItemResponse{ - Status: status.NewStatusFromErrType(ctx, "RestoreRecycleItem source ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.RestoreRef), err), }, nil } var dstProvider *registry.ProviderInfo @@ -1328,6 +1332,7 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc if srcRef.ResourceId.StorageId != dstRef.ResourceId.StorageId || srcProvider.Address != dstProvider.Address { return &provider.RestoreRecycleItemResponse{ + // TODO in Move() we return an unimplemented / supported ... align? Status: status.NewPermissionDenied(ctx, err, "gateway: cross-storage restores are forbidden"), }, nil } @@ -1336,7 +1341,7 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc c, err := s.getStorageProviderClient(ctx, srcProvider) if err != nil { return &provider.RestoreRecycleItemResponse{ - Status: status.NewInternal(ctx, "gateway: could not get storage provider client"), + Status: status.NewStatusFromErrType(ctx, "gateway could not get storage provider client", err), }, nil } @@ -1345,7 +1350,9 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc res, err := c.RestoreRecycleItem(ctx, req) if err != nil { - return nil, errors.Wrap(err, "gateway: error calling RestoreRecycleItem") + return &provider.RestoreRecycleItemResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call RestoreRecycleItem", err), + }, nil } s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId) @@ -1360,7 +1367,7 @@ func (s *svc) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleReques c, _, relativeReference, err := s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.PurgeRecycleResponse{ - Status: status.NewStatusFromErrType(ctx, "PurgeRecycle ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } @@ -1370,7 +1377,9 @@ func (s *svc) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleReques Key: req.Key, }) if err != nil { - return nil, errors.Wrap(err, "gateway: error calling PurgeRecycle") + return &provider.PurgeRecycleResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call PurgeRecycle", err), + }, nil } s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId) @@ -1381,7 +1390,7 @@ func (s *svc) GetQuota(ctx context.Context, req *gateway.GetQuotaRequest) (*prov c, _, relativeReference, err := s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.GetQuotaResponse{ - Status: status.NewStatusFromErrType(ctx, "GetQuota ref="+req.Ref.String(), err), + Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not find space for ref=%+v", req.Ref), err), }, nil } @@ -1390,7 +1399,9 @@ func (s *svc) GetQuota(ctx context.Context, req *gateway.GetQuotaRequest) (*prov Ref: relativeReference, }) if err != nil { - return nil, errors.Wrap(err, "gateway: error calling GetQuota") + return &provider.GetQuotaResponse{ + Status: status.NewStatusFromErrType(ctx, "gateway could not call GetQuota", err), + }, nil } return res, nil } @@ -1447,7 +1458,6 @@ func (s *svc) findAndUnwrap(ctx context.Context, ref *provider.Reference) (provi func (s *svc) getStorageProviderClient(_ context.Context, p *registry.ProviderInfo) (provider.ProviderAPIClient, error) { c, err := pool.GetStorageProviderServiceClient(p.Address) if err != nil { - err = errors.Wrap(err, "gateway: error getting a storage provider client") return nil, err } @@ -1457,33 +1467,12 @@ func (s *svc) getStorageProviderClient(_ context.Context, p *registry.ProviderIn func (s *svc) getStorageRegistryClient(_ context.Context, address string) (registry.RegistryAPIClient, error) { c, err := pool.GetStorageRegistryClient(address) if err != nil { - err = errors.Wrap(err, "gateway: error getting a storage provider client") return nil, err } return s.cache.StorageRegistryClient(c), nil } -// func (s *svc) findMountPoint(ctx context.Context, id *provider.ResourceId) ([]*registry.ProviderInfo, error) { -// // TODO can we use a provider cache for mount points? -// if id == nil { -// return nil, errtypes.BadRequest("invalid reference, at least path or id must be set") -// } - -// filters := map[string]string{ -// "type": "mountpoint", -// "storage_id": id.StorageId, -// "opaque_id": id.OpaqueId, -// } - -// listReq := ®istry.ListStorageProvidersRequest{ -// Opaque: &typesv1beta1.Opaque{}, -// } -// sdk.EncodeOpaqueMap(listReq.Opaque, filters) - -// return s.findProvider(ctx, listReq) -// } - func (s *svc) findSpaces(ctx context.Context, ref *provider.Reference) ([]*registry.ProviderInfo, error) { switch { case ref == nil: diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index 6a090c4791..ad7d5f1171 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -1026,15 +1026,7 @@ func (s *service) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleRe key, itemPath := router.ShiftPath(req.Key) if key != "" { if err := s.storage.PurgeRecycleItem(ctx, req.Ref, key, itemPath); err != nil { - var st *rpc.Status - switch err.(type) { - case errtypes.IsNotFound: - st = status.NewNotFound(ctx, "path not found when purging recycle item") - case errtypes.PermissionDenied: - st = status.NewPermissionDenied(ctx, err, "permission denied") - default: - st = status.NewInternal(ctx, "error purging recycle item") - } + st := status.NewStatusFromErrType(ctx, "error purging recycle item", err) appctx.GetLogger(ctx). Error(). Err(err). @@ -1048,15 +1040,7 @@ func (s *service) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleRe } } else if err := s.storage.EmptyRecycle(ctx, req.Ref); err != nil { // otherwise try emptying the whole recycle bin - var st *rpc.Status - switch err.(type) { - case errtypes.IsNotFound: - st = status.NewNotFound(ctx, "path not found when purging recycle bin") - case errtypes.PermissionDenied: - st = status.NewPermissionDenied(ctx, err, "permission denied") - default: - st = status.NewInternal(ctx, "error purging recycle bin") - } + st := status.NewStatusFromErrType(ctx, "error emptying recycle", err) appctx.GetLogger(ctx). Error(). Err(err). diff --git a/pkg/rgrpc/status/status.go b/pkg/rgrpc/status/status.go index ec4bcdf808..7f519eea90 100644 --- a/pkg/rgrpc/status/status.go +++ b/pkg/rgrpc/status/status.go @@ -135,16 +135,16 @@ func NewStatusFromErrType(ctx context.Context, msg string, err error) *rpc.Statu case nil: NewOK(ctx) case errtypes.IsNotFound: - return NewNotFound(ctx, "gateway: "+msg+": "+err.Error()) + return NewNotFound(ctx, msg+": "+err.Error()) case errtypes.IsInvalidCredentials: // TODO this maps badly - return NewUnauthenticated(ctx, err, "gateway: "+msg+": "+err.Error()) + return NewUnauthenticated(ctx, err, msg+": "+err.Error()) case errtypes.PermissionDenied: - return NewPermissionDenied(ctx, e, "gateway: "+msg+": "+err.Error()) + return NewPermissionDenied(ctx, e, msg+": "+err.Error()) case errtypes.IsNotSupported: - return NewUnimplemented(ctx, err, "gateway: "+msg+":"+err.Error()) + return NewUnimplemented(ctx, err, msg+":"+err.Error()) case errtypes.BadRequest: - return NewInvalidArg(ctx, "gateway: "+msg+":"+err.Error()) + return NewInvalidArg(ctx, msg+":"+err.Error()) } // map GRPC status codes coming from the auth middleware @@ -154,11 +154,11 @@ func NewStatusFromErrType(ctx context.Context, msg string, err error) *rpc.Statu if ok { switch st.Code() { case codes.NotFound: - return NewNotFound(ctx, "gateway: "+msg+": "+err.Error()) + return NewNotFound(ctx, msg+": "+err.Error()) case codes.Unauthenticated: - return NewUnauthenticated(ctx, err, "gateway: "+msg+": "+err.Error()) + return NewUnauthenticated(ctx, err, msg+": "+err.Error()) case codes.PermissionDenied: - return NewPermissionDenied(ctx, err, "gateway: "+msg+": "+err.Error()) + return NewPermissionDenied(ctx, err, msg+": "+err.Error()) } } // the actual error can be wrapped multiple times @@ -168,7 +168,7 @@ func NewStatusFromErrType(ctx context.Context, msg string, err error) *rpc.Statu } } - return NewInternal(ctx, "gateway: "+msg+":"+err.Error()) + return NewInternal(ctx, msg+":"+err.Error()) } // NewErrorFromCode returns a standardized Error for a given RPC code.