Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error handling using pkg/errors #201

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions api/converter/from_bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@
package converter

import (
"fmt"

"github.com/gogo/protobuf/proto"
"github.com/pkg/errors"

"github.com/yorkie-team/yorkie/api"
"github.com/yorkie-team/yorkie/internal/log"
Expand All @@ -35,7 +34,7 @@ func BytesToObject(snapshot []byte) (*json.Object, error) {

pbElem := &api.JSONElement{}
if err := proto.Unmarshal(snapshot, pbElem); err != nil {
return nil, err
return nil, errors.WithStack(err)
}

obj, err := fromJSONObject(pbElem.GetJsonObject())
Expand All @@ -61,7 +60,7 @@ func fromJSONElement(pbElem *api.JSONElement) (json.Element, error) {
case *api.JSONElement_Counter_:
return fromJSONCounter(decoded.Counter)
default:
return nil, fmt.Errorf("%s: %w", decoded, ErrUnsupportedElement)
return nil, errors.Wrapf(ErrUnsupportedElement, "decoded: %s", decoded)
}
}

Expand Down
16 changes: 8 additions & 8 deletions api/converter/from_pb.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package converter

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

"github.com/yorkie-team/yorkie/api"
"github.com/yorkie-team/yorkie/pkg/document/change"
Expand Down Expand Up @@ -46,10 +46,10 @@ func FromClient(pbClient *api.Client) (*types.Client, error) {
// FromChangePack converts the given Protobuf format to model format.
func FromChangePack(pbPack *api.ChangePack) (*change.Pack, error) {
if pbPack == nil {
return nil, ErrPackRequired
return nil, errors.WithStack(ErrPackRequired)
}
if pbPack.Checkpoint == nil {
return nil, ErrCheckpointRequired
return nil, errors.WithStack(ErrCheckpointRequired)
}

changes, err := fromChanges(pbPack.Changes)
Expand Down Expand Up @@ -137,7 +137,7 @@ func FromEventType(pbDocEventType api.DocEventType) (types.DocEventType, error)
case api.DocEventType_DOCUMENTS_UNWATCHED:
return types.DocumentsUnwatchedEvent, nil
}
return "", fmt.Errorf("%v: %w", pbDocEventType, ErrUnsupportedEventType)
return "", errors.Wrapf(ErrUnsupportedEventType, "document event type: %v", pbDocEventType)
}

// FromDocEvent converts the given Protobuf format to model format.
Expand Down Expand Up @@ -185,7 +185,7 @@ func FromOperations(pbOps []*api.Operation) ([]operation.Operation, error) {
case *api.Operation_Increase_:
op, err = fromIncrease(decoded.Increase)
default:
return nil, ErrUnsupportedOperation
return nil, errors.WithStack(ErrUnsupportedOperation)
}
if err != nil {
return nil, err
Expand Down Expand Up @@ -553,7 +553,7 @@ func fromElement(pbElement *api.JSONElementSimple) (json.Element, error) {
), nil
}

return nil, fmt.Errorf("%d, %w", pbElement.Type, ErrUnsupportedElement)
return nil, errors.Wrapf(ErrUnsupportedElement, "element type: %d", pbElement.Type)
}

func fromPrimitiveValueType(valueType api.ValueType) (json.ValueType, error) {
Expand All @@ -576,7 +576,7 @@ func fromPrimitiveValueType(valueType api.ValueType) (json.ValueType, error) {
return json.Date, nil
}

return 0, fmt.Errorf("%d, %w", valueType, ErrUnsupportedValueType)
return 0, errors.Wrapf(ErrUnsupportedValueType, "value type: %d", valueType)
}

func fromCounterType(valueType api.ValueType) (json.CounterType, error) {
Expand All @@ -589,5 +589,5 @@ func fromCounterType(valueType api.ValueType) (json.CounterType, error) {
return json.DoubleCnt, nil
}

return 0, fmt.Errorf("%d, %w", valueType, ErrUnsupportedCounterType)
return 0, errors.Wrapf(ErrUnsupportedCounterType, "value type: %d", valueType)
}
8 changes: 3 additions & 5 deletions api/converter/to_bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@
package converter

import (
"fmt"
"reflect"

"github.com/gogo/protobuf/proto"
"github.com/pkg/errors"

"github.com/yorkie-team/yorkie/api"
"github.com/yorkie-team/yorkie/internal/log"
"github.com/yorkie-team/yorkie/pkg/document/json"
)

