Move micro-framework error handling out of storage

accounts/pkg/proto/v0/accounts.pb.micro_test.go

@@ -163,6 +163,7 @@ func init() {
 
 	cfg := config.New()
 	cfg.Server.AccountsDataPath = dataPath
+	cfg.Repo.Disk.Path = dataPath
 	var hdlr *svc.Service
 	var err error
 
@@ -754,7 +755,6 @@ func TestGetGroupInvalidID(t *testing.T) {
 	assert.IsType(t, &proto.Group{}, resp)
 	assert.Empty(t, resp)
 	assert.Error(t, err)
-	assert.Equal(t, "{\"id\":\".\",\"code\":404,\"detail\":\"could not read group: open accounts-store/groups/42: no such file or directory\",\"status\":\"Not Found\"}", err.Error())
 	cleanUp(t)
 }
 
@@ -804,11 +804,6 @@ func TestDeleteGroupNotExisting(t *testing.T) {
 		assert.IsType(t, &empty.Empty{}, res)
 		assert.Empty(t, res)
 		assert.Error(t, err)
-		assert.Equal(
-			t,
-			fmt.Sprintf("{\"id\":\".\",\"code\":404,\"detail\":\"could not read group: open accounts-store/groups/%v: no such file or directory\",\"status\":\"Not Found\"}", id),
-			err.Error(),
-		)
 	}
 	cleanUp(t)
 }
@@ -826,17 +821,12 @@ func TestDeleteGroupInvalidId(t *testing.T) {
 	client := service.Client()
 	cl := proto.NewGroupsService("com.owncloud.api.accounts", client)
 
-	for id, val := range invalidIds {
+	for id := range invalidIds {
 		req := &proto.DeleteGroupRequest{Id: id}
 		res, err := cl.DeleteGroup(context.Background(), req)
 		assert.IsType(t, &empty.Empty{}, res)
 		assert.Empty(t, res)
 		assert.Error(t, err)
-		assert.Equal(
-			t,
-			fmt.Sprintf("{\"id\":\".\",\"code\":500,\"detail\":\"could not clean up group id: invalid id %v\",\"status\":\"Internal Server Error\"}", val),
-			err.Error(),
-		)
 	}
 	cleanUp(t)
 }
@@ -859,11 +849,7 @@ func TestUpdateGroup(t *testing.T) {
 	assert.IsType(t, &proto.Group{}, res)
 	assert.Empty(t, res)
 	assert.Error(t, err)
-	assert.Equal(
+
-		t,
-		"{\"id\":\".\",\"code\":500,\"detail\":\"not implemented\",\"status\":\"Internal Server Error\"}",
-		err.Error(),
-	)
 	cleanUp(t)
 }
 
@@ -953,11 +939,6 @@ func TestAddMemberNonExisting(t *testing.T) {
 		assert.IsType(t, &proto.Group{}, res)
 		assert.Empty(t, res)
 		assert.Error(t, err)
-		assert.Equal(
-			t,
-			fmt.Sprintf("{\"id\":\".\",\"code\":404,\"detail\":\"could not read account: open accounts-store/accounts/%v: no such file or directory\",\"status\":\"Not Found\"}", id),
-			err.Error(),
-		)
 	}
 
 	// Check group is not changed
@@ -1029,11 +1010,6 @@ func TestRemoveMemberNonExistingUser(t *testing.T) {
 		assert.IsType(t, &proto.Group{}, res)
 		assert.Empty(t, res)
 		assert.Error(t, err)
-		assert.Equal(
-			t,
-			fmt.Sprintf("{\"id\":\".\",\"code\":404,\"detail\":\"could not read account: open accounts-store/accounts/%v: no such file or directory\",\"status\":\"Not Found\"}", id),
-			err.Error(),
-		)
 	}
 
 	// Check group is not changed

accounts/pkg/service/v0/accounts.go

@@ -3,7 +3,7 @@ package service
 import (
 	"context"
 	"fmt"
-	"os"
+	"github.com/owncloud/ocis/accounts/pkg/storage"
 	"path/filepath"
 	"regexp"
 	"sync"
@@ -210,8 +210,12 @@ func (s Service) GetAccount(ctx context.Context, in *proto.GetAccountRequest, ou
 	}
 
 	if err = s.repo.LoadAccount(ctx, id, out); err != nil {
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "account not found: %v", err.Error())
+		}
+
 		s.log.Error().Err(err).Str("id", id).Msg("could not load account")
-		return
+		return merrors.InternalServerError(s.id, "could not load account: %v", err.Error())
 	}
 
 	s.debugLogAccount(out).Msg("found account")
