MG-149 - Fix leaking database errors (#183)

* fix: handle database errors in svc Signed-off-by: felix.gateru <felix.gateru@gmail.com> * fix: add descriptive error to postgres errors Signed-off-by: felix.gateru <felix.gateru@gmail.com> --------- Signed-off-by: felix.gateru <felix.gateru@gmail.com>
2026-06-23 04:10:28 +00:00 · 2023-12-20 12:51:04 +03:00
parent 648655036e
commit a6dd160091
7 changed files with 141 additions and 41 deletions
@@ -143,7 +143,11 @@ func (svc service) RetrieveKey(ctx context.Context, token, id string) (Key, erro
 		return Key{}, errors.Wrap(errRetrieve, err)
 	}

-	return svc.keys.Retrieve(ctx, issuerID, id)
+	key, err := svc.keys.Retrieve(ctx, issuerID, id)
+	if err != nil {
+		return Key{}, errors.Wrap(errRetrieve, err)
+	}
+	return key, nil
 }

 func (svc service) Identify(ctx context.Context, token string) (Key, error) {
@@ -499,8 +503,12 @@ func (svc service) CreateDomain(ctx context.Context, token string, d Domain) (do
 			}
 		}
 	}()
+	dom, err := svc.domains.Save(ctx, d)
+	if err != nil {
+		return Domain{}, errors.Wrap(svcerr.ErrCreateEntity, err)
+	}

-	return svc.domains.Save(ctx, d)
+	return dom, nil
 }

 func (svc service) RetrieveDomain(ctx context.Context, token, id string) (Domain, error) {
@@ -514,8 +522,11 @@ func (svc service) RetrieveDomain(ctx context.Context, token, id string) (Domain
 	}); err != nil {
 		return Domain{}, errors.Wrap(svcerr.ErrAuthorization, err)
 	}
-
-	return svc.domains.RetrieveByID(ctx, id)
+	dom, err := svc.domains.RetrieveByID(ctx, id)
+	if err != nil {
+		return Domain{}, errors.Wrap(svcerr.ErrNotFound, err)
+	}
+	return dom, nil
 }

 func (svc service) RetrieveDomainPermissions(ctx context.Context, token, id string) (Permissions, error) {
@@ -562,7 +573,12 @@ func (svc service) UpdateDomain(ctx context.Context, token, id string, d DomainR
 	}); err != nil {
 		return Domain{}, errors.Wrap(svcerr.ErrAuthorization, err)
 	}
-	return svc.domains.Update(ctx, id, key.User, d)
+
+	dom, err := svc.domains.RetrieveByID(ctx, id)
+	if err != nil {
+		return Domain{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+	return dom, nil
 }

 func (svc service) ChangeDomainStatus(ctx context.Context, token, id string, d DomainReq) (Domain, error) {
@@ -580,7 +596,12 @@ func (svc service) ChangeDomainStatus(ctx context.Context, token, id string, d D
 	}); err != nil {
 		return Domain{}, errors.Wrap(svcerr.ErrAuthorization, err)
 	}
-	return svc.domains.Update(ctx, id, key.User, d)
+
+	dom, err := svc.domains.Update(ctx, id, key.User, d)
+	if err != nil {
+		return Domain{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+	return dom, nil
 }

 func (svc service) ListDomains(ctx context.Context, token string, p Page) (DomainsPage, error) {
@@ -671,7 +692,10 @@ func (svc service) UnassignUsers(ctx context.Context, token, id string, userIds
 		return err
 	}

-	return svc.removeDomainPolicies(ctx, id, relation, userIds...)
+	if err := svc.removeDomainPolicies(ctx, id, relation, userIds...); err != nil {
+		return errors.Wrap(errRemovePolicies, err)
+	}
+	return nil
 }

 // IMPROVEMENT NOTE: Take decision: Only Patform admin or both Patform and domain admins can see others users domain.
@@ -694,7 +718,11 @@ func (svc service) ListUserDomains(ctx context.Context, token, userID string, p
 	} else {
 		p.SubjectID = res.User
 	}
-	return svc.domains.ListDomains(ctx, p)
+	dp, err := svc.domains.ListDomains(ctx, p)
+	if err != nil {
+		return DomainsPage{}, errors.Wrap(svcerr.ErrViewEntity, err)
+	}
+	return dp, nil
 }

 func (svc service) addDomainPolicies(ctx context.Context, domainID, relation string, userIDs ...string) (err error) {
@@ -728,7 +756,11 @@ func (svc service) addDomainPolicies(ctx context.Context, domainID, relation str
 			}
 		}
 	}()
