Skip to content

Commit

Permalink
refactor: replace deprecated errors module in src (magma#12729)
Browse files Browse the repository at this point in the history
* refactor(src): Replaces errors.Wrap[f] with fmt.Errorf.

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

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

Also update go.mod file

Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>
  • Loading branch information
MoritzThomasHuebner authored and emakeev committed Aug 5, 2022
1 parent f49c713 commit 775ff99
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 67 deletions.
14 changes: 1 addition & 13 deletions src/go/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,24 +74,12 @@ client/server implementation. Tightly coupled with protobuf.

Considered alternatives: Thrift, Swagger, JSON-RPC

### github.com/pkg/errors

BSD-2 license.

Very lightweight library, does not have any transitive dependencies. A well-
reasoned extension to built-in errors, this package provides stack tracing and
error wrap/unwrapping. Some of this functionality was added to the built-in
errors package as of Golang 1.13 (and this package influenced part of that
design), but the built-in is lacking easy stack trace support.

Considered alternatives: built-in errors, github.com/cockroachdb/errors

### Uber Zap

MIT license.

Medium weight, pulls in a few transitive dependencies, but has a large overlap
with dependencies we also use (github.com/pkg/errors, testify). Provides
with testify. Provides
scoped/named loggers and parameterized logging while achieving better
performance than other libraries proven via benchmarks.

Expand Down
13 changes: 6 additions & 7 deletions src/go/agwd/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"sync"

"github.com/magma/magma/src/go/protos/magma/config"
"github.com/pkg/errors"
"google.golang.org/grpc/resolver"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/proto"
Expand Down Expand Up @@ -199,21 +198,21 @@ func loadConfigFile(
path string,
) (*config.AgwD, error) {
if _, err := osStat(path); err != nil {
return nil, errors.Wrap(err, "path="+path)
return nil, fmt.Errorf("path=%s: %w", path, err)
}

bytes, err := readFile(path)
if err != nil {
return nil, errors.Wrap(err, "path="+path)
return nil, fmt.Errorf("path=%s: %w", path, err)
}
filtered := []byte(filterJSONComments(string(bytes)))
config := &config.AgwD{}
if err := unmarshalProto(filtered, config); err != nil {
return nil, errors.Wrapf(
err,
"path=%s filtered=%s",
return nil, fmt.Errorf(
"path=%s filtered=%s: %w",
path,
string(filtered))
string(filtered),
err)
}
return config, nil
}
Expand Down
4 changes: 2 additions & 2 deletions src/go/agwd/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
package config

import (
"errors"
"os"
"path/filepath"
"strings"
"sync"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc/resolver"
"google.golang.org/protobuf/proto"
Expand Down Expand Up @@ -258,7 +258,7 @@ func TestNewConfigManager_DefaultNotFound(t *testing.T) {

cm := NewConfigManager()
err := LoadConfigFile(cm, filepath.Join("testdata", "doesnotexist.json"))
assert.True(t, os.IsNotExist(errors.Cause(err)))
assert.True(t, os.IsNotExist(errors.Unwrap(err)))
assert.True(t, proto.Equal(newDefaultConfig(), cm.Config()))
}

Expand Down
77 changes: 38 additions & 39 deletions src/go/agwd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
configpb "github.com/magma/magma/src/go/protos/magma/config"
service_capture "github.com/magma/magma/src/go/service/capture"
service_config "github.com/magma/magma/src/go/service/config"
"github.com/pkg/errors"
"google.golang.org/grpc"

"github.com/magma/magma/src/go/agwd/config"
Expand Down Expand Up @@ -63,7 +62,7 @@ func cleanupUnixSocket(
return nil
}
if err != nil {
return errors.Wrapf(err, "os.Stat(%s)", path)
return fmt.Errorf("os.Stat(%s): %w", path, err)
}

// attempt to connect to see if someone is still bound to the socket.
Expand All @@ -76,18 +75,18 @@ func cleanupUnixSocket(
}
logger.Warning().Printf("Removing existing socket file; previous unclean shutdown?")
if err := osRemove(path); err != nil {
return errors.Wrapf(err, "os.Stat(%s)", path)
return fmt.Errorf("os.Stat(%s): %w", path, err)
}

return nil
}

func cleanupUnixSocketOrDie(logger log.Logger, path string) {
if err := cleanupUnixSocket(logger, os.Stat, os.Remove, net.DialTimeout, path); err != nil {
panic(errors.Wrapf(
err,
"cleanupUnixSocket(logger=_, target.Endpoint=%s)",
path))
panic(fmt.Errorf(
"cleanupUnixSocket(logger=_, target.Endpoint=%s): %w",
path,
err))
}
}

Expand All @@ -102,23 +101,23 @@ func startSctpdDownlinkServer(
listener, err := net.Listen(
target.Scheme, target.Endpoint)
if err != nil {
panic(errors.Wrapf(
err,
"net.Listen(network=%s, address=%s)",
panic(fmt.Errorf(
"net.Listen(network=%s, address=%s): %w",
target.Scheme,
target.Endpoint))
target.Endpoint,
err))
}

grpcServer := grpc.NewServer(so...)
sctpdDownlinkServer := sctpd.NewProxyDownlinkServer(logger, sr)
sctpdpb.RegisterSctpdDownlinkServer(grpcServer, sctpdDownlinkServer)
go func() {
if err := grpcServer.Serve(listener); err != nil {
panic(errors.Wrapf(
err,
"startSctpdDownlinkServer(network=%s, address=%s)",
panic(fmt.Errorf(
"startSctpdDownlinkServer(network=%s, address=%s): %w",
target.Scheme,
target.Endpoint))
target.Endpoint,
err))
}
}()
}
Expand All @@ -133,23 +132,23 @@ func startSctpdUplinkServer(

listener, err := net.Listen(target.Scheme, target.Endpoint)
if err != nil {
panic(errors.Wrapf(
err,
"net.Listen(network=%s, address=%s)",
panic(fmt.Errorf(
"net.Listen(network=%s, address=%s): %w",
target.Scheme,
target.Endpoint))
target.Endpoint,
err))
}

grpcServer := grpc.NewServer(so...)
sctpdUplinkServer := sctpd.NewProxyUplinkServer(logger, sr)
sctpdpb.RegisterSctpdUplinkServer(grpcServer, sctpdUplinkServer)
go func() {
if err := grpcServer.Serve(listener); err != nil {
panic(errors.Wrapf(
err,
"startSctpdUplinkServer(network=%s, address=%s)",
panic(fmt.Errorf(
"startSctpdUplinkServer(network=%s, address=%s): %w",
target.Scheme,
target.Endpoint))
target.Endpoint,
err))
}
}()
}
Expand All @@ -159,22 +158,22 @@ func startPipelinedServer(cfgr config.Configer, logger log.Logger) {
listener, err := net.Listen(target.Scheme, target.Endpoint)

if err != nil {
panic(errors.Wrapf(
err,
"net.Listen(network=%s, address=%s)",
panic(fmt.Errorf(
"net.Listen(network=%s, address=%s): %w",
target.Scheme,
target.Endpoint))
target.Endpoint,
err))
}
grpcServer := grpc.NewServer()
pipelinedServer := pipelined.NewPipelinedServer(logger)
pipelinedpb.RegisterPipelinedServer(grpcServer, pipelinedServer)
go func() {
if err := grpcServer.Serve(listener); err != nil {
panic(errors.Wrapf(
err,
"startPipelinedServer(network=%s, address=%s)",
panic(fmt.Errorf(
"startPipelinedServer(network=%s, address=%s): %w",
target.Scheme,
target.Endpoint))
target.Endpoint,
err))
}
}()
}
Expand All @@ -185,11 +184,11 @@ func startCaptureServer(
address := fmt.Sprintf(":%s", cfgr.Config().GetCaptureServicePort())
listener, err := net.Listen(config.TCP, address)
if err != nil {
panic(errors.Wrapf(
err,
"net.Listen(network=%s, address=%s)",
panic(fmt.Errorf(
"net.Listen(network=%s, address=%s): %w",
config.TCP,
address))
address,
err))
}

grpcServer := grpc.NewServer()
Expand All @@ -205,11 +204,11 @@ func startConfigServer(

listener, err := net.Listen(config.TCP, address)
if err != nil {
panic(errors.Wrapf(
err,
"net.Listen(network=%s, address=%s)",
panic(fmt.Errorf(
"net.Listen(network=%s, address=%s): %w",
config.TCP,
address))
address,
err))
}

grpcServer := grpc.NewServer()
Expand Down
2 changes: 1 addition & 1 deletion src/go/crash/crash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
package crash_test

import (
"errors"
"testing"
"time"

"github.com/golang/mock/gomock"
"github.com/magma/magma/src/go/crash"
mock_crash "github.com/magma/magma/src/go/crash/mock_crash"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
)

Expand Down
4 changes: 2 additions & 2 deletions src/go/crash/sentry/sentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
package sentry

import (
"fmt"
"time"

"github.com/getsentry/sentry-go"
"github.com/magma/magma/src/go/crash"
"github.com/pkg/errors"
)

//go:generate go run github.com/golang/mock/mockgen -source=sentry.go -package mock_sentry -destination mock_sentry/mock_sentry_hub.go sentryHub
Expand All @@ -37,7 +37,7 @@ type Crash struct {
func NewCrash(options sentry.ClientOptions) crash.Crash {
client, err := sentry.NewClient(options)
if err != nil {
panic(errors.Wrapf(err, "sentry.ClientOptions=%+v", options))
panic(fmt.Errorf("sentry.ClientOptions=%+v: %w", options, err))
}
newHub := sentry.NewHub(client, sentry.NewScope())
return &Crash{
Expand Down
1 change: 0 additions & 1 deletion src/go/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ go 1.18
require (
github.com/getsentry/sentry-go v0.11.0
github.com/golang/mock v1.6.0
github.com/pkg/errors v0.8.1
github.com/stretchr/testify v1.7.0
go.uber.org/zap v1.19.0
google.golang.org/grpc v1.40.0
Expand Down
5 changes: 3 additions & 2 deletions src/go/log/zap/zap.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
package zap

import (
"github.com/pkg/errors"
"fmt"

"go.uber.org/zap"
"go.uber.org/zap/zapcore"

Expand Down Expand Up @@ -204,7 +205,7 @@ func NewLoggerAtLevel(lvl log.Level, paths ...string) *Logger {
}
ws, _, err := zap.Open(paths...)
if err != nil {
panic(errors.Wrapf(err, "paths=%s", paths))
panic(fmt.Errorf("paths=%s: %w", paths, err))
}

return New(
Expand Down

0 comments on commit 775ff99

Please sign in to comment.