Skip to content

Commit

Permalink
refactor: optimize the code and use golangci-lint to check for any er…
Browse files Browse the repository at this point in the history
…rors (go-kratos#1298)

* feat: optimize the code and use golangci-lint to check for any errors

* fix: TestLogger unit test
  • Loading branch information
fifsky authored Aug 16, 2021
1 parent 3026f94 commit b44e47b
Show file tree
Hide file tree
Showing 16 changed files with 79 additions and 47 deletions.
3 changes: 2 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ linters:
- staticcheck
- govet
- gosimple
- gofmt
- gofmt
- errcheck
7 changes: 5 additions & 2 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type App struct {
func New(opts ...Option) *App {
options := options{
ctx: context.Background(),
logger: log.DefaultLogger,
logger: log.NewHelper(log.DefaultLogger),
sigs: []os.Signal{syscall.SIGTERM, syscall.SIGQUIT, syscall.SIGINT},
registrarTimeout: 10 * time.Second,
}
Expand Down Expand Up @@ -114,7 +114,10 @@ func (a *App) Run() error {
case <-ctx.Done():
return ctx.Err()
case <-c:
a.Stop()
err := a.Stop()
if err != nil {
a.opts.logger.Errorf("failed to app stop: %v", err)
}
}
}
})
Expand Down
6 changes: 3 additions & 3 deletions internal/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ func Merge(parent1, parent2 context.Context) (context.Context, context.CancelFun
}
select {
case <-parent1.Done():
mc.finish(parent1.Err())
_ = mc.finish(parent1.Err())
case <-parent2.Done():
mc.finish(parent2.Err())
_ = mc.finish(parent2.Err())
default:
go mc.wait()
}
Expand All @@ -57,7 +57,7 @@ func (mc *mergeCtx) wait() {
case <-mc.cancelCh:
err = context.Canceled
}
mc.finish(err)
_ = mc.finish(err)
}