-	return svc.domains.SavePolicies(ctx, pcs...)
+
+	if err = svc.domains.SavePolicies(ctx, pcs...); err != nil {
+		return errors.Wrap(errAddPolicies, err)
+	}
+	return nil
 }

 func (svc service) createDomainPolicy(ctx context.Context, userID, domainID, relation string) (err error) {
@@ -759,13 +791,17 @@ func (svc service) createDomainPolicy(ctx context.Context, userID, domainID, rel
 			}
 		}
 	}()
-	return svc.domains.SavePolicies(ctx, Policy{
+	err = svc.domains.SavePolicies(ctx, Policy{
 		SubjectType: UserType,
 		SubjectID:   userID,
 		Relation:    relation,
 		ObjectType:  DomainType,
 		ObjectID:    domainID,
 	})
+	if err != nil {
+		return errors.Wrap(errCreateDomainPolicy, err)
+	}
+	return err
 }

 func (svc service) createDomainPolicyRollback(ctx context.Context, userID, domainID, relation string) error {
@@ -827,8 +863,11 @@ func (svc service) removeDomainPolicies(ctx context.Context, domainID, relation
 	if err := svc.agent.DeletePolicies(ctx, prs); err != nil {
 		return errors.Wrap(errRemovePolicies, err)
 	}
-
-	return svc.domains.DeletePolicies(ctx, pcs...)
+	err = svc.domains.DeletePolicies(ctx, pcs...)
+	if err != nil {
+		return errors.Wrap(errRemovePolicies, err)
+	}
+	return err
 }

 func EncodeDomainUserID(domainID, userID string) string {
@@ -261,7 +261,7 @@ func TestUpdate(t *testing.T) {
 	for _, tc := range cases {
 		repoCall := auth.On("Identify", mock.Anything, &magistrala.IdentityReq{Token: tc.token}).Return(&magistrala.IdentityRes{Id: validID}, nil)
 		err := svc.Update(context.Background(), tc.token, tc.config)
-		assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err))
+		assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err))

 		streams := redisClient.XRead(context.Background(), &redis.XReadArgs{
 			Streams: []string{streamID, lastID},
@@ -172,8 +172,11 @@ func (bs bootstrapService) View(ctx context.Context, token, id string) (Config,
 	if err != nil {
 		return Config{}, errors.Wrap(svcerr.ErrAuthentication, err)
 	}
-
-	return bs.configs.RetrieveByID(ctx, owner, id)
+	cfg, err := bs.configs.RetrieveByID(ctx, owner, id)
+	if err != nil {
+		return Config{}, errors.Wrap(svcerr.ErrViewEntity, err)
+	}
+	return cfg, nil
 }

 func (bs bootstrapService) Update(ctx context.Context, token string, cfg Config) error {
@@ -183,8 +186,10 @@ func (bs bootstrapService) Update(ctx context.Context, token string, cfg Config)
 	}

 	cfg.Owner = owner
-
-	return bs.configs.Update(ctx, cfg)
+	if err = bs.configs.Update(ctx, cfg); err != nil {
+		return errors.Wrap(errUpdateConnections, err)
+	}
+	return nil
 }

 func (bs bootstrapService) UpdateCert(ctx context.Context, token, thingID, clientCert, clientKey, caCert string) (Config, error) {
@@ -249,8 +254,10 @@ func (bs bootstrapService) UpdateConnections(ctx context.Context, token, id stri
 			return ErrThings
 		}
 	}
-
-	return bs.configs.UpdateConnections(ctx, owner, id, channels, connections)
+	if err := bs.configs.UpdateConnections(ctx, owner, id, channels, connections); err != nil {
+		return errors.Wrap(errUpdateConnections, err)
+	}
+	return nil
 }

 func (bs bootstrapService) List(ctx context.Context, token string, filter Filter, offset, limit uint64) (ConfigsPage, error) {
@@ -258,7 +265,6 @@ func (bs bootstrapService) List(ctx context.Context, token string, filter Filter
 	if err != nil {
 		return ConfigsPage{}, errors.Wrap(svcerr.ErrAuthentication, err)
 	}
-
 	return bs.configs.RetrieveAll(ctx, owner, filter, offset, limit), nil
 }

@@ -12,10 +12,12 @@ import (
 // Postgres error codes:
 // https://www.postgresql.org/docs/current/errcodes-appendix.html
 const (
-	errDuplicate  = "23505" // unique_violation
-	errTruncation = "22001" // string_data_right_truncation
-	errFK         = "23503" // foreign_key_violation
-	errInvalid    = "22P02" // invalid_text_representation
+	errDuplicate      = "23505" // unique_violation
+	errTruncation     = "22001" // string_data_right_truncation
+	errFK             = "23503" // foreign_key_violation
+	errInvalid        = "22P02" // invalid_text_representation
+	errUntranslatable = "22P05" // untranslatable_character
+	errInvalidChar    = "22021" // character_not_in_repertoire
 )

 func HandleError(wrapper, err error) error {
@@ -24,7 +26,7 @@ func HandleError(wrapper, err error) error {
 		switch pqErr.Code {
 		case errDuplicate:
 			return errors.Wrap(repoerr.ErrConflict, err)
-		case errInvalid, errTruncation:
+		case errInvalid, errInvalidChar, errTruncation, errUntranslatable:
 			return errors.Wrap(repoerr.ErrMalformedEntity, err)
 		case errFK:
 			return errors.Wrap(repoerr.ErrCreateEntity, err)
@@ -133,7 +133,7 @@ func (repo ClientRepository) RetrieveByIdentity(ctx context.Context, identity st
 		if err == sql.ErrNoRows {
 			return clients.Client{}, errors.Wrap(repoerr.ErrNotFound, err)
 		}
-		return clients.Client{}, errors.Wrap(repoerr.ErrViewEntity, err)
+		return clients.Client{}, postgres.HandleError(repoerr.ErrViewEntity, err)
 	}

 	defer row.Close()
@@ -131,8 +131,11 @@ func (svc service) ViewClient(ctx context.Context, token, id string) (mgclients.
 	if err != nil {
 		return mgclients.Client{}, errors.Wrap(svcerr.ErrAuthorization, err)
 	}
-
-	return svc.clients.RetrieveByID(ctx, id)
+	client, err := svc.clients.RetrieveByID(ctx, id)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrNotFound, err)
+	}
+	return client, nil
 }

 func (svc service) ViewClientPerms(ctx context.Context, token, id string) ([]string, error) {
@@ -301,7 +304,11 @@ func (svc service) UpdateClient(ctx context.Context, token string, cli mgclients
 		UpdatedAt: time.Now(),
 		UpdatedBy: userID,
 	}
-	return svc.clients.Update(ctx, client)
+	client, err = svc.clients.Update(ctx, client)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+	return client, nil
 }

 func (svc service) UpdateClientTags(ctx context.Context, token string, cli mgclients.Client) (mgclients.Client, error) {
@@ -316,7 +323,11 @@ func (svc service) UpdateClientTags(ctx context.Context, token string, cli mgcli
 		UpdatedAt: time.Now(),
 		UpdatedBy: userID,
 	}
-	return svc.clients.UpdateTags(ctx, client)
+	client, err = svc.clients.UpdateTags(ctx, client)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+	return client, nil
 }

 func (svc service) UpdateClientSecret(ctx context.Context, token, id, key string) (mgclients.Client, error) {
@@ -334,7 +345,11 @@ func (svc service) UpdateClientSecret(ctx context.Context, token, id, key string
 		UpdatedBy: userID,
 		Status:    mgclients.EnabledStatus,
 	}
-	return svc.clients.UpdateSecret(ctx, client)
+	client, err = svc.clients.UpdateSecret(ctx, client)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+	return client, nil
 }

 func (svc service) EnableClient(ctx context.Context, token, id string) (mgclients.Client, error) {
@@ -441,7 +456,12 @@ func (svc service) changeClientStatus(ctx context.Context, token string, client
 	}

 	client.UpdatedBy = userID
-	return svc.clients.ChangeStatus(ctx, client)
+
+	client, err = svc.clients.ChangeStatus(ctx, client)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+	return client, nil
 }

 func (svc service) ListClientsByGroup(ctx context.Context, token, groupID string, pm mgclients.Page) (mgclients.MembersPage, error) {
@@ -106,8 +106,11 @@ func (svc service) RegisterClient(ctx context.Context, token string, cli mgclien
 			}
 		}
 	}()
-
-	return svc.clients.Save(ctx, cli)
+	client, err := svc.clients.Save(ctx, cli)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(repoerr.ErrCreateEntity, err)
+	}
+	return client, nil
 }

 func (svc service) IssueToken(ctx context.Context, identity, secret, domainID string) (*magistrala.Token, error) {
@@ -175,7 +178,11 @@ func (svc service) ListClients(ctx context.Context, token string, pm mgclients.P
 		return mgclients.ClientsPage{}, err
 	}
 	if err := svc.checkSuperAdmin(ctx, userID); err == nil {
-		return svc.clients.RetrieveAll(ctx, pm)
+		pg, err := svc.clients.RetrieveAll(ctx, pm)
+		if err != nil {
+			return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrNotFound, err)
+		}
+		return pg, err
 	}
 	role := mgclients.UserRole
 	p := mgclients.Page{
@@ -186,7 +193,11 @@ func (svc service) ListClients(ctx context.Context, token string, pm mgclients.P
 		Identity: pm.Identity,
 		Role:     &role,
 	}
-	return svc.clients.RetrieveAllBasicInfo(ctx, p)
+	pg, err := svc.clients.RetrieveAll(ctx, p)
+	if err != nil {
+		return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrNotFound, err)
+	}
+	return pg, nil
 }

 func (svc service) UpdateClient(ctx context.Context, token string, cli mgclients.Client) (mgclients.Client, error) {
@@ -209,7 +220,11 @@ func (svc service) UpdateClient(ctx context.Context, token string, cli mgclients
 		UpdatedBy: tokenUserID,
 	}

-	return svc.clients.Update(ctx, client)
+	client, err = svc.clients.Update(ctx, client)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+	return client, nil
 }

 func (svc service) UpdateClientTags(ctx context.Context, token string, cli mgclients.Client) (mgclients.Client, error) {
@@ -230,8 +245,12 @@ func (svc service) UpdateClientTags(ctx context.Context, token string, cli mgcli
 		UpdatedAt: time.Now(),
 		UpdatedBy: tokenUserID,
 	}
+	client, err = svc.clients.UpdateTags(ctx, client)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}