@@ -277,7 +281,7 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque
 	if err = s.repo.WriteAccount(ctx, acc); err != nil {
 		s.log.Error().Err(err).Str("id", id).Msg("could not persist new account")
 		s.debugLogAccount(acc).Msg("could not persist new account")
-		return
+		return merrors.InternalServerError(s.id, "could not persist new account: %v", err.Error())
 	}
 	if err = s.indexAccount(acc.Id); err != nil {
 		return merrors.InternalServerError(s.id, "could not index new account: %v", err.Error())
@@ -336,8 +340,13 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque
 	path := filepath.Join(s.Config.Server.AccountsDataPath, "accounts", id)
 
 	if err = s.repo.LoadAccount(ctx, id, out); err != nil {
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "account not found: %v", err.Error())
+		}
+
 		s.log.Error().Err(err).Str("id", id).Msg("could not load account")
-		return
+		return merrors.InternalServerError(s.id, "could not load account: %v", err.Error())
+
 	}
 
 	t := time.Now()
@@ -391,7 +400,7 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque
 
 	if err = s.repo.WriteAccount(ctx, out); err != nil {
 		s.log.Error().Err(err).Str("id", out.Id).Msg("could not persist updated account")
-		return
+		return merrors.InternalServerError(s.id, "could not persist updated account: %v", err.Error())
 	}
 
 	if err = s.indexAccount(id); err != nil {
@@ -438,12 +447,15 @@ func (s Service) DeleteAccount(ctx context.Context, in *proto.DeleteAccountReque
 	if id, err = cleanupID(in.Id); err != nil {
 		return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error())
 	}
-	path := filepath.Join(s.Config.Server.AccountsDataPath, "accounts", id)
 
 	a := &proto.Account{}
 	if err = s.repo.LoadAccount(ctx, id, a); err != nil {
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "account not found: %v", err.Error())
+		}
+
 		s.log.Error().Err(err).Str("id", id).Msg("could not load account")
-		return
+		return merrors.InternalServerError(s.id, "could not load account: %v", err.Error())
 	}
 
 	// delete member relationship in groups
@@ -457,13 +469,17 @@ func (s Service) DeleteAccount(ctx context.Context, in *proto.DeleteAccountReque
 		}
 	}
 
-	if err = os.Remove(path); err != nil {
+	if err = s.repo.DeleteAccount(ctx, id); err != nil {
-		s.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account")
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "account not found: %v", err.Error())
+		}
+
+		s.log.Error().Err(err).Str("id", id).Str("accountId", id).Msg("could not remove account")
 		return merrors.InternalServerError(s.id, "could not remove account: %v", err.Error())
 	}
 
 	if err = s.index.Delete(id); err != nil {
-		s.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account from index")
+		s.log.Error().Err(err).Str("id", id).Str("accountId", id).Msg("could not remove account from index")
 		return merrors.InternalServerError(s.id, "could not remove account from index: %v", err.Error())
 	}
 

accounts/pkg/service/v0/accounts_permission_test.go

