Skip to content

Commit

Permalink
Improve error handling using pkg/errors
Browse files Browse the repository at this point in the history
In the case of zap currently in use, the place where the log is
output is shown as the error occurrence point in the stack
trace. Introduced pkg/errors to allow log output to be processed
at the end of the logic.
  • Loading branch information
dc7303 committed Jun 12, 2021
1 parent 9a0f843 commit 6698ffc
Show file tree
Hide file tree
Showing 42 changed files with 247 additions and 231 deletions.
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")
)

// 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

0 comments on commit 6698ffc

Please sign in to comment.