Expand All @@ -36,8 +35,7 @@ func ObjectToBytes(obj *json.Object) ([]byte, error) {

bytes, err := proto.Marshal(pbElem)
if err != nil {
log.Logger.Error(err)
return nil, err
return nil, errors.WithStack(err)
}
return bytes, nil
}
Expand All @@ -57,7 +55,7 @@ func toJSONElement(elem json.Element) (*api.JSONElement, error) {
case *json.Counter:
return toCounter(elem)
default:
return nil, fmt.Errorf("%v: %w", reflect.TypeOf(elem), ErrUnsupportedElement)
return nil, errors.Wrapf(ErrUnsupportedElement, "element type: %v", reflect.TypeOf(elem))
}
}

Expand Down
13 changes: 7 additions & 6 deletions api/converter/to_pb.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
package converter

import (
"fmt"
"reflect"

"github.com/pkg/errors"

"github.com/yorkie-team/yorkie/api"
"github.com/yorkie-team/yorkie/pkg/document/change"
"github.com/yorkie-team/yorkie/pkg/document/checkpoint"
Expand Down Expand Up @@ -133,7 +134,7 @@ func ToDocEventType(eventType types.DocEventType) (api.DocEventType, error) {
case types.DocumentsUnwatchedEvent:
return api.DocEventType_DOCUMENTS_UNWATCHED, nil
default:
return 0, fmt.Errorf("%s: %w", eventType, ErrUnsupportedEventType)
return 0, errors.Wrapf(ErrUnsupportedEventType, "document event type: %s", eventType)
}
}

Expand Down Expand Up @@ -178,7 +179,7 @@ func ToOperations(operations []operation.Operation) ([]*api.Operation, error) {
case *operation.Increase:
pbOperation.Body, err = toIncrease(op)
default:
return nil, ErrUnsupportedOperation
return nil, errors.WithStack(ErrUnsupportedOperation)
}
if err != nil {
return nil, err
Expand Down Expand Up @@ -353,7 +354,7 @@ func toJSONElementSimple(elem json.Element) (*api.JSONElementSimple, error) {
}, nil
}

return nil, fmt.Errorf("%v, %w", reflect.TypeOf(elem), ErrUnsupportedElement)
return nil, errors.Wrapf(ErrUnsupportedElement, "element type: %v", reflect.TypeOf(elem))
}

func toTextNodePos(pos *json.RGATreeSplitNodePos) *api.TextNodePos {
Expand Down Expand Up @@ -406,7 +407,7 @@ func toValueType(valueType json.ValueType) (api.ValueType, error) {
return api.ValueType_DATE, nil
}

return 0, fmt.Errorf("%d, %w", valueType, ErrUnsupportedValueType)
return 0, errors.Wrapf(ErrUnsupportedValueType, "value type: %d", valueType)
}

func toCounterType(valueType json.CounterType) (api.ValueType, error) {
Expand All @@ -419,5 +420,5 @@ func toCounterType(valueType json.CounterType) (api.ValueType, error) {
return api.ValueType_DOUBLE_CNT, nil
}

return 0, fmt.Errorf("%d, %w", valueType, ErrUnsupportedCounterType)
return 0, errors.Wrapf(ErrUnsupportedCounterType, "value type: %d", valueType)
}
38 changes: 21 additions & 17 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ package client

