Refactoring of the error responses and of the logging around them (cs…

…3org#225)

ADOPTERS.md

@@ -6,4 +6,4 @@ If you'd like to be include here, please send a pull request modifying this file
 We intend to use this list to contact you for surveys and for sending invitations to events.
 We will also point to this list if we are asked who uses REVA.
 
-We will not use any of the information here for protomotios nor to send other regular communications.
+We will not use any of the information here for promotions nor to send other regular communications.

CONTRIBUTORS.md

@@ -1,6 +1,7 @@
 - Alex Unger <[email protected]>
 - Diogo Castro <[email protected]>
 - Felix Hillingshaeuser <[email protected]>
+- Giuseppe Lo Presti <[email protected]>
 - Hugo Gonzalez Labrador <[email protected]>
 - Jörn Friedrich Dreyer <[email protected]>
 - Mohitty <[email protected]>

cmd/revad/svcs/grpcsvcs/appprovidersvc/appprovidersvc.go

@@ -24,12 +24,12 @@ import (
 	"io"
 
 	appproviderv0alphapb "github.com/cs3org/go-cs3apis/cs3/appprovider/v0alpha"
-	rpcpb "github.com/cs3org/go-cs3apis/cs3/rpc"
 	"github.com/cs3org/reva/cmd/revad/grpcserver"
+	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/status"
 	"github.com/cs3org/reva/pkg/app"
 	"github.com/cs3org/reva/pkg/app/provider/demo"
-	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/mitchellh/mapstructure"
+	"github.com/pkg/errors"
 	"google.golang.org/grpc"
 )
 
@@ -89,19 +89,16 @@ func getProvider(c *config) (app.Provider, error) {
 }
 
 func (s *service) Open(ctx context.Context, req *appproviderv0alphapb.OpenRequest) (*appproviderv0alphapb.OpenResponse, error) {
-	log := appctx.GetLogger(ctx)
-	token := req.AccessToken
-
-	iframeLocation, err := s.provider.GetIFrame(ctx, req.ResourceInfo.Id, token)
+	iframeLocation, err := s.provider.GetIFrame(ctx, req.ResourceInfo.Id, req.AccessToken)
 	if err != nil {
-		log.Error().Err(err).Msg("error getting iframe")
+		err := errors.Wrap(err, "appprovidersvc: error calling GetIFrame")
 		res := &appproviderv0alphapb.OpenResponse{
-			Status: &rpcpb.Status{Code: rpcpb.Code_CODE_INTERNAL},
+			Status: status.NewInternal(ctx, err, "error getting app's iframe"),
 		}
 		return res, nil
 	}
 	res := &appproviderv0alphapb.OpenResponse{
-		Status:    &rpcpb.Status{Code: rpcpb.Code_CODE_OK},
+		Status:    status.NewOK(ctx),
 		IframeUrl: iframeLocation,
 	}
 	return res, nil

cmd/revad/svcs/grpcsvcs/appregistrysvc/appregistrysvc.go

@@ -23,14 +23,13 @@ import (
 	"fmt"
 	"io"
 
-	rpcpb "github.com/cs3org/go-cs3apis/cs3/rpc"
 	"google.golang.org/grpc"
 
 	appregistryv0alphapb "github.com/cs3org/go-cs3apis/cs3/appregistry/v0alpha"
 	"github.com/cs3org/reva/cmd/revad/grpcserver"
+	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/status"
 	"github.com/cs3org/reva/pkg/app"
 	"github.com/cs3org/reva/pkg/app/registry/static"
-	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/mitchellh/mapstructure"
 )
 
@@ -90,19 +89,16 @@ func getRegistry(c *config) (app.Registry, error) {
 }
 
 func (s *svc) GetAppProviders(ctx context.Context, req *appregistryv0alphapb.GetAppProvidersRequest) (*appregistryv0alphapb.GetAppProvidersResponse, error) {
-	log := appctx.GetLogger(ctx)
 	p, err := s.registry.FindProvider(ctx, req.ResourceInfo.MimeType)
 	if err != nil {
-		log.Error().Err(err).Msg("error sending grpc find provider request")
-		res := &appregistryv0alphapb.GetAppProvidersResponse{
-			Status: &rpcpb.Status{Code: rpcpb.Code_CODE_INTERNAL},
-		}
-		return res, nil
+		return &appregistryv0alphapb.GetAppProvidersResponse{
+			Status: status.NewInternal(ctx, err, "error looking for the app provider"),
+		}, nil
 	}
 
 	provider := format(p)
 	res := &appregistryv0alphapb.GetAppProvidersResponse{
-		Status:    &rpcpb.Status{Code: rpcpb.Code_CODE_OK},
+		Status:    status.NewOK(ctx),
 		Providers: []*appregistryv0alphapb.ProviderInfo{provider},
 	}
 	return res, nil
@@ -111,18 +107,17 @@ func (s *svc) GetAppProviders(ctx context.Context, req *appregistryv0alphapb.Get
 func (s *svc) ListAppProviders(ctx context.Context, req *appregistryv0alphapb.ListAppProvidersRequest) (*appregistryv0alphapb.ListAppProvidersResponse, error) {
 	pvds, err := s.registry.ListProviders(ctx)
 	if err != nil {
-		res := &appregistryv0alphapb.ListAppProvidersResponse{
-			Status: &rpcpb.Status{Code: rpcpb.Code_CODE_INTERNAL},
-		}
-		return res, nil
+		return &appregistryv0alphapb.ListAppProvidersResponse{
+			Status: status.NewInternal(ctx, err, "error listing the app providers"),
+		}, nil
 	}
 	providers := make([]*appregistryv0alphapb.ProviderInfo, 0, len(pvds))
 	for _, pvd := range pvds {
 		providers = append(providers, format(pvd))
 	}
 
 	res := &appregistryv0alphapb.ListAppProvidersResponse{
-		Status:    &rpcpb.Status{Code: rpcpb.Code_CODE_OK},
+		Status:    status.NewOK(ctx),
 		Providers: providers,
 	}
 	return res, nil

cmd/revad/svcs/grpcsvcs/authsvc/authsvc.go

@@ -24,6 +24,7 @@ import (
 	"io"
 
 	"github.com/cs3org/reva/cmd/revad/grpcserver"
+	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/status"
 	"github.com/cs3org/reva/pkg/auth/manager/registry"
 	tokenmgr "github.com/cs3org/reva/pkg/token/manager/registry"
 	usermgr "github.com/cs3org/reva/pkg/user/manager/registry"
@@ -34,7 +35,6 @@ import (
 	"github.com/cs3org/reva/pkg/user"
 
 	authv0alphapb "github.com/cs3org/go-cs3apis/cs3/auth/v0alpha"
-	rpcpb "github.com/cs3org/go-cs3apis/cs3/rpc"
 
 	typespb "github.com/cs3org/go-cs3apis/cs3/types"
 	"github.com/cs3org/reva/pkg/appctx"
@@ -134,49 +134,51 @@ func (s *service) GenerateAccessToken(ctx context.Context, req *authv0alphapb.Ge
 
 	ctx, err := s.authmgr.Authenticate(ctx, username, password)
 	if err != nil {
-		log.Error().Err(err).Msg("error authenticating user")
-		status := &rpcpb.Status{Code: rpcpb.Code_CODE_UNAUTHENTICATED}
-		res := &authv0alphapb.GenerateAccessTokenResponse{Status: status}
+		err = errors.Wrap(err, "authsvc: error in Authenticate")
+		res := &authv0alphapb.GenerateAccessTokenResponse{
+			Status: status.NewUnauthenticated(ctx, err, "error authenticating user"),
+		}
 		return res, nil
 	}
 
 	user, err := s.usermgr.GetUser(ctx, uid)
 	if err != nil {
-		log.Error().Err(err).Msg("error getting user information")
-		status := &rpcpb.Status{Code: rpcpb.Code_CODE_UNAUTHENTICATED}
-		res := &authv0alphapb.GenerateAccessTokenResponse{Status: status}
+		err = errors.Wrap(err, "authsvc: error in GetUser")
+		res := &authv0alphapb.GenerateAccessTokenResponse{
+			Status: status.NewUnauthenticated(ctx, err, "error getting user information"),
+		}
 		return res, nil
 	}
 
 	accessToken, err := s.tokenmgr.MintToken(ctx, user)
 	if err != nil {
-		err = errors.Wrap(err, "error creating access token")
-		log.Error().Err(err).Msg("error creating access token")
-		status := &rpcpb.Status{Code: rpcpb.Code_CODE_UNAUTHENTICATED}
-		res := &authv0alphapb.GenerateAccessTokenResponse{Status: status}
+		err = errors.Wrap(err, "authsvc: error in MintToken")
+		res := &authv0alphapb.GenerateAccessTokenResponse{
+			Status: status.NewUnauthenticated(ctx, err, "error creating access token"),
+		}
 		return res, nil
 	}
 
 	log.Info().Msgf("user %s authenticated", user.Username)
-	status := &rpcpb.Status{Code: rpcpb.Code_CODE_OK}
-	res := &authv0alphapb.GenerateAccessTokenResponse{Status: status, AccessToken: accessToken}
+	res := &authv0alphapb.GenerateAccessTokenResponse{
+		Status:      status.NewOK(ctx),
+		AccessToken: accessToken,
+	}
 	return res, nil
-
 }
 
 func (s *service) WhoAmI(ctx context.Context, req *authv0alphapb.WhoAmIRequest) (*authv0alphapb.WhoAmIResponse, error) {
-	log := appctx.GetLogger(ctx)
-	token := req.AccessToken
-	u, err := s.tokenmgr.DismantleToken(ctx, token)
+	u, err := s.tokenmgr.DismantleToken(ctx, req.AccessToken)
 	if err != nil {
-		err = errors.Wrap(err, "error getting user from access token")
-		log.Error().Err(err).Msg("error dismantling access token")
-		status := &rpcpb.Status{Code: rpcpb.Code_CODE_UNAUTHENTICATED}
-		res := &authv0alphapb.WhoAmIResponse{Status: status}
-		return res, nil
+		err = errors.Wrap(err, "authsvc: error getting user from access token")
+		return &authv0alphapb.WhoAmIResponse{
+			Status: status.NewUnauthenticated(ctx, err, "error dismantling access token"),
+		}, nil
 	}
 
-	status := &rpcpb.Status{Code: rpcpb.Code_CODE_OK}
-	res := &authv0alphapb.WhoAmIResponse{Status: status, User: u}
+	res := &authv0alphapb.WhoAmIResponse{
+		Status: status.NewOK(ctx),
+		User:   u,
+	}
 	return res, nil
 }

cmd/revad/svcs/grpcsvcs/gatewaysvc/appprovidersvc.go

@@ -26,43 +26,38 @@ import (
 	rpcpb "github.com/cs3org/go-cs3apis/cs3/rpc"
 	storageproviderv0alphapb "github.com/cs3org/go-cs3apis/cs3/storageprovider/v0alpha"
 	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/pool"
-	"github.com/cs3org/reva/pkg/appctx"
+	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/status"
 	"github.com/cs3org/reva/pkg/errtypes"
 	"github.com/pkg/errors"
 )
 
 func (s *svc) Open(ctx context.Context, req *appproviderv0alphapb.OpenRequest) (*appproviderv0alphapb.OpenResponse, error) {
-	log := appctx.GetLogger(ctx)
 	provider, err := s.findAppProvider(ctx, req.ResourceInfo)
 	if err != nil {
-		log.Err(err).Msg("gatewaysvc: error finding app provider")
+		err = errors.Wrap(err, "gatewaysvc: error calling findAppProvider")
+		var st *rpcpb.Status
 		if _, ok := err.(errtypes.IsNotFound); ok {
-			return &appproviderv0alphapb.OpenResponse{
-				Status: &rpcpb.Status{
-					Code: rpcpb.Code_CODE_NOT_FOUND,
-				},
-			}, nil
+			st = status.NewNotFound(ctx, "app provider not found")
+		} else {
+			st = status.NewInternal(ctx, err, "error searching for app provider")
 		}
+
 		return &appproviderv0alphapb.OpenResponse{
-			Status: &rpcpb.Status{
-				Code: rpcpb.Code_CODE_INTERNAL,
-			},
+			Status: st,
 		}, nil
 	}
 
 	c, err := pool.GetAppProviderClient(provider.Address)
 	if err != nil {
-		log.Err(err).Msg("gatewaysvc: error getting appprovider client")
+		err = errors.Wrap(err, "gatewaysvc: error calling GetAppProviderClient")
 		return &appproviderv0alphapb.OpenResponse{
-			Status: &rpcpb.Status{
-				Code: rpcpb.Code_CODE_INTERNAL,
-			},
+			Status: status.NewInternal(ctx, err, "error getting appprovider client"),
 		}, nil
 	}
 
 	res, err := c.Open(ctx, req)
 	if err != nil {
-		return nil, errors.Wrap(err, "gatewaysvc: error calling Open")
+		return nil, errors.Wrap(err, "gatewaysvc: error calling c.Open")
 	}
 
 	return res, nil
@@ -91,7 +86,7 @@ func (s *svc) findAppProvider(ctx context.Context, ri *storageproviderv0alphapb.
 	}
 
 	if res.Status.Code == rpcpb.Code_CODE_NOT_FOUND {
-		return nil, errtypes.NotFound("gatewaysvc: app provider not found for resource:" + ri.String())
+		return nil, errtypes.NotFound("gatewaysvc: app provider not found for resource: " + ri.String())
 	}
 
 	return nil, errors.New("gatewaysvc: error finding a storage provider")

cmd/revad/svcs/grpcsvcs/gatewaysvc/appregistry.go

@@ -22,22 +22,16 @@ import (
 	"context"
 
 	appregistryv0alphapb "github.com/cs3org/go-cs3apis/cs3/appregistry/v0alpha"
-	rpcpb "github.com/cs3org/go-cs3apis/cs3/rpc"
 	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/pool"
-	"github.com/cs3org/reva/pkg/appctx"
+	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/status"
 	"github.com/pkg/errors"
 )
 
 func (s *svc) GetAppProviders(ctx context.Context, req *appregistryv0alphapb.GetAppProvidersRequest) (*appregistryv0alphapb.GetAppProvidersResponse, error) {
-	log := appctx.GetLogger(ctx)
-
 	c, err := pool.GetAppRegistryClient(s.c.AppRegistryEndpoint)
 	if err != nil {
-		log.Err(err).Msg("gatewaysvc: error getting appregistry client")
 		return &appregistryv0alphapb.GetAppProvidersResponse{
-			Status: &rpcpb.Status{
-				Code: rpcpb.Code_CODE_INTERNAL,
-			},
+			Status: status.NewInternal(ctx, err, "error getting app registry client"),
 		}, nil
 	}
 
@@ -50,15 +44,10 @@ func (s *svc) GetAppProviders(ctx context.Context, req *appregistryv0alphapb.Get
 }
 
 func (s *svc) ListAppProviders(ctx context.Context, req *appregistryv0alphapb.ListAppProvidersRequest) (*appregistryv0alphapb.ListAppProvidersResponse, error) {
-	log := appctx.GetLogger(ctx)
-
 	c, err := pool.GetAppRegistryClient(s.c.AppRegistryEndpoint)
 	if err != nil {
-		log.Err(err).Msg("gatewaysvc: error getting appregistry client")
 		return &appregistryv0alphapb.ListAppProvidersResponse{
-			Status: &rpcpb.Status{
-				Code: rpcpb.Code_CODE_INTERNAL,
-			},
+			Status: status.NewInternal(ctx, err, "error getting app registry client"),
 		}, nil
 	}
 

cmd/revad/svcs/grpcsvcs/gatewaysvc/authsvc.go

@@ -22,22 +22,16 @@ import (
 	"context"
 
 	authv0alphapb "github.com/cs3org/go-cs3apis/cs3/auth/v0alpha"
-	rpcpb "github.com/cs3org/go-cs3apis/cs3/rpc"
 	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/pool"
-	"github.com/cs3org/reva/pkg/appctx"
+	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/status"
 	"github.com/pkg/errors"
 )
 
 func (s *svc) GenerateAccessToken(ctx context.Context, req *authv0alphapb.GenerateAccessTokenRequest) (*authv0alphapb.GenerateAccessTokenResponse, error) {
-	log := appctx.GetLogger(ctx)
-
 	c, err := pool.GetAuthServiceClient(s.c.AuthEndpoint)
 	if err != nil {
-		log.Err(err).Msg("gatewaysvc: error getting auth client")
 		return &authv0alphapb.GenerateAccessTokenResponse{
-			Status: &rpcpb.Status{
-				Code: rpcpb.Code_CODE_INTERNAL,
-			},
+			Status: status.NewInternal(ctx, err, "error getting auth client"),
 		}, nil
 	}
 
@@ -50,15 +44,10 @@ func (s *svc) GenerateAccessToken(ctx context.Context, req *authv0alphapb.Genera
 }
 
 func (s *svc) WhoAmI(ctx context.Context, req *authv0alphapb.WhoAmIRequest) (*authv0alphapb.WhoAmIResponse, error) {
-	log := appctx.GetLogger(ctx)
-
 	c, err := pool.GetAuthServiceClient(s.c.AuthEndpoint)
 	if err != nil {
-		log.Err(err).Msg("gatewaysvc: error getting auth client")
 		return &authv0alphapb.WhoAmIResponse{
-			Status: &rpcpb.Status{
-				Code: rpcpb.Code_CODE_INTERNAL,
-			},
+			Status: status.NewInternal(ctx, err, "error getting auth client"),
 		}, nil
 	}
 

cmd/revad/svcs/grpcsvcs/gatewaysvc/preferencessvc.go

@@ -22,22 +22,17 @@ import (
 	"context"
 
 	preferencesv0alphapb "github.com/cs3org/go-cs3apis/cs3/preferences/v0alpha"
-	rpcpb "github.com/cs3org/go-cs3apis/cs3/rpc"
 	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/pool"
-	"github.com/cs3org/reva/pkg/appctx"
+	"github.com/cs3org/reva/cmd/revad/svcs/grpcsvcs/status"
 	"github.com/pkg/errors"
 )
 
 func (s *svc) SetKey(ctx context.Context, req *preferencesv0alphapb.SetKeyRequest) (*preferencesv0alphapb.SetKeyResponse, error) {
-	log := appctx.GetLogger(ctx)
-
 	c, err := pool.GetPreferencesClient(s.c.PreferencesEndpoint)
 	if err != nil {
-		log.Err(err).Msg("gatewaysvc: error getting preferences client")
+		err = errors.Wrap(err, "gatewaysvc: error calling GetPreferencesClient")
 		return &preferencesv0alphapb.SetKeyResponse{
-			Status: &rpcpb.Status{
-				Code: rpcpb.Code_CODE_INTERNAL,
-			},
+			Status: status.NewInternal(ctx, err, "error getting preferences client"),
 		}, nil
 	}
 
@@ -50,15 +45,11 @@ func (s *svc) SetKey(ctx context.Context, req *preferencesv0alphapb.SetKeyReques
 }
 
 func (s *svc) GetKey(ctx context.Context, req *preferencesv0alphapb.GetKeyRequest) (*preferencesv0alphapb.GetKeyResponse, error) {
-	log := appctx.GetLogger(ctx)
-
 	c, err := pool.GetPreferencesClient(s.c.PreferencesEndpoint)
 	if err != nil {
-		log.Err(err).Msg("gatewaysvc: error getting preferences client")
+		err = errors.Wrap(err, "gatewaysvc: error calling GetPreferencesClient")
 		return &preferencesv0alphapb.GetKeyResponse{
-			Status: &rpcpb.Status{
-				Code: rpcpb.Code_CODE_INTERNAL,
-			},
+			Status: status.NewInternal(ctx, err, "error getting preferences client"),
 		}, nil
 	}