refactor(cwf): Replaces more pkg/errors usages with std lib.

Also fixes an incorrect use of commit/rollback error handling in ue_store_utils.go and uesim.go.

Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>

cwf/cloud/go/go.mod

@@ -29,7 +29,6 @@ require (
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
 	github.com/golang/protobuf v1.5.2
 	github.com/labstack/echo v3.3.10+incompatible
-	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.5.1
 	github.com/prometheus/common v0.9.1
 	github.com/stretchr/testify v1.7.0
@@ -82,6 +81,7 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/oklog/ulid v1.3.1 // indirect
 	github.com/olivere/elastic/v7 v7.0.6 // indirect
+	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_model v0.2.0 // indirect
 	github.com/prometheus/procfs v0.0.8 // indirect

cwf/cloud/go/services/cwf/obsidian/models/validate.go

@@ -15,12 +15,12 @@ package models
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 
 	"github.com/go-openapi/strfmt"
 	"github.com/go-openapi/swag"
-	"github.com/pkg/errors"
 )
 
 func (m *CwfNetwork) ValidateModel(context.Context) error {

cwf/cloud/go/services/cwf/servicers/protected/builder_servicer.go

@@ -20,7 +20,6 @@ import (
 
 	"github.com/go-openapi/swag"
 	"github.com/golang/protobuf/proto"
-	"github.com/pkg/errors"
 
 	"magma/cwf/cloud/go/cwf"
 	cwf_mconfig "magma/cwf/cloud/go/protos/mconfig"
@@ -90,7 +89,7 @@ func (s *builderServicer) Build(ctx context.Context, request *builder_protos.Bui
 	}
 	vals, err := buildFromConfigs(nwConfig, gwConfig.Config.(*models.GatewayCwfConfigs), haPairConfigs)
 	if err != nil {
-		return nil, errors.WithStack(err)
+		return nil, err
 	}
 	ret.ConfigsByKey, err = mconfig.MarshalConfigs(vals)
 	if err != nil {

cwf/gateway/go.mod

@@ -36,7 +36,6 @@ require (
 	github.com/go-redis/redis v6.15.5+incompatible
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
 	github.com/golang/protobuf v1.5.2
-	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.11.0
 	github.com/shirou/gopsutil/v3 v3.21.5
 	github.com/sparrc/go-ping v0.0.0-20190613174326-4e5b6552494c
@@ -108,6 +107,7 @@ require (
 	github.com/oklog/ulid v1.3.1 // indirect
 	github.com/olivere/elastic/v7 v7.0.6 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect
+	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_model v0.2.0 // indirect
 	github.com/prometheus/common v0.26.0 // indirect

cwf/gateway/integ_tests/service_wrappers.go

@@ -15,6 +15,7 @@ package integration
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"magma/cwf/gateway/registry"
@@ -29,7 +30,6 @@ import (
 
 	"github.com/go-redis/redis"
 	"github.com/golang/glog"
-	"github.com/pkg/errors"
 	"google.golang.org/grpc"
 )
 

cwf/gateway/integ_tests/test_runner.go

@@ -30,7 +30,6 @@ import (
 	lteprotos "magma/lte/cloud/go/protos"
 
 	"github.com/golang/protobuf/ptypes/wrappers"
-	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -80,7 +79,7 @@ const (
 	GyValidityTime  = 60   // in second
 )
 
-//TestRunner helps setting up all associated services
+// TestRunner helps setting up all associated services
 type TestRunner struct {
 	t           *testing.T
 	imsis       map[string]bool
@@ -162,7 +161,7 @@ func (tr *TestRunner) ConfigUEsPerInstance(IMSIs []string, pcrfInstance, ocsInst
 	for _, imsi := range IMSIs {
 		// If IMSIs were generated properly they should never give an error here
 		if _, present := tr.imsis[imsi]; present {
-			return nil, errors.Errorf("IMSI %s already exist in database, use generateRandomIMSIS(num, tr.imsis) to create unique list", imsi)
+			return nil, fmt.Errorf("IMSI %s already exist in database, use generateRandomIMSIS(num, tr.imsis) to create unique list", imsi)
 		}
 		key, opc, err := getRandKeyOpcFromOp([]byte(Op))
 		if err != nil {
@@ -281,14 +280,14 @@ func (tr *TestRunner) GenULTrafficBasedOnPolicyUsage(req *cwfprotos.GenTrafficRe
 	for start := time.Now(); time.Since(start) < waitFor; {
 		startGenTrafficTime := time.Now()
 		res, err = uesim.GenTrafficWithReatempts(req)
-		if err != nil{
+		if err != nil {
 			return res, fmt.Errorf("GenULTrafficBasedOnPolicyUsage failed during GenTraffic: %s", err)
 		}
-		completeCycle := time.Now().Sub(startGenTrafficTime)%time.Second
-		if  waitNeeded {
+		completeCycle := time.Now().Sub(startGenTrafficTime) % time.Second
+		if waitNeeded {
 			// this wait makes sure the time passes is exact in seconds. This way
 			// we make sure policies had time to sync
-			time.Sleep(completeCycle + 200 * time.Millisecond)
+			time.Sleep(completeCycle + 200*time.Millisecond)
 			waitNeeded = false
 		}
 		time.Sleep(2200 * time.Millisecond)
@@ -306,19 +305,21 @@ func (tr *TestRunner) GenULTrafficBasedOnPolicyUsage(req *cwfprotos.GenTrafficRe
 		req.Bitrate = &wrappers.StringValue{Value: "10M"}
 		if remaining > 100*KiloBytes {
 			newVolume = uint64(float64(remaining) * 0.95)
-			if newVolume > 5 * MegaBytes {
+			if newVolume > 5*MegaBytes {
 				newVolume = 5 * MegaBytes
 			}
 			// only add wait for the bigger chunks
 			waitNeeded = true
 		}
-		if newVolume < 1000 {newVolume = 1000}
+		if newVolume < 1000 {
+			newVolume = 1000
+		}
 		newVolumeStr := fmt.Sprintf("%dK", newVolume/1000)
 
 		req.Volume = &wrappers.StringValue{Value: newVolumeStr}
 		fmt.Printf("- not enough traffic genereted, sending %dKB more. Will be around %d%% of volume requested\n",
 			newVolume/1000,
-			100 * (record.BytesTx+newVolume)/totalVolume,
+			100*(record.BytesTx+newVolume)/totalVolume,
 		)
 	}
 	return res, fmt.Errorf("error: not enough traffic generated to fullfil GenULTrafficBasedOnPolicyUsage requieriment: %s", err)
@@ -477,34 +478,34 @@ func (tr *TestRunner) WaitForEnforcementStatsForRuleGreaterThanOrDoesNotExist(im
 	}
 }
 
- func (tr *TestRunner) WaitForEnforcementStatsForRuleGreaterThanOrDoesNotExistFunc (imsi, ruleID string, min uint64)(*lteprotos.RuleRecord, bool) {
-		fmt.Printf("\tWaiting until %s, %s has more than %d bytes in enforcement stats or rule does not exist ...\n", imsi, ruleID, min)
-		records, err := tr.GetPolicyUsage()
-		if err != nil {
-			return nil, false
-		}
-		imsi = prependIMSIPrefix(imsi)
-		if records[imsi] == nil {
-			// Session is gone
-			fmt.Printf("\tSession for %s, does not exist...\n", imsi)
-			return nil, true
-		}
-		record := records[imsi][ruleID]
-		if record == nil {
-			// Session is gone
-			fmt.Printf("\tRule %s for %s, does not exist...\n", ruleID, imsi)
-			return nil, true
-		}
-		txBytes := record.BytesTx
-		if record.BytesTx < min {
-			return record, false
-		}
-		fmt.Printf("\t\u2713 %s, %s now passed %d > %d in enforcement stats!(%d%%)\n",
-			imsi, ruleID, txBytes, min, 100* txBytes/min)
-		return record, true
+func (tr *TestRunner) WaitForEnforcementStatsForRuleGreaterThanOrDoesNotExistFunc(imsi, ruleID string, min uint64) (*lteprotos.RuleRecord, bool) {
+	fmt.Printf("\tWaiting until %s, %s has more than %d bytes in enforcement stats or rule does not exist ...\n", imsi, ruleID, min)
+	records, err := tr.GetPolicyUsage()
+	if err != nil {
+		return nil, false
+	}
+	imsi = prependIMSIPrefix(imsi)
+	if records[imsi] == nil {
+		// Session is gone
+		fmt.Printf("\tSession for %s, does not exist...\n", imsi)
+		return nil, true
+	}
+	record := records[imsi][ruleID]
+	if record == nil {
+		// Session is gone
+		fmt.Printf("\tRule %s for %s, does not exist...\n", ruleID, imsi)
+		return nil, true
 	}
+	txBytes := record.BytesTx
+	if record.BytesTx < min {
+		return record, false
+	}
+	fmt.Printf("\t\u2713 %s, %s now passed %d > %d in enforcement stats!(%d%%)\n",
+		imsi, ruleID, txBytes, min, 100*txBytes/min)
+	return record, true
+}
 
-//WaitForPolicyReAuthToProcess returns a method which checks for reauth answer and
+// WaitForPolicyReAuthToProcess returns a method which checks for reauth answer and
 // if it has sessionID which contains the IMSI
 func (tr *TestRunner) WaitForPolicyReAuthToProcess(raa *fegprotos.PolicyReAuthAnswer, imsi string) func() bool {
 	// Todo figure out the best way to figure out when RAR is processed
@@ -516,7 +517,7 @@ func (tr *TestRunner) WaitForPolicyReAuthToProcess(raa *fegprotos.PolicyReAuthAn
 	}
 }
 
-//WaitForChargingReAuthToProcess returns a method which checks for reauth answer and
+// WaitForChargingReAuthToProcess returns a method which checks for reauth answer and
 // if it has sessionID which contains the IMSI
 func (tr *TestRunner) WaitForChargingReAuthToProcess(raa *fegprotos.ChargingReAuthAnswer, imsi string) func() bool {
 	// Todo figure out the best way to figure out when RAR is processed

cwf/gateway/services/uesim/servicers/eap.go

@@ -19,8 +19,6 @@ import (
 	cwfprotos "magma/cwf/cloud/go/protos"
 	fegprotos "magma/feg/gateway/services/aaa/protos"
 	"magma/feg/gateway/services/eap"
-
-	"github.com/pkg/errors"
 )
 
 // todo Replace constants with configurable fields
@@ -41,7 +39,7 @@ func (srv *UESimServer) HandleEap(ue *cwfprotos.UEConfig, req eap.Packet) (eap.P
 	case fegprotos.EapType_AKA:
 		return srv.handleEapAka(ue, req)
 	}
-	return nil, errors.Errorf("Unsupported Eap Type: %d", req[eap.EapMsgMethodType])
+	return nil, fmt.Errorf("Unsupported Eap Type: %d", req[eap.EapMsgMethodType])
 }
 
 func (srv *UESimServer) eapIdentityRequest(ue *cwfprotos.UEConfig, req eap.Packet) (res eap.Packet, err error) {

cwf/gateway/services/uesim/servicers/eap_aka.go

@@ -18,16 +18,15 @@ import (
 	"encoding/binary"
 	"fmt"
 	"io"
-	"magma/feg/gateway/services/testcore/hss/servicers"
 	"reflect"
 
 	"magma/cwf/cloud/go/protos"
 	"magma/feg/gateway/services/eap"
+	"magma/feg/gateway/services/testcore/hss/servicers"
 	"magma/feg/gateway/services/eap/providers/aka"
 	"magma/lte/cloud/go/crypto"
 
 	"github.com/golang/glog"
-	"github.com/pkg/errors"
 )
 
 // todo Replace constants with configurable fields
@@ -54,7 +53,7 @@ func (srv *UESimServer) handleEapAka(ue *protos.UEConfig, req eap.Packet) (eap.P
 	case aka.SubtypeChallenge:
 		return srv.eapAkaChallengeRequest(ue, req)
 	default:
-		return nil, errors.Errorf("Unsupported Subtype: %d", req[eap.EapSubtype])
+		return nil, fmt.Errorf("Unsupported Subtype: %d", req[eap.EapSubtype])
 	}
 }
 
@@ -113,7 +112,7 @@ func (srv *UESimServer) eapAkaChallengeRequest(ue *protos.UEConfig, req eap.Pack
 		return nil, fmt.Errorf("Error while parsing attributes of request packet: %w", err)
 	}
 	if attrs.rand == nil || attrs.autn == nil || attrs.mac == nil {
-		return nil, errors.Errorf("Missing one or more expected attributes\nRAND: %s\nAUTN: %s\nMAC: %s\n", attrs.rand, attrs.autn, attrs.mac)
+		return nil, fmt.Errorf("Missing one or more expected attributes\nRAND: %s\nAUTN: %s\nMAC: %s\n", attrs.rand, attrs.autn, attrs.mac)
 	}
 
 	// Parse out RAND, expected AUTN, and expected MAC values.

cwf/gateway/services/uesim/servicers/ue_store_utils.go

@@ -19,6 +19,8 @@ import (
 	cwfprotos "magma/cwf/cloud/go/protos"
 	"magma/orc8r/cloud/go/blobstore"
 	"magma/orc8r/lib/go/protos"
+
+	"github.com/hashicorp/go-multierror"
 )
 
 func addUeToStore(srvstore blobstore.StoreFactory, ue *cwfprotos.UEConfig) {
@@ -30,16 +32,15 @@ func addUeToStore(srvstore blobstore.StoreFactory, ue *cwfprotos.UEConfig) {
 		return
 	}
 	defer func() {
-		switch err {
-		case nil:
+		if err == nil {
 			if commitErr := store.Commit(); commitErr != nil {
-				err = fmt.Errorf("Error while committing transaction: %w", err)
+				err = fmt.Errorf("Error while committing transaction: %w", commitErr)
 				err = ConvertStorageErrorToGrpcStatus(err)
 			}
-		default:
+		} else {
 			if rollbackErr := store.Rollback(); rollbackErr != nil {
-				err = fmt.Errorf("Error while rolling back transaction: %w", err)
-				err = ConvertStorageErrorToGrpcStatus(err)
+				errs := multierror.Append(err, fmt.Errorf("Error while rolling back transaction: %w", rollbackErr))
+				err = ConvertStorageErrorToGrpcStatus(errs.ErrorOrNil())
 			}
 		}
 	}()

cwf/gateway/services/uesim/servicers/uesim.go

@@ -31,6 +31,7 @@ import (
 	"magma/orc8r/lib/go/protos"
 
 	"github.com/golang/glog"
+	"github.com/hashicorp/go-multierror"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -266,14 +267,15 @@ func getUE(blobStoreFactory blobstore.StoreFactory, imsi string) (ue *cwfprotos.
 		return
 	}
 	defer func() {
-		switch err {
-		case nil:
+		if err == nil {
 			if commitErr := store.Commit(); commitErr != nil {
-				err = fmt.Errorf("Error while committing transaction: %w", err)
+				err = fmt.Errorf("Error while committing transaction: %w", commitErr)
 			}
-		default:
+		} else {
 			if rollbackErr := store.Rollback(); rollbackErr != nil {
-				glog.Errorf("Error while rolling back transaction: %s", err)
+				glog.Errorf("Error while rolling back transaction: %s", rollbackErr)
+				errs := multierror.Append(err, fmt.Errorf("Error while rolling back transaction: %w", rollbackErr))
+				err = errs.ErrorOrNil()
 			}
 		}
 	}()