import (
"context"
"errors"
goerrors "errors"

"github.com/google/uuid"
"github.com/pkg/errors"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"

Expand All @@ -43,11 +44,11 @@ const (
var (
// ErrClientNotActivated occurs when an inactive client executes a function
// that can only be executed when activated.
ErrClientNotActivated = errors.New("client is not activated")
ErrClientNotActivated = goerrors.New("client is not activated")

// ErrDocumentNotAttached occurs when the given document is not attached to
// this client.
ErrDocumentNotAttached = errors.New("document is not attached")
ErrDocumentNotAttached = goerrors.New("document is not attached")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 I came here from pkg/errors#242

I tend to use fmt.Errorf for declaring error variables so the only errors package in the codebase is github.com/pkg/errors. This way I don't have to think about which errors package I'm using, it is never std errors.

)

// Client is a normal client that can communicate with the agent.
Expand Down Expand Up @@ -105,8 +106,7 @@ func NewClient(opts ...Option) (*Client, error) {
serverNameOverride,
)
if err != nil {
log.Logger.Error(err)
return nil, err
return nil, errors.WithStack(err)
}
dialOptions = append(dialOptions, grpc.WithTransportCredentials(creds))
} else {
Expand Down Expand Up @@ -150,8 +150,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 {
log.Logger.Error(err)
return err
return errors.WithStack(err)
}

c.conn = conn
Expand All @@ -163,6 +162,7 @@ func (c *Client) Dial(rpcAddr string) error {
// Close closes all resources of this client.
func (c *Client) Close() error {
if err := c.Deactivate(context.Background()); err != nil {
log.Logger.Errorf("%+v", err)
return err
}

Expand All @@ -185,14 +185,14 @@ func (c *Client) Activate(ctx context.Context) error {
response, err := c.client.ActivateClient(ctx, &api.ActivateClientRequest{
ClientKey: c.key,
})

if err != nil {
log.Logger.Error(err)
return err
}

clientID, err := time.ActorIDFromBytes(response.ClientId)
if err != nil {
log.Logger.Errorf("%+v", err)
return err
}

Expand All @@ -212,8 +212,7 @@ func (c *Client) Deactivate(ctx context.Context) error {
ClientId: c.id.Bytes(),
})
if err != nil {
log.Logger.Error(err)
return err
return errors.WithStack(err)
}

c.status = deactivated
Expand All @@ -232,6 +231,7 @@ func (c *Client) Attach(ctx context.Context, doc *document.Document) error {

pbChangePack, err := converter.ToChangePack(doc.CreateChangePack())
if err != nil {
log.Logger.Errorf("%+v", err)
return err
}

Expand All @@ -246,11 +246,12 @@ func (c *Client) Attach(ctx context.Context, doc *document.Document) error {

pack, err := converter.FromChangePack(res.ChangePack)
if err != nil {
log.Logger.Errorf("%+v", err)
return err
}

if err := doc.ApplyChangePack(pack); err != nil {
log.Logger.Error(err)
log.Logger.Errorf("%+v", err)
return err
}

Expand All @@ -268,15 +269,18 @@ func (c *Client) Attach(ctx context.Context, doc *document.Document) error {
// document is no longer used by this client, it should be detached.
func (c *Client) Detach(ctx context.Context, doc *document.Document) error {
if c.status != activated {
log.Logger.Error(ErrClientNotActivated)
return ErrClientNotActivated
}

if _, ok := c.attachedDocs[doc.Key().BSONKey()]; !ok {
log.Logger.Error(ErrClientNotActivated)
return ErrDocumentNotAttached
}

pbChangePack, err := converter.ToChangePack(doc.CreateChangePack())
if err != nil {
log.Logger.Errorf("%+v", err)
return err
}

Expand All @@ -291,11 +295,12 @@ func (c *Client) Detach(ctx context.Context, doc *document.Document) error {

pack, err := converter.FromChangePack(res.ChangePack)
if err != nil {
log.Logger.Errorf("%+v", err)
return err
}

if err := doc.ApplyChangePack(pack); err != nil {
log.Logger.Error(err)
log.Logger.Errorf("%+v", err)
return err
}

Expand All @@ -317,6 +322,7 @@ func (c *Client) Sync(ctx context.Context, keys ...*key.Key) error {

for _, k := range keys {
if err := c.sync(ctx, k); err != nil {
log.Logger.Errorf("%+v", err)
return err
}
}
Expand Down Expand Up @@ -423,12 +429,12 @@ func (c *Client) IsActive() bool {

func (c *Client) sync(ctx context.Context, key *key.Key) error {
if c.status != activated {
return ErrClientNotActivated
return errors.WithStack(ErrClientNotActivated)
}

doc, ok := c.attachedDocs[key.BSONKey()]
if !ok {
return ErrDocumentNotAttached
return errors.WithStack(ErrDocumentNotAttached)
}

pbChangePack, err := converter.ToChangePack(doc.CreateChangePack())
Expand All @@ -441,8 +447,7 @@ func (c *Client) sync(ctx context.Context, key *key.Key) error {
ChangePack: pbChangePack,
})
if err != nil {
log.Logger.Error(err)
return err
return errors.WithStack(err)
}

pack, err := converter.FromChangePack(res.ChangePack)
Expand All @@ -451,7 +456,6 @@ func (c *Client) sync(ctx context.Context, key *key.Key) error {
}

if err := doc.ApplyChangePack(pack); err != nil {
log.Logger.Error(err)
return err
}

Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
github.com/moby/locker v1.0.1
github.com/pkg/errors v0.9.1 // indirect
github.com/prometheus/client_golang v1.10.0
github.com/rs/xid v1.2.1
github.com/spf13/cobra v1.1.1
Expand Down
2 changes: 2 additions & 0 deletions internal/cli/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,12 @@ func newAgentCmd() *cobra.Command {

r, err := yorkie.New(conf)
if err != nil {
log.Logger.Errorf("%+v", err)
return err
}

if err := r.Start(); err != nil {
log.Logger.Errorf("%+v", err)
return err
}

Expand Down
2 changes: 1 addition & 1 deletion internal/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func init() {
zapcore.AddSync(os.Stdout),
zap.InfoLevel,
),
), zap.AddStacktrace(zap.ErrorLevel))
))

Logger = rawLogger.Sugar()
}
Loading