func (mc *mergeCtx) cancel() {
Expand Down
32 changes: 16 additions & 16 deletions log/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,83 +28,83 @@ func (h *Helper) WithContext(ctx context.Context) *Helper {

// Log Print log by level and keyvals.
func (h *Helper) Log(level Level, keyvals ...interface{}) {
h.logger.Log(level, keyvals...)
_ = h.logger.Log(level, keyvals...)
}

// Debug logs a message at debug level.
func (h *Helper) Debug(a ...interface{}) {
h.logger.Log(LevelDebug, "msg", fmt.Sprint(a...))
h.Log(LevelDebug, "msg", fmt.Sprint(a...))
}

// Debugf logs a message at debug level.
func (h *Helper) Debugf(format string, a ...interface{}) {
h.logger.Log(LevelDebug, "msg", fmt.Sprintf(format, a...))
h.Log(LevelDebug, "msg", fmt.Sprintf(format, a...))
}

// Debugw logs a message at debug level.
func (h *Helper) Debugw(keyvals ...interface{}) {
h.logger.Log(LevelDebug, keyvals...)
h.Log(LevelDebug, keyvals...)
}

// Info logs a message at info level.
func (h *Helper) Info(a ...interface{}) {
h.logger.Log(LevelInfo, "msg", fmt.Sprint(a...))
h.Log(LevelInfo, "msg", fmt.Sprint(a...))
}

// Infof logs a message at info level.
func (h *Helper) Infof(format string, a ...interface{}) {
h.logger.Log(LevelInfo, "msg", fmt.Sprintf(format, a...))
h.Log(LevelInfo, "msg", fmt.Sprintf(format, a...))
}

// Infow logs a message at info level.
func (h *Helper) Infow(keyvals ...interface{}) {
h.logger.Log(LevelInfo, keyvals...)
h.Log(LevelInfo, keyvals...)
}

// Warn logs a message at warn level.
func (h *Helper) Warn(a ...interface{}) {
h.logger.Log(LevelWarn, "msg", fmt.Sprint(a...))
h.Log(LevelWarn, "msg", fmt.Sprint(a...))
}

// Warnf logs a message at warnf level.
func (h *Helper) Warnf(format string, a ...interface{}) {
h.logger.Log(LevelWarn, "msg", fmt.Sprintf(format, a...))
h.Log(LevelWarn, "msg", fmt.Sprintf(format, a...))
}

// Warnw logs a message at warnf level.
func (h *Helper) Warnw(keyvals ...interface{}) {
h.logger.Log(LevelWarn, keyvals...)
h.Log(LevelWarn, keyvals...)
}

// Error logs a message at error level.
func (h *Helper) Error(a ...interface{}) {
h.logger.Log(LevelError, "msg", fmt.Sprint(a...))
h.Log(LevelError, "msg", fmt.Sprint(a...))
}

// Errorf logs a message at error level.
func (h *Helper) Errorf(format string, a ...interface{}) {
h.logger.Log(LevelError, "msg", fmt.Sprintf(format, a...))
h.Log(LevelError, "msg", fmt.Sprintf(format, a...))
}

// Errorw logs a message at error level.
func (h *Helper) Errorw(keyvals ...interface{}) {
h.logger.Log(LevelError, keyvals...)
h.Log(LevelError, keyvals...)
}

// Fatal logs a message at fatal level.
func (h *Helper) Fatal(a ...interface{}) {
h.logger.Log(LevelFatal, "msg", fmt.Sprint(a...))
h.Log(LevelFatal, "msg", fmt.Sprint(a...))
os.Exit(1)
}

// Fatalf logs a message at fatal level.
func (h *Helper) Fatalf(format string, a ...interface{}) {
h.logger.Log(LevelFatal, "msg", fmt.Sprintf(format, a...))
h.Log(LevelFatal, "msg", fmt.Sprintf(format, a...))
os.Exit(1)
}

// Fatalw logs a message at fatal level.
func (h *Helper) Fatalw(keyvals ...interface{}) {
h.logger.Log(LevelFatal, keyvals...)
h.Log(LevelFatal, keyvals...)
os.Exit(1)
}
4 changes: 2 additions & 2 deletions log/std.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ func (l *stdLogger) Log(level Level, keyvals ...interface{}) error {
buf := l.pool.Get().(*bytes.Buffer)
buf.WriteString(level.String())
for i := 0; i < len(keyvals); i += 2 {
fmt.Fprintf(buf, " %s=%v", keyvals[i], keyvals[i+1])
_, _ = fmt.Fprintf(buf, " %s=%v", keyvals[i], keyvals[i+1])
}
l.log.Output(4, buf.String())
_ = l.log.Output(4, buf.String())
buf.Reset()
l.pool.Put(buf)
return nil
Expand Down
4 changes: 2 additions & 2 deletions middleware/logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func Server(logger log.Logger) middleware.Middleware {
reason = se.Reason
}
level, stack := extractError(err)
log.WithContext(ctx, logger).Log(level,
_ = log.WithContext(ctx, logger).Log(level,
"kind", "server",
"component", kind,
"operation", operation,
Expand Down Expand Up @@ -68,7 +68,7 @@ func Client(logger log.Logger) middleware.Middleware {
reason = se.Reason
}
level, stack := extractError(err)
log.WithContext(ctx, logger).Log(level,
_ = log.WithContext(ctx, logger).Log(level,
"kind", "client",
"component", kind,
"operation", operation,
Expand Down
6 changes: 4 additions & 2 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type options struct {
ctx context.Context
sigs []os.Signal

logger log.Logger
logger *log.Helper
registrar registry.Registrar
registrarTimeout time.Duration
servers []transport.Server
Expand Down Expand Up @@ -63,7 +63,9 @@ func Context(ctx context.Context) Option {

// Logger with service logger.
func Logger(logger log.Logger) Option {
return func(o *options) { o.logger = logger }
return func(o *options) {
o.logger = log.NewHelper(logger)
}
}

// Server with transport servers.
Expand Down
2 changes: 1 addition & 1 deletion options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestLogger(t *testing.T) {
o := &options{}
v := xlog.NewStdLogger(log.Writer())
Logger(v)(o)
assert.Equal(t, v, o.logger)
assert.Equal(t, xlog.NewHelper(v), o.logger)
}

type mockServer struct{}
Expand Down
4 changes: 2 additions & 2 deletions transport/grpc/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package grpc
import (
"context"
"crypto/tls"
"fmt"
"time"

"github.com/go-kratos/kratos/v2/middleware"
Expand Down Expand Up @@ -106,8 +107,7 @@ func dial(ctx context.Context, insecure bool, opts ...ClientOption) (*grpc.Clien
ints = append(ints, options.ints...)
}
var grpcOpts = []grpc.DialOption{
//todo: grpc.WithBalancerName is deprecated.
grpc.WithBalancerName(roundrobin.Name), //nolint:staticcheck
grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"LoadBalancingPolicy": "%s"}`, roundrobin.Name)),
grpc.WithChainUnaryInterceptor(ints...),
}
if options.discovery != nil {
Expand Down
5 changes: 4 additions & 1 deletion transport/grpc/resolver/direct/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@ func (d *directBuilder) Build(target resolver.Target, cc resolver.ClientConn, op
for _, addr := range strings.Split(target.Endpoint, ",") {
addrs = append(addrs, resolver.Address{Addr: addr})
}
cc.UpdateState(resolver.State{
err := cc.UpdateState(resolver.State{
Addresses: addrs,
})
if err != nil {
return nil, err
}
return newDirectResolver(), nil
}

Expand Down
10 changes: 8 additions & 2 deletions transport/grpc/resolver/discovery/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,20 @@ func (r *discoveryResolver) update(ins []*registry.ServiceInstance) {
r.log.Warnf("[resolver] Zero endpoint found,refused to write, instances: %v", ins)
return
}
r.cc.UpdateState(resolver.State{Addresses: addrs})
err := r.cc.UpdateState(resolver.State{Addresses: addrs})
if err != nil {
r.log.Errorf("[resolver] failed to update state: %s", err)
}
b, _ := json.Marshal(ins)
r.log.Infof("[resolver] update instances: %s", b)
}

func (r *discoveryResolver) Close() {
r.cancel()
r.w.Stop()
err := r.w.Stop()
if err != nil {
r.log.Errorf("[resolver] failed to watch top: %s", err)
}
}

func (r *discoveryResolver) ResolveNow(options resolver.ResolveNowOptions) {}
Expand Down
4 changes: 2 additions & 2 deletions transport/grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (s *Server) Endpoint() (*url.URL, error) {
}
addr, err := host.Extract(s.address, lis)
if err != nil {
lis.Close()
_ = lis.Close()
s.err = err
return
}
Expand Down Expand Up @@ -211,7 +211,7 @@ func (s *Server) unaryServerInterceptor() grpc.UnaryServerInterceptor {
}
reply, err := h(ctx, req)
if len(replyHeader) > 0 {
grpc.SetHeader(ctx, replyHeader)
_ = grpc.SetHeader(ctx, replyHeader)
}
return reply, err
}
Expand Down
7 changes: 5 additions & 2 deletions transport/http/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ func DefaultResponseEncoder(w http.ResponseWriter, r *http.Request, v interface{
return err
}
w.Header().Set("Content-Type", httputil.ContentType(codec.Name()))
w.Write(data)
_, err = w.Write(data)
if err != nil {
return err
}
return nil
}

Expand All @@ -60,7 +63,7 @@ func DefaultErrorEncoder(w http.ResponseWriter, r *http.Request, err error) {
}
w.Header().Set("Content-Type", httputil.ContentType(codec.Name()))
w.WriteHeader(int(se.Code))
w.Write(body)
_, _ = w.Write(body)
}

// CodecForRequest get encoding.Codec via http.Request
Expand Down
10 changes: 8 additions & 2 deletions transport/http/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,20 @@ func (c *wrapper) XML(code int, v interface{}) error {
func (c *wrapper) String(code int, text string) error {
c.res.Header().Set("Content-Type", "text/plain")
c.res.WriteHeader(code)
c.res.Write([]byte(text))
_, err := c.res.Write([]byte(text))
if err != nil {
return err
}
return nil
}

func (c *wrapper) Blob(code int, contentType string, data []byte) error {
c.res.Header().Set("Content-Type", contentType)
c.res.WriteHeader(code)
c.res.Write(data)
_, err := c.res.Write(data)
if err != nil {
return err
}
return nil
}

Expand Down
14 changes: 10 additions & 4 deletions transport/http/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,18 @@ func newResolver(ctx context.Context, discovery registry.Discovery, target *Targ
select {
case err := <-done:
if err != nil {
watcher.Stop()
err := watcher.Stop()
if err != nil {
r.logger.Errorf("failed to http client watch stop: %v", target)
}
return nil, err
}
case <-ctx.Done():
r.logger.Errorf("http client watch service %v reaching context deadline!", target)
watcher.Stop()
err := watcher.Stop()
if err != nil {
r.logger.Errorf("failed to http client watch stop: %v", target)
}
return nil, ctx.Err()
}
}
Expand All @@ -116,12 +122,12 @@ func newResolver(ctx context.Context, discovery registry.Discovery, target *Targ
func (r *resolver) update(services []*registry.ServiceInstance) {
var nodes []*registry.ServiceInstance
for _, in := range services {
endpoint, err := endpoint.ParseEndpoint(in.Endpoints, "http", !r.insecure)
ept, err := endpoint.ParseEndpoint(in.Endpoints, "http", !r.insecure)
if err != nil {
r.logger.Errorf("Failed to parse (%v) discovery endpoint: %v error %v", r.target, in.Endpoints, err)
continue
}
if endpoint == "" {
if ept == "" {
continue
}
nodes = append(nodes, in)
Expand Down
8 changes: 5 additions & 3 deletions transport/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,22 @@ type Header interface {

// Transporter is transport context value interface.
type Transporter interface {
// Kind transporter
// grpc
// http
Kind() Kind
// Endpoint return server or client endpoint
// Server Transport: grpc://127.0.0.1:9000
// Client Transport: discovery:///provider-demo
Endpoint() string
// Service full method selector generated by protobuf
// Operation Service full method selector generated by protobuf
// example: /helloworld.Greeter/SayHello
Operation() string
// request header
// RequestHeader return transport request header
// http: http.Header
// grpc: metadata.MD
RequestHeader() Header
// reply header
// ReplyHeader return transport reply/response header
// only valid for server transport
// http: http.Header
// grpc: metadata.MD
Expand Down

0 comments on commit b44e47b

Please sign in to comment.