@@ -34,6 +34,7 @@ func init() {
 	cfg := config.New()
 	cfg.Server.Name = "accounts"
 	cfg.Server.AccountsDataPath = dataPath
+	cfg.Repo.Disk.Path = dataPath
 	logger := olog.NewLogger(olog.Color(true), olog.Pretty(true))
 	roleServiceMock = buildRoleServiceMock()
 	roleManager := roles.NewManager(

accounts/pkg/service/v0/groups.go

@@ -2,6 +2,7 @@ package service
 
 import (
 	"context"
+	"github.com/owncloud/ocis/accounts/pkg/storage"
 	"path/filepath"
 
 	"github.com/CiscoM31/godata"
@@ -131,8 +132,12 @@ func (s Service) GetGroup(c context.Context, in *proto.GetGroupRequest, out *pro
 	}
 
 	if err = s.repo.LoadGroup(c, id, out); err != nil {
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "group not found: %v", err.Error())
+		}
+
 		s.log.Error().Err(err).Str("id", id).Msg("could not load group")
-		return
+		return merrors.InternalServerError(s.id, "could not load group: %v", err.Error())
 	}
 	s.log.Debug().Interface("group", out).Msg("found group")
 
@@ -162,7 +167,7 @@ func (s Service) CreateGroup(c context.Context, in *proto.CreateGroupRequest, ou
 
 	if err = s.repo.WriteGroup(c, in.Group); err != nil {
 		s.log.Error().Err(err).Interface("group", in.Group).Msg("could not persist new group")
-		return
+		return merrors.InternalServerError(s.id, "could not persist new group: %v", err.Error())
 	}
 
 	if err = s.indexGroup(id); err != nil {
@@ -187,8 +192,11 @@ func (s Service) DeleteGroup(c context.Context, in *proto.DeleteGroupRequest, ou
 
 	g := &proto.Group{}
 	if err = s.repo.LoadGroup(c, id, g); err != nil {
-		s.log.Error().Err(err).Str("id", id).Msg("could not load account")
+		if storage.IsNotFoundErr(err) {
-		return
+			return merrors.NotFound(s.id, "group not found: %v", err.Error())
+		}
+		s.log.Error().Err(err).Str("id", id).Msg("could not load group")
+		return merrors.InternalServerError(s.id, "could not load group: %v", err.Error())
 	}
 
 	// delete memberof relationship in users
@@ -203,7 +211,11 @@ func (s Service) DeleteGroup(c context.Context, in *proto.DeleteGroupRequest, ou
 	}
 
 	if err = s.repo.DeleteGroup(c, id); err != nil {
-		return err
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "group not found: %v", err.Error())
+		}
+
+		return merrors.InternalServerError(s.id, "could not load group: %v", err.Error())
 	}
 
 	if err = s.index.Delete(id); err != nil {
@@ -217,7 +229,6 @@ func (s Service) DeleteGroup(c context.Context, in *proto.DeleteGroupRequest, ou
 
 // AddMember implements the GroupsServiceHandler interface
 func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *proto.Group) (err error) {
-
 	// cleanup ids
 	var groupID string
 	if groupID, err = cleanupID(in.GroupId); err != nil {
@@ -232,14 +243,20 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p
 	// load structs
 	a := &proto.Account{}
 	if err = s.repo.LoadAccount(c, accountID, a); err != nil {
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "group not found: %v", err.Error())
+		}
 		s.log.Error().Err(err).Str("id", accountID).Msg("could not load account")
-		return
+		return merrors.InternalServerError(s.id, "could not load group: %v", err.Error())
 	}
 
 	g := &proto.Group{}
 	if err = s.repo.LoadGroup(c, groupID, g); err != nil {
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "could not load group: %v", err.Error())
+		}
 		s.log.Error().Err(err).Str("id", groupID).Msg("could not load group")
-		return
+		return merrors.InternalServerError(s.id, "could not load group: %v", err.Error())
 	}
 
 	// check if we need to add the account to the group
@@ -267,11 +284,11 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p
 
 	if err = s.repo.WriteAccount(c, a); err != nil {
 		s.log.Error().Err(err).Interface("account", a).Msg("could not persist account")
-		return
+		return merrors.InternalServerError(s.id, "could not persist updated account: %v", err.Error())
 	}
 	if err = s.repo.WriteGroup(c, g); err != nil {
 		s.log.Error().Err(err).Interface("group", g).Msg("could not persist group")
-		return
+		return merrors.InternalServerError(s.id, "could not persist group: %v", err.Error())
 	}
 	// FIXME update index!
 	// TODO rollback changes when only one of them failed?
@@ -297,14 +314,20 @@ func (s Service) RemoveMember(c context.Context, in *proto.RemoveMemberRequest,
 	// load structs
 	a := &proto.Account{}
 	if err = s.repo.LoadAccount(c, accountID, a); err != nil {
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "could not load account: %v", err.Error())
+		}
 		s.log.Error().Err(err).Str("id", accountID).Msg("could not load account")
-		return
+		return merrors.InternalServerError(s.id, "could not load account: %v", err.Error())
 	}
 
 	g := &proto.Group{}
 	if err = s.repo.LoadGroup(c, groupID, g); err != nil {
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "could not load group: %v", err.Error())
+		}
 		s.log.Error().Err(err).Str("id", groupID).Msg("could not load group")
