Skip to content

Commit

Permalink
Introduce wrapcheck to prevent omitting fmt.Errorf for external errors (
Browse files Browse the repository at this point in the history
yorkie-team#420)

Since Go 1.13, error wrapping using fmt.Errorf is the recommended way
to compose errors in Go in order to add additional information.

This commit adds context to the error by wrapping the error in an
external package with fmt.Errorf. Errors in inner packages wrap
contextually.
  • Loading branch information
hackerwins authored Oct 4, 2022
1 parent 8589a8b commit 2b9b3ba
Show file tree
Hide file tree
Showing 32 changed files with 230 additions and 177 deletions.
7 changes: 7 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ linters:
- lll
- misspell
- nakedret
- wrapcheck
disable:
- gosimple
- staticcheck
Expand All @@ -27,6 +28,12 @@ linters-settings:
gosec:
excludes:
- G107 # Potential HTTP request made with variable url
wrapcheck:
ignoreSigRegexps:
- github.com/yorkie-team/yorkie # Ignore all methods in internal package
- google.golang.org/grpc/status # Ignore all methods in grpc/status package
- google.golang.org/grpc/internal/status # Ignore all methods in grpc/internal/status package
- context.Context # Ignore all methods in context package

issues:
exclude-use-default: false
11 changes: 8 additions & 3 deletions admin/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package admin

import (
"context"
"fmt"

"go.uber.org/zap"
"google.golang.org/grpc"
Expand Down Expand Up @@ -80,7 +81,7 @@ func New(opts ...Option) (*Client, error) {
if logger == nil {
l, err := zap.NewProduction()
if err != nil {
return nil, err
return nil, fmt.Errorf("create logger: %w", err)
}
logger = l
}
Expand Down Expand Up @@ -110,7 +111,7 @@ func Dial(adminAddr string, opts ...Option) (*Client, error) {
func (c *Client) Dial(adminAddr string) error {
conn, err := grpc.Dial(adminAddr, c.dialOptions...)
if err != nil {
return err
return fmt.Errorf("dial to %s: %w", adminAddr, err)
}

c.conn = conn
Expand All @@ -121,7 +122,11 @@ func (c *Client) Dial(adminAddr string) error {

// Close closes the connection to the admin service.
func (c *Client) Close() error {
return c.conn.Close()
if err := c.conn.Close(); err != nil {
return fmt.Errorf("close connection: %w", err)
}

return nil
}

// LogIn logs in a user.
Expand Down
2 changes: 1 addition & 1 deletion api/converter/from_bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func BytesToObject(snapshot []byte) (*crdt.Object, error) {

pbElem := &api.JSONElement{}
if err := proto.Unmarshal(snapshot, pbElem); err != nil {
return nil, err
return nil, fmt.Errorf("unmarshal element: %w", err)
}

obj, err := fromJSONObject(pbElem.GetJsonObject())
Expand Down
12 changes: 6 additions & 6 deletions api/converter/from_pb.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
func FromUser(pbUser *api.User) (*types.User, error) {
createdAt, err := protoTypes.TimestampFromProto(pbUser.CreatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert createdAt to timestamp: %w", err)
}

return &types.User{
Expand All @@ -62,11 +62,11 @@ func FromProjects(pbProjects []*api.Project) ([]*types.Project, error) {
func FromProject(pbProject *api.Project) (*types.Project, error) {
createdAt, err := protoTypes.TimestampFromProto(pbProject.CreatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert createdAt to timestamp: %w", err)
}
updatedAt, err := protoTypes.TimestampFromProto(pbProject.UpdatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert updatedAt to timestamp: %w", err)
}
return &types.Project{
ID: types.ID(pbProject.Id),
Expand Down Expand Up @@ -97,15 +97,15 @@ func FromDocumentSummaries(pbSummaries []*api.DocumentSummary) ([]*types.Documen
func FromDocumentSummary(pbSummary *api.DocumentSummary) (*types.DocumentSummary, error) {
createdAt, err := protoTypes.TimestampFromProto(pbSummary.CreatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert createdAt to timestamp: %w", err)
}
accessedAt, err := protoTypes.TimestampFromProto(pbSummary.AccessedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert accessedAt to timestamp: %w", err)
}
updatedAt, err := protoTypes.TimestampFromProto(pbSummary.UpdatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert updatedAt to timestamp: %w", err)
}
return &types.DocumentSummary{
ID: types.ID(pbSummary.Id),
Expand Down
2 changes: 1 addition & 1 deletion api/converter/to_bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func ObjectToBytes(obj *crdt.Object) ([]byte, error) {

bytes, err := proto.Marshal(pbElem)
if err != nil {
return nil, err
return nil, fmt.Errorf("marshal JSON element to bytes: %w", err)
}
return bytes, nil
}
Expand Down
12 changes: 6 additions & 6 deletions api/converter/to_pb.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import (
func ToUser(user *types.User) (*api.User, error) {
pbCreatedAt, err := protoTypes.TimestampProto(user.CreatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert createdAt to protobuf: %w", err)
}

return &api.User{
Expand Down Expand Up @@ -64,11 +64,11 @@ func ToProjects(projects []*types.Project) ([]*api.Project, error) {
func ToProject(project *types.Project) (*api.Project, error) {
pbCreatedAt, err := protoTypes.TimestampProto(project.CreatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert createdAt to protobuf: %w", err)
}
pbUpdatedAt, err := protoTypes.TimestampProto(project.UpdatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert updatedAt to protobuf: %w", err)
}

return &api.Project{
Expand Down Expand Up @@ -100,15 +100,15 @@ func ToDocumentSummaries(summaries []*types.DocumentSummary) ([]*api.DocumentSum
func ToDocumentSummary(summary *types.DocumentSummary) (*api.DocumentSummary, error) {
pbCreatedAt, err := protoTypes.TimestampProto(summary.CreatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert createdAt to protobuf: %w", err)
}
pbAccessedAt, err := protoTypes.TimestampProto(summary.AccessedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert accessedAt to protobuf: %w", err)
}
pbUpdatedAt, err := protoTypes.TimestampProto(summary.UpdatedAt)
if err != nil {
return nil, err
return nil, fmt.Errorf("convert updatedAt to protobuf: %w", err)
}

return &api.DocumentSummary{
Expand Down
9 changes: 7 additions & 2 deletions api/types/auth_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,13 @@ func NewAuthWebhookResponse(reader io.Reader) (*AuthWebhookResponse, error) {
func (r *AuthWebhookResponse) Write(writer io.Writer) (int, error) {
resBody, err := json.Marshal(r)
if err != nil {
return 0, err
return 0, fmt.Errorf("marshal response: %w", err)
}

return writer.Write(resBody)
count, err := writer.Write(resBody)
if err != nil {
return 0, fmt.Errorf("write response: %w", err)
}

return count, nil
}
5 changes: 3 additions & 2 deletions api/types/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package types

import (
"encoding/json"
"fmt"

"github.com/yorkie-team/yorkie/pkg/document/time"
)
Expand All @@ -17,7 +18,7 @@ func NewClient(encoded []byte) (*Client, error) {
cli := &Client{}
err := json.Unmarshal(encoded, cli)
if err != nil {
return nil, err
return nil, fmt.Errorf("unmarshal client: %w", err)
}
return cli, nil
}
Expand All @@ -26,7 +27,7 @@ func NewClient(encoded []byte) (*Client, error) {
func (c *Client) Marshal() (string, error) {
encoded, err := json.Marshal(c)
if err != nil {
return "", err
return "", fmt.Errorf("marshal client: %w", err)
}

return string(encoded), nil
Expand Down
2 changes: 1 addition & 1 deletion api/types/id.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (id *ID) String() string {
func (id *ID) Bytes() ([]byte, error) {
decoded, err := hex.DecodeString(id.String())
if err != nil {
return nil, err
return nil, fmt.Errorf("decode hex string: %w", err)
}
return decoded, nil
}
Expand Down
11 changes: 8 additions & 3 deletions api/types/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
package types

import (
"fmt"

"github.com/go-playground/locales/en"
ut "github.com/go-playground/universal-translator"
"github.com/go-playground/validator/v10"
en_translations "github.com/go-playground/validator/v10/translations/en"
entranslations "github.com/go-playground/validator/v10/translations/en"
)

var (
Expand Down Expand Up @@ -68,7 +70,10 @@ func registerValidation(tag string, fn validator.Func) {
// that registers translations against the provided tag with given msg.
func registerTranslation(tag, msg string) {
if err := defaultValidator.RegisterTranslation(tag, trans, func(ut ut.Translator) error {
return ut.Add(tag, msg, true)
if err := ut.Add(tag, msg, true); err != nil {
return fmt.Errorf("add translation: %w", err)
}
return nil
}, func(ut ut.Translator, fe validator.FieldError) string {
t, _ := ut.T(tag, fe.Field())
return t
Expand All @@ -78,7 +83,7 @@ func registerTranslation(tag, msg string) {
}

func init() {
if err := en_translations.RegisterDefaultTranslations(defaultValidator, trans); err != nil {
if err := entranslations.RegisterDefaultTranslations(defaultValidator, trans); err != nil {
panic(err)
}
}
12 changes: 8 additions & 4 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func New(opts ...Option) (*Client, error) {
if options.CertFile != "" {
creds, err := credentials.NewClientTLSFromFile(options.CertFile, options.ServerNameOverride)
if err != nil {
return nil, err
return nil, fmt.Errorf("create client tls from file: %w", err)
}
transportCreds = grpc.WithTransportCredentials(creds)
}
Expand All @@ -136,7 +136,7 @@ func New(opts ...Option) (*Client, error) {
if logger == nil {
l, err := zap.NewProduction()
if err != nil {
return nil, err
return nil, fmt.Errorf("create logger: %w", err)
}
logger = l
}
Expand Down Expand Up @@ -170,7 +170,7 @@ func Dial(rpcAddr string, opts ...Option) (*Client, error) {
func (c *Client) Dial(rpcAddr string) error {
conn, err := grpc.Dial(rpcAddr, c.dialOptions...)
if err != nil {
return err
return fmt.Errorf("dial to %s: %w", rpcAddr, err)
}

c.conn = conn
Expand All @@ -185,7 +185,11 @@ func (c *Client) Close() error {
return err
}

return c.conn.Close()
if err := c.conn.Close(); err != nil {
return fmt.Errorf("close connection: %w", err)
}

return nil
}

// Activate activates this client. That is, it registers itself to the server
Expand Down
9 changes: 5 additions & 4 deletions cmd/yorkie/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package config

import (
"encoding/json"
"fmt"
"os"
"path"
)
Expand Down Expand Up @@ -70,15 +71,15 @@ func Load() (*Config, error) {
return New(), nil
}

return nil, err
return nil, fmt.Errorf("open config file: %w", err)
}
defer func() {
_ = file.Close()
}()

var config *Config
if err := json.NewDecoder(file).Decode(&config); err != nil {
return nil, err
return nil, fmt.Errorf("decode config file: %w", err)
}

return config, nil
Expand All @@ -88,14 +89,14 @@ func Load() (*Config, error) {
func Save(config *Config) error {
file, err := os.Create(configPath())
if err != nil {
return err
return fmt.Errorf("create config file: %w", err)
}
defer func() {
_ = file.Close()
}()

if err := json.NewEncoder(file).Encode(config); err != nil {
return err
return fmt.Errorf("encode config file: %w", err)
}

return nil
Expand Down
3 changes: 2 additions & 1 deletion cmd/yorkie/project/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"

"github.com/spf13/cobra"
"google.golang.org/genproto/googleapis/rpc/errdetails"
Expand Down Expand Up @@ -70,7 +71,7 @@ func newCreateCommand() *cobra.Command {

encoded, err := json.Marshal(project)
if err != nil {
return err
return fmt.Errorf("marshal project: %w", err)
}

cmd.Println(string(encoded))
Expand Down
3 changes: 2 additions & 1 deletion cmd/yorkie/project/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"

"github.com/spf13/cobra"
"google.golang.org/genproto/googleapis/rpc/errdetails"
Expand Down Expand Up @@ -100,7 +101,7 @@ func newUpdateCommand() *cobra.Command {

encoded, err := json.Marshal(updated)
if err != nil {
return err
return fmt.Errorf("marshal project: %w", err)
}

cmd.Println(string(encoded))
Expand Down
9 changes: 7 additions & 2 deletions pkg/document/time/actor_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,14 @@ func (id *ActorID) Compare(other *ActorID) int {
// MarshalJSON ensures that when calling json.Marshal(),
// it is marshaled including private field.
func (id *ActorID) MarshalJSON() ([]byte, error) {
return json.Marshal(&struct{ Bytes [actorIDSize]byte }{
result, err := json.Marshal(&struct{ Bytes [actorIDSize]byte }{
Bytes: id.bytes,
})
if err != nil {
return nil, fmt.Errorf("marshal JSON: %w", err)
}

return result, nil
}

// UnmarshalJSON ensures that when calling json.Unmarshal(),
Expand All @@ -139,7 +144,7 @@ func (id *ActorID) UnmarshalJSON(bytes []byte) error {
Bytes: id.bytes,
})
if err := json.Unmarshal(bytes, temp); err != nil {
return err
return fmt.Errorf("unmarshal JSON: %w", err)
}

id.bytes = temp.Bytes
Expand Down
Loading

0 comments on commit 2b9b3ba

Please sign in to comment.