Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate using errors.New and fmt.Errorf (#1673) #1680

Merged
merged 4 commits into from
May 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/unreleased/fdeprecate-errors-new-fmt-errorf.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Enhancement: Deprecate using errors.New and fmt.Errorf

Previously we were using errors.New and fmt.Errorf to create errors.
Now we use the errors defined in the errtypes package.

https://github.com/cs3org/reva/issues/1673
8 changes: 4 additions & 4 deletions internal/grpc/interceptors/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ package auth

import (
"context"
"fmt"

userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/token"
tokenmgr "github.com/cs3org/reva/pkg/token/manager/registry"
"github.com/cs3org/reva/pkg/user"
Expand Down Expand Up @@ -67,7 +67,7 @@ func NewUnary(m map[string]interface{}, unprotected []string) (grpc.UnaryServerI

h, ok := tokenmgr.NewFuncs[conf.TokenManager]
if !ok {
return nil, errors.New("auth: token manager does not exist: " + conf.TokenManager)
return nil, errtypes.NotFound("auth: token manager does not exist: " + conf.TokenManager)
}

tokenManager, err := h(conf.TokenManagers[conf.TokenManager])
Expand Down Expand Up @@ -140,12 +140,12 @@ func NewStream(m map[string]interface{}, unprotected []string) (grpc.StreamServe

h, ok := tokenmgr.NewFuncs[conf.TokenManager]
if !ok {
return nil, fmt.Errorf("auth: token manager not found: %s", conf.TokenManager)
return nil, errtypes.NotFound("auth: token manager not found: " + conf.TokenManager)
}

tokenManager, err := h(conf.TokenManagers[conf.TokenManager])
if err != nil {
return nil, errors.New("auth: token manager not found: " + conf.TokenManager)
return nil, errtypes.NotFound("auth: token manager not found: " + conf.TokenManager)
}

interceptor := func(srv interface{}, ss grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
Expand Down
7 changes: 4 additions & 3 deletions internal/grpc/services/appprovider/appprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/cs3org/reva/pkg/app"
"github.com/cs3org/reva/pkg/app/provider/demo"
"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rgrpc"
"github.com/cs3org/reva/pkg/rgrpc/status"
"github.com/cs3org/reva/pkg/rhttp"
Expand Down Expand Up @@ -109,7 +110,7 @@ func getProvider(c *config) (app.Provider, error) {
case "demo":
return demo.New(c.Demo)
default:
return nil, fmt.Errorf("driver not found: %s", c.Driver)
return nil, errtypes.NotFound("driver not found: " + c.Driver)
}
}

Expand All @@ -131,7 +132,7 @@ func (s *service) getWopiAppEndpoints(ctx context.Context) (map[string]interface
}
defer appsRes.Body.Close()
if appsRes.StatusCode != http.StatusOK {
return nil, fmt.Errorf("Request to WOPI server returned %d", appsRes.StatusCode)
return nil, errtypes.InternalError(fmt.Sprintf("Request to WOPI server returned %d", appsRes.StatusCode))
}
appsBody, err := ioutil.ReadAll(appsRes.Body)
if err != nil {
Expand Down Expand Up @@ -263,7 +264,7 @@ func (s *service) OpenFileInAppProvider(ctx context.Context, req *providerpb.Ope
}
defer bridgeRes.Body.Close()
if bridgeRes.StatusCode != http.StatusFound {
return nil, fmt.Errorf("Request to WOPI bridge returned %d", bridgeRes.StatusCode)
return nil, errtypes.InternalError(fmt.Sprintf("Request to WOPI bridge returned %d", bridgeRes.StatusCode))
}
appProviderURL = bridgeRes.Header.Get("Location")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/appregistry/appregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ package appregistry

import (
"context"
"fmt"

"google.golang.org/grpc"

registrypb "github.com/cs3org/go-cs3apis/cs3/app/registry/v1beta1"
"github.com/cs3org/reva/pkg/app"
"github.com/cs3org/reva/pkg/app/registry/static"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rgrpc"
"github.com/cs3org/reva/pkg/rgrpc/status"
"github.com/mitchellh/mapstructure"
Expand Down Expand Up @@ -90,7 +90,7 @@ func getRegistry(c *config) (app.Registry, error) {
case "static":
return static.New(c.Static)
default:
return nil, fmt.Errorf("driver not found: %s", c.Driver)
return nil, errtypes.NotFound("driver not found: " + c.Driver)
}
}

Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/appregistry/appregistry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,12 @@ func TestNew(t *testing.T) {
{
name: "not existing driver",
m: map[string]interface{}{"Driver": "doesnotexist"},
wantErr: "driver not found: doesnotexist",
wantErr: "error: not found: driver not found: doesnotexist",
},
{
name: "empty",
m: map[string]interface{}{},
wantErr: "driver not found: ",
wantErr: "error: not found: driver not found: ",
},
{
name: "extra not existing field in setting",
Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/authprovider/authprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ func parseConfig(m map[string]interface{}) (*config, error) {

func getAuthManager(manager string, m map[string]map[string]interface{}) (auth.Manager, error) {
if manager == "" {
return nil, errors.New("authsvc: driver not configured for auth manager")
return nil, errtypes.InternalError("authsvc: driver not configured for auth manager")
}
if f, ok := registry.NewFuncs[manager]; ok {
return f(m[manager])
}
return nil, fmt.Errorf("authsvc: driver %s not found for auth manager", manager)
return nil, errtypes.NotFound(fmt.Sprintf("authsvc: driver %s not found for auth manager", manager))
}

// New returns a new AuthProviderServiceServer.
Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/authregistry/authregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ package authregistry

import (
"context"
"fmt"

registrypb "github.com/cs3org/go-cs3apis/cs3/auth/registry/v1beta1"
"github.com/cs3org/reva/pkg/auth"
"github.com/cs3org/reva/pkg/auth/registry/registry"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rgrpc"
"github.com/cs3org/reva/pkg/rgrpc/status"
"github.com/mitchellh/mapstructure"
Expand Down Expand Up @@ -98,7 +98,7 @@ func getRegistry(c *config) (auth.Registry, error) {
if f, ok := registry.NewFuncs[c.Driver]; ok {
return f(c.Drivers[c.Driver])
}
return nil, fmt.Errorf("authregistrysvc: driver not found: %s", c.Driver)
return nil, errtypes.NotFound("authregistrysvc: driver not found: " + c.Driver)
}

func (s *service) ListAuthProviders(ctx context.Context, req *registrypb.ListAuthProvidersRequest) (*registrypb.ListAuthProvidersResponse, error) {
Expand Down
7 changes: 4 additions & 3 deletions internal/grpc/services/datatx/datatx.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"context"

datatx "github.com/cs3org/go-cs3apis/cs3/tx/v1beta1"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rgrpc"
"github.com/cs3org/reva/pkg/rgrpc/status"
"github.com/mitchellh/mapstructure"
Expand Down Expand Up @@ -82,18 +83,18 @@ func (s *service) UnprotectedEndpoints() []string {

func (s *service) CreateTransfer(ctx context.Context, req *datatx.CreateTransferRequest) (*datatx.CreateTransferResponse, error) {
return &datatx.CreateTransferResponse{
Status: status.NewUnimplemented(ctx, errors.New("CreateTransfer not implemented"), "CreateTransfer not implemented"),
Status: status.NewUnimplemented(ctx, errtypes.NotSupported("CreateTransfer not implemented"), "CreateTransfer not implemented"),
}, nil
}

func (s *service) GetTransferStatus(ctx context.Context, in *datatx.GetTransferStatusRequest) (*datatx.GetTransferStatusResponse, error) {
return &datatx.GetTransferStatusResponse{
Status: status.NewUnimplemented(ctx, errors.New("GetTransferStatus not implemented"), "GetTransferStatus not implemented"),
Status: status.NewUnimplemented(ctx, errtypes.NotSupported("GetTransferStatus not implemented"), "GetTransferStatus not implemented"),
}, nil
}

func (s *service) CancelTransfer(ctx context.Context, in *datatx.CancelTransferRequest) (*datatx.CancelTransferResponse, error) {
return &datatx.CancelTransferResponse{
Status: status.NewUnimplemented(ctx, errors.New("CancelTransfer not implemented"), "CancelTransfer not implemented"),
Status: status.NewUnimplemented(ctx, errtypes.NotSupported("CancelTransfer not implemented"), "CancelTransfer not implemented"),
}, nil
}
4 changes: 2 additions & 2 deletions internal/grpc/services/gateway/appprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func (s *svc) openLocalResources(ctx context.Context, ri *storageprovider.Resour
accessToken, ok := token.ContextGetToken(ctx)
if !ok || accessToken == "" {
return &providerpb.OpenFileInAppProviderResponse{
Status: status.NewUnauthenticated(ctx, errors.New("Access token is invalid or empty"), ""),
Status: status.NewUnauthenticated(ctx, errtypes.InvalidCredentials("Access token is invalid or empty"), ""),
}, nil
}

Expand Down Expand Up @@ -246,7 +246,7 @@ func (s *svc) findAppProvider(ctx context.Context, ri *storageprovider.ResourceI
return nil, errtypes.NotFound("gateway: app provider not found for resource: " + ri.String())
}

return nil, errors.New("gateway: error finding a storage provider")
return nil, errtypes.InternalError("gateway: error finding a storage provider")
}

func getGRPCConfig(opaque *typespb.Opaque) (bool, bool) {
Expand Down
8 changes: 4 additions & 4 deletions internal/grpc/services/gateway/authprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest
// find auth provider
c, err := s.findAuthProvider(ctx, req.Type)
if err != nil {
err = errors.New("gateway: error finding auth provider for type: " + req.Type)
err = errtypes.NotFound("gateway: error finding auth provider for type: " + req.Type)
return &gateway.AuthenticateResponse{
Status: status.NewInternal(ctx, err, "error getting auth provider client"),
}, nil
Expand Down Expand Up @@ -77,7 +77,7 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest

// validate valid userId
if res.User == nil {
err := errors.New("gateway: user after Authenticate is nil")
err := errtypes.NotFound("gateway: user after Authenticate is nil")
log.Err(err).Msg("user is nil")
return &gateway.AuthenticateResponse{
Status: status.NewInternal(ctx, err, "user is nil"),
Expand All @@ -86,7 +86,7 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest

uid := res.User.Id
if uid == nil {
err := errors.New("gateway: uid after Authenticate is nil")
err := errtypes.NotFound("gateway: uid after Authenticate is nil")
log.Err(err).Msg("user id is nil")
return &gateway.AuthenticateResponse{
Status: status.NewInternal(ctx, err, "user id is nil"),
Expand Down Expand Up @@ -191,5 +191,5 @@ func (s *svc) findAuthProvider(ctx context.Context, authType string) (provider.P
return nil, errtypes.NotFound("gateway: auth provider not found for type:" + authType)
}

return nil, errors.New("gateway: error finding an auth provider for type: " + authType)
return nil, errtypes.InternalError("gateway: error finding an auth provider for type: " + authType)
}
3 changes: 2 additions & 1 deletion internal/grpc/services/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"

"github.com/ReneKroon/ttlcache/v2"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rgrpc"
"github.com/cs3org/reva/pkg/sharedconf"
"github.com/cs3org/reva/pkg/token"
Expand Down Expand Up @@ -177,5 +178,5 @@ func getTokenManager(manager string, m map[string]map[string]interface{}) (token
return f(m[manager])
}

return nil, fmt.Errorf("driver %s not found for token manager", manager)
return nil, errtypes.NotFound(fmt.Sprintf("driver %s not found for token manager", manager))
}
4 changes: 2 additions & 2 deletions internal/grpc/services/gateway/ocmshareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func (s *svc) UpdateReceivedOCMShare(ctx context.Context, req *ocm.UpdateReceive
}

// TODO(labkode): implementing updating display name
err = errors.New("gateway: update of display name is not yet implemented")
err = errtypes.NotSupported("gateway: update of display name is not yet implemented")
return &ocm.UpdateReceivedOCMShareResponse{
Status: status.NewUnimplemented(ctx, err, "error updating received share"),
}, nil
Expand Down Expand Up @@ -304,7 +304,7 @@ func (s *svc) createWebdavReference(ctx context.Context, share *ocm.Share) (*rpc
case "plain":
token = string(tokenOpaque.Value)
default:
err := errors.New("opaque entry decoder not recognized: " + tokenOpaque.Decoder)
err := errtypes.NotSupported("opaque entry decoder not recognized: " + tokenOpaque.Decoder)
return status.NewInternal(ctx, err, "invalid opaque entry decoder"), nil
}

Expand Down
3 changes: 2 additions & 1 deletion internal/grpc/services/gateway/publicshareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ import (
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
"github.com/pkg/errors"
)

func (s *svc) CreatePublicShare(ctx context.Context, req *link.CreatePublicShareRequest) (*link.CreatePublicShareResponse, error) {
if s.isSharedFolder(ctx, req.ResourceInfo.GetPath()) {
return nil, errors.New("gateway: can't create a public share of the share folder itself")
return nil, errtypes.AlreadyExists("gateway: can't create a public share of the share folder itself")
}

log := appctx.GetLogger(ctx)
Expand Down
16 changes: 8 additions & 8 deletions internal/grpc/services/gateway/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func (s *svc) InitiateFileDownload(ctx context.Context, req *provider.InitiateFi
}

if statRes.Info.Type != provider.ResourceType_RESOURCE_TYPE_REFERENCE {
err := errors.New(fmt.Sprintf("gateway: expected reference: got:%+v", statRes.Info))
err := errtypes.BadRequest(fmt.Sprintf("gateway: expected reference: got:%+v", statRes.Info))
log.Err(err).Msg("gateway: error stating share name")
return &gateway.InitiateFileDownloadResponse{
Status: status.NewInternal(ctx, err, "gateway: error initiating download"),
Expand Down Expand Up @@ -456,7 +456,7 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile
}

if statRes.Info.Type != provider.ResourceType_RESOURCE_TYPE_REFERENCE {
err := errors.New(fmt.Sprintf("gateway: expected reference: got:%+v", statRes.Info))
err := errtypes.BadRequest(fmt.Sprintf("gateway: expected reference: got:%+v", statRes.Info))
log.Err(err).Msg("gateway: error stating share name")
return &gateway.InitiateFileUploadResponse{
Status: status.NewInternal(ctx, err, "gateway: error initiating upload"),
Expand Down Expand Up @@ -1387,7 +1387,7 @@ func (s *svc) checkRef(ctx context.Context, ri *provider.ResourceInfo) (*provide
case "webdav":
return nil, "webdav", nil
default:
err := errors.New("gateway: no reference handler for scheme: " + uri.Scheme)
err := errtypes.BadRequest("gateway: no reference handler for scheme: " + uri.Scheme)
return nil, "", err
}
}
Expand Down Expand Up @@ -1429,12 +1429,12 @@ func (s *svc) handleCS3Ref(ctx context.Context, opaque string) (*provider.Resour
case rpc.Code_CODE_UNIMPLEMENTED:
return nil, errtypes.NotSupported(req.Ref.String())
default:
return nil, errors.New("gateway: error stating target reference")
return nil, errtypes.InternalError("gateway: error stating target reference")
}
}

if res.Info.Type == provider.ResourceType_RESOURCE_TYPE_REFERENCE {
err := errors.New("gateway: error the target of a reference cannot be another reference")
err := errtypes.BadRequest("gateway: error the target of a reference cannot be another reference")
return nil, err
}

Expand Down Expand Up @@ -1903,7 +1903,7 @@ func (s *svc) getSharedFolder(ctx context.Context) string {

func (s *svc) CreateSymlink(ctx context.Context, req *provider.CreateSymlinkRequest) (*provider.CreateSymlinkResponse, error) {
return &provider.CreateSymlinkResponse{
Status: status.NewUnimplemented(ctx, errors.New("CreateSymlink not implemented"), "CreateSymlink not implemented"),
Status: status.NewUnimplemented(ctx, errtypes.NotSupported("CreateSymlink not implemented"), "CreateSymlink not implemented"),
}, nil
}

Expand Down Expand Up @@ -1940,7 +1940,7 @@ func (s *svc) RestoreFileVersion(ctx context.Context, req *provider.RestoreFileV
}

func (s *svc) ListRecycleStream(_ *gateway.ListRecycleStreamRequest, _ gateway.GatewayAPI_ListRecycleStreamServer) error {
return errors.New("Unimplemented")
return errtypes.NotSupported("ListRecycleStream unimplemented")
}

// TODO use the ListRecycleRequest.Ref to only list the trash of a specific storage
Expand Down Expand Up @@ -2083,7 +2083,7 @@ func (s *svc) findProviders(ctx context.Context, ref *provider.Reference) ([]*re
}

if res.Providers == nil {
return nil, errors.New("gateway: provider is nil")
return nil, errtypes.NotFound("gateway: provider is nil")
}

return res.Providers, nil
Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/gateway/usershareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, error) {

if s.isSharedFolder(ctx, req.ResourceInfo.GetPath()) {
return nil, errors.New("gateway: can't share the share folder itself")
return nil, errtypes.AlreadyExists("gateway: can't share the share folder itself")
}

c, err := pool.GetUserShareProviderClient(s.c.UserShareProviderEndpoint)
Expand Down Expand Up @@ -344,7 +344,7 @@ func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.Update
}

// TODO(labkode): implementing updating display name
err = errors.New("gateway: update of display name is not yet implemented")
err = errtypes.NotSupported("gateway: update of display name is not yet implemented")
return &collaboration.UpdateReceivedShareResponse{
Status: status.NewUnimplemented(ctx, err, "error updating received share"),
}, nil
Expand Down
2 changes: 1 addition & 1 deletion internal/grpc/services/gateway/webdavstorageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (s *svc) webdavRefTransferEndpoint(ctx context.Context, targetURL string, n

func (s *svc) extractEndpointInfo(ctx context.Context, targetURL string) (*webdavEndpoint, error) {
if targetURL == "" {
return nil, errors.New("gateway: ref target is an empty uri")
return nil, errtypes.BadRequest("gateway: ref target is an empty uri")
}

uri, err := url.Parse(targetURL)
Expand Down
3 changes: 2 additions & 1 deletion internal/grpc/services/groupprovider/groupprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"

grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/group"
"github.com/cs3org/reva/pkg/group/manager/registry"
"github.com/cs3org/reva/pkg/rgrpc"
Expand Down Expand Up @@ -62,7 +63,7 @@ func getDriver(c *config) (group.Manager, error) {
return f(c.Drivers[c.Driver])
}

return nil, fmt.Errorf("driver %s not found for group manager", c.Driver)
return nil, errtypes.NotFound(fmt.Sprintf("driver %s not found for group manager", c.Driver))
}

// New returns a new GroupProviderServiceServer.
Expand Down
Loading