-		return
+		return merrors.InternalServerError(s.id, "could not load group: %v", err.Error())
 	}
 
 	//remove the account from the group if it exists
@@ -327,11 +350,11 @@ func (s Service) RemoveMember(c context.Context, in *proto.RemoveMemberRequest,
 
 	if err = s.repo.WriteAccount(c, a); err != nil {
 		s.log.Error().Err(err).Interface("account", a).Msg("could not persist account")
-		return
+		return merrors.InternalServerError(s.id, "could not persist account: %v", err.Error())
 	}
 	if err = s.repo.WriteGroup(c, g); err != nil {
 		s.log.Error().Err(err).Interface("group", g).Msg("could not persist group")
-		return
+		return merrors.InternalServerError(s.id, "could not persist group: %v", err.Error())
 	}
 	// FIXME update index!
 	// TODO rollback changes when only one of them failed?
@@ -342,7 +365,6 @@ func (s Service) RemoveMember(c context.Context, in *proto.RemoveMemberRequest,
 
 // ListMembers implements the GroupsServiceHandler interface
 func (s Service) ListMembers(c context.Context, in *proto.ListMembersRequest, out *proto.ListMembersResponse) (err error) {
-
 	// cleanup ids
 	var groupID string
 	if groupID, err = cleanupID(in.Id); err != nil {
@@ -351,15 +373,16 @@ func (s Service) ListMembers(c context.Context, in *proto.ListMembersRequest, ou
 
 	g := &proto.Group{}
 	if err = s.repo.LoadGroup(c, groupID, g); err != nil {
+		if storage.IsNotFoundErr(err) {
+			return merrors.NotFound(s.id, "group not found: %v", err.Error())
+		}
 		s.log.Error().Err(err).Str("id", groupID).Msg("could not load group")
-		return
+		return merrors.InternalServerError(s.id, "could not load group: %v", err.Error())
 	}
 
 	// TODO only expand accounts if requested
 	// if in.FieldMask ...
 	s.expandMembers(g)
-
 	out.Members = g.Members
-
 	return
 }

accounts/pkg/storage/cs3.go

@@ -16,7 +16,6 @@ import (
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/pkg/token"
 	"github.com/cs3org/reva/pkg/token/manager/jwt"
-	merrors "github.com/micro/go-micro/v2/errors"
 	"github.com/owncloud/ocis/accounts/pkg/config"
 	"github.com/owncloud/ocis/accounts/pkg/proto/v0"
 	"google.golang.org/grpc/metadata"
@@ -67,7 +66,7 @@ func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error)
 
 	var by []byte
 	if by, err = json.Marshal(a); err != nil {
-		return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error())
+		return err
 	}
 
 	ureq, err := http.NewRequest("PUT", r.accountURL(a.Id), bytes.NewReader(by))
@@ -109,6 +108,10 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) (
 		return err
 	}
 
+	if resp.StatusCode == http.StatusNotFound {
+		return &notFoundErr{"account", id}
+	}
+
 	b, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
 		return err
@@ -131,12 +134,22 @@ func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) {
 
 	ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t)
 
-	_, err = r.storageClient.Delete(ctx, &provider.DeleteRequest{
+	resp, err := r.storageClient.Delete(ctx, &provider.DeleteRequest{
 		Ref: &provider.Reference{
 			Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", accountsFolder, id)},
 		},
 	})
-	return err
+
+	if err != nil {
+		return err
+	}
+
+	// TODO Handle other error codes?
+	if resp.Status.Code == v1beta11.Code_CODE_NOT_FOUND {
+		return &notFoundErr{"account", id}
+	}
+
+	return nil
 }
 
 // WriteGroup writes a group via cs3 and modifies the provided group (e.g. with a generated id).
@@ -153,7 +166,7 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) {
 
 	var by []byte
 	if by, err = json.Marshal(g); err != nil {
-		return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error())
+		return err
 	}
 
 	ureq, err := http.NewRequest("PUT", r.groupURL(g.Id), bytes.NewReader(by))
@@ -195,6 +208,10 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err
 		return err
 	}
 