-	return svc.clients.UpdateTags(ctx, client)
+	return client, nil
 }

 func (svc service) UpdateClientIdentity(ctx context.Context, token, clientID, identity string) (mgclients.Client, error) {
@@ -254,7 +273,11 @@ func (svc service) UpdateClientIdentity(ctx context.Context, token, clientID, id
 		UpdatedAt: time.Now(),
 		UpdatedBy: tokenUserID,
 	}
-	return svc.clients.UpdateIdentity(ctx, cli)
+	cli, err = svc.clients.UpdateIdentity(ctx, cli)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+	return cli, nil
 }

 func (svc service) GenerateResetToken(ctx context.Context, email, host string) error {
@@ -330,7 +353,12 @@ func (svc service) UpdateClientSecret(ctx context.Context, token, oldSecret, new
 	dbClient.UpdatedAt = time.Now()
 	dbClient.UpdatedBy = id

-	return svc.clients.UpdateSecret(ctx, dbClient)
+	dbClient, err = svc.clients.UpdateSecret(ctx, dbClient)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+
+	return dbClient, nil
 }

 func (svc service) SendPasswordReset(_ context.Context, host, email, user, token string) error {
@@ -412,7 +440,12 @@ func (svc service) changeClientStatus(ctx context.Context, token string, client
 		return mgclients.Client{}, mgclients.ErrStatusAlreadyAssigned
 	}
 	client.UpdatedBy = tokenUserID
-	return svc.clients.ChangeStatus(ctx, client)
+
+	client, err = svc.clients.ChangeStatus(ctx, client)
+	if err != nil {
+		return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err)
+	}
+	return client, err
 }

 func (svc service) ListMembers(ctx context.Context, token, objectKind, objectID string, pm mgclients.Page) (mgclients.MembersPage, error) {