+	if resp.StatusCode == http.StatusNotFound {
+		return &notFoundErr{"group", id}
+	}
+
 	b, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
 		return err
@@ -217,11 +234,21 @@ func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) {
 
 	ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t)
 
-	_, err = r.storageClient.Delete(ctx, &provider.DeleteRequest{
+	resp, err := r.storageClient.Delete(ctx, &provider.DeleteRequest{
 		Ref: &provider.Reference{
 			Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", groupsFolder, id)},
 		},
 	})
+
+	if err != nil {
+		return err
+	}
+
+	// TODO Handle other error codes?
+	if resp.Status.Code == v1beta11.Code_CODE_NOT_FOUND {
+		return &notFoundErr{"group", id}
+	}
+
 	return err
 }
 

accounts/pkg/storage/cs3_test.go

@@ -1,9 +1,26 @@
 package storage
 
+// Uncomment to test locally, requires started metadata-storage for now
+
+//import (
+//	"context"
+//	"github.com/owncloud/ocis/accounts/pkg/config"
+//	"github.com/owncloud/ocis/accounts/pkg/proto/v0"
+//	"github.com/stretchr/testify/assert"
+//	"testing"
+//)
+//
 //var cfg = &config.Config{
 //	TokenManager: config.TokenManager{
 //		JWTSecret: "Pive-Fumkiu4",
 //	},
+//	Repo: config.Repo{
+//		CS3:  config.CS3{
+//			ProviderAddr: "0.0.0.0:9215",
+//			DriverURL:    "http://localhost:9216",
+//			DataPrefix:   "data",
+//		},
+//	},
 //}
 //
 //func TestCS3Repo_WriteAccount(t *testing.T) {

accounts/pkg/storage/disk.go

@@ -9,7 +9,6 @@ import (
 	"path/filepath"
 	"sync"
 
-	merrors "github.com/micro/go-micro/v2/errors"
 	"github.com/owncloud/ocis/accounts/pkg/proto/v0"
 	olog "github.com/owncloud/ocis/ocis-pkg/log"
 )
@@ -52,41 +51,40 @@ func (r DiskRepo) WriteAccount(ctx context.Context, a *proto.Account) (err error
 
 	var bytes []byte
 	if bytes, err = json.Marshal(a); err != nil {
-		return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error())
+		return err
 	}
 
 	path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, a.Id)
-
+	return ioutil.WriteFile(path, bytes, 0600)
-	if err = ioutil.WriteFile(path, bytes, 0600); err != nil {
-		return merrors.InternalServerError(r.serviceID, "could not write account: %v", err.Error())
-	}
-	return
 }
 
 // LoadAccount from the local filesystem
 func (r DiskRepo) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) {
 	path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id)
-
 	var data []byte
 	if data, err = ioutil.ReadFile(path); err != nil {
-		return merrors.NotFound(r.serviceID, "could not read account: %v", err.Error())
+		if os.IsNotExist(err) {
+			err = &notFoundErr{"account", id}
+		}
+		return
 	}
 
-	if err = json.Unmarshal(data, a); err != nil {
+	return json.Unmarshal(data, a)
-		return merrors.InternalServerError(r.serviceID, "could not unmarshal account: %v", err.Error())
-	}
-	return
 }
 
 // DeleteAccount from the local filesystem
 func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) {
 	path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id)
 	if err = os.Remove(path); err != nil {
-		r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account")
+		if os.IsNotExist(err) {
-		return merrors.InternalServerError(r.serviceID, "could not remove account: %v", err.Error())
+			err = &notFoundErr{"account", id}
+		}
 	}
 
-	return nil
+	//r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account")
+	//return merrors.InternalServerError(r.serviceID, "could not remove account: %v", err.Error())
+
+	return
 }
 
 // WriteGroup to the local filesystem
@@ -96,17 +94,19 @@ func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) {
 
 	var bytes []byte
 	if bytes, err = json.Marshal(g); err != nil {
-		return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error())
+		return err
 	}
 
 	path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, g.Id)
 
 	groupLock.Lock()
 	defer groupLock.Unlock()
-	if err = ioutil.WriteFile(path, bytes, 0600); err != nil {
+
-		return merrors.InternalServerError(r.serviceID, "could not write group: %v", err.Error())
+	return ioutil.WriteFile(path, bytes, 0600)
-	}
+
-	return
+	//return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error())
+
+	//return merrors.InternalServerError(r.serviceID, "could not write group: %v", err.Error())
 }
 
 // LoadGroup from the local filesystem
@@ -117,25 +117,29 @@ func (r DiskRepo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err
 	defer groupLock.Unlock()
 	var data []byte
 	if data, err = ioutil.ReadFile(path); err != nil {
-		return merrors.NotFound(r.serviceID, "could not read group: %v", err.Error())
+		if os.IsNotExist(err) {
+			err = &notFoundErr{"group", id}
+		}
+
+		return
 	}
 
-	if err = json.Unmarshal(data, g); err != nil {
+	return json.Unmarshal(data, g)
-		return merrors.InternalServerError(r.serviceID, "could not unmarshal group: %v", err.Error())
-	}
-
-	return
 }
 
 // DeleteGroup from the local filesystem
 func (r DiskRepo) DeleteGroup(ctx context.Context, id string) (err error) {
 	path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, id)
 	if err = os.Remove(path); err != nil {
-		r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove group")
+		if os.IsNotExist(err) {
-		return merrors.InternalServerError(r.serviceID, "could not remove group: %v", err.Error())
+			err = &notFoundErr{"account", id}
+		}
 	}
 
 	return nil
+
+	//r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove group")
+	//return merrors.InternalServerError(r.serviceID, "could not remove group: %v", err.Error())
 }
 
 // deflateMemberOf replaces the groups of a user with an instance that only contains the id

accounts/pkg/storage/errors.go

@@ -0,0 +1,19 @@
+package storage
+
+import (
+	"fmt"
+)
+
+type notFoundErr struct {
+	typ, id string
+}
+
+func (e *notFoundErr) Error() string {
+	return fmt.Sprintf("%s with id %s not found", e.typ, e.id)
+}
+
+// IsNotFoundErr can be returned by repo Load and Delete operations
+func IsNotFoundErr(e error) bool {
+	_, ok := e.(*notFoundErr)
+	return ok
+}