Skip to content

Commit

Permalink
error: Export fields and remove methods.
Browse files Browse the repository at this point in the history
Rather than exposing trivial getter methods to fetch the fields of the Error
type, simply export the fields. This is simpler and more idiomatic.

This is a breaking change for code that extracts the *Error value.
In particular, uses of the Error type methods will need to be updated to remove
the call brackets, e.g.,

   e.Code()    ⇒ e.Code
   e.Message() ⇒ e.Message

See issue #46.
  • Loading branch information
creachadair committed May 15, 2021
1 parent e03b0f0 commit 5b6bbf0
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 49 deletions.
2 changes: 1 addition & 1 deletion base.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ func isNull(msg json.RawMessage) bool {
// filterError filters an *Error value to distinguish context errors from other
// error types. If err is not a context error, it is returned unchanged.
func filterError(e *Error) error {
switch e.code {
switch e.Code {
case code.Cancelled:
return context.Canceled
case code.DeadlineExceeded:
Expand Down
8 changes: 4 additions & 4 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ func (c *Client) deliver(rsp *jmessage) {
p.ch <- &jmessage{
ID: rsp.ID,
E: &Error{
code: code.InvalidRequest,
message: fmt.Sprintf("incorrect version marker %q", rsp.V),
Code: code.InvalidRequest,
Message: fmt.Sprintf("incorrect version marker %q", rsp.V),
},
}
c.log("Invalid response for ID %q", id)
Expand Down Expand Up @@ -255,9 +255,9 @@ func (c *Client) waitComplete(pctx context.Context, id string, p *Response) {

var jerr *Error
if c.err != nil && !isUninteresting(c.err) {
jerr = &Error{code: code.InternalError, message: c.err.Error()}
jerr = &Error{Code: code.InternalError, Message: c.err.Error()}
} else if err != nil {
jerr = &Error{code: code.FromError(err), message: err.Error()}
jerr = &Error{Code: code.FromError(err), Message: err.Error()}
}

p.ch <- &jmessage{
Expand Down
22 changes: 11 additions & 11 deletions code/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,27 @@ func (c Code) String() string {
return fmt.Sprintf("error code %d", c)
}

// A Coder is a value that can report an error code value.
type Coder interface {
Code() Code
// An ErrCoder is a value that can report an error code value.
type ErrCoder interface {
ErrCode() Code
}

// A codeError wraps a Code to satisfy the standard error interface. This
// indirection prevents a Code from accidentally being used as an error value.
// It also satisfies the Coder interface, allowing the code to be recovered.
// It also satisfies the ErrCoder interface, allowing the code to be recovered.
type codeError Code

// Error satisfies the error interface using the registered string for the
// code, if one is defined, or else a placeholder that describes the value.
func (c codeError) Error() string { return Code(c).String() }

// Code trivially satisfies the Coder interface.
func (c codeError) Code() Code { return Code(c) }
// ErrCode trivially satisfies the ErrCoder interface.
func (c codeError) ErrCode() Code { return Code(c) }

// Is reports whether err is c or has a code equal to c.
func (c codeError) Is(err error) bool {
v, ok := err.(Coder) // including codeError
return ok && v.Code() == Code(c)
v, ok := err.(ErrCoder) // including codeError
return ok && v.ErrCode() == Code(c)
}

// Err converts c to an error value, which is nil for code.NoError and
Expand Down Expand Up @@ -102,17 +102,17 @@ func Register(value int32, message string) Code {

// FromError returns a Code to categorize the specified error.
// If err == nil, it returns code.NoError.
// If err is a Coder, it returns the reported code value.
// If err is an ErrCoder, it returns the reported code value.
// If err is context.Canceled, it returns code.Cancelled.
// If err is context.DeadlineExceeded, it returns code.DeadlineExceeded.
// Otherwise it returns code.SystemError.
func FromError(err error) Code {
if err == nil {
return NoError
}
var c Coder
var c ErrCoder
if errors.As(err, &c) {
return c.Code()
return c.ErrCode()
} else if errors.Is(err, context.Canceled) {
return Cancelled
} else if errors.Is(err, context.DeadlineExceeded) {
Expand Down
4 changes: 2 additions & 2 deletions code/code_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ func TestRegistrationError(t *testing.T) {

type testCoder Code

func (t testCoder) Code() Code { return Code(t) }
func (testCoder) Error() string { return "bogus" }
func (t testCoder) ErrCode() Code { return Code(t) }
func (testCoder) Error() string { return "bogus" }

func TestFromError(t *testing.T) {
tests := []struct {
Expand Down
35 changes: 16 additions & 19 deletions error.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,35 +10,32 @@ import (

// Error is the concrete type of errors returned from RPC calls.
type Error struct {
message string
code code.Code
data json.RawMessage
Message string // the human-readable error message
Code code.Code // the machine-readable error code
Data json.RawMessage // optional ancillary error data
}

// Error renders e to a human-readable string for the error interface.
func (e Error) Error() string { return fmt.Sprintf("[%d] %s", e.code, e.message) }

// Code returns the error code value associated with e.
func (e Error) Code() code.Code { return e.code }

// Message returns the message string associated with e.
func (e Error) Message() string { return e.message }
func (e Error) Error() string { return fmt.Sprintf("[%d] %s", e.Code, e.Message) }

// HasData reports whether e has error data to unmarshal.
func (e Error) HasData() bool { return len(e.data) != 0 }
func (e Error) HasData() bool { return len(e.Data) != 0 }

// ErrCode trivially satisfies the code.ErrCoder interface for an *Error.
func (e *Error) ErrCode() code.Code { return e.Code }

// UnmarshalData decodes the error data associated with e into v. It returns
// UnmarshalData decodes the error data associated with e into v. It reports
// ErrNoData without modifying v if there was no data message attached to e.
func (e Error) UnmarshalData(v interface{}) error {
if !e.HasData() {
return ErrNoData
}
return json.Unmarshal([]byte(e.data), v)
return json.Unmarshal(e.Data, v)
}

// MarshalJSON implements the json.Marshaler interface for Error values.
func (e Error) MarshalJSON() ([]byte, error) {
return json.Marshal(jerror{C: int32(e.code), M: e.message, D: e.data})
return json.Marshal(jerror{C: int32(e.Code), M: e.Message, D: e.Data})
}

// UnmarshalJSON implements the json.Unmarshaler interface for Error values.
Expand All @@ -47,9 +44,9 @@ func (e *Error) UnmarshalJSON(data []byte) error {
if err := json.Unmarshal(data, &v); err != nil {
return err
}
e.code = code.Code(v.C)
e.message = v.M
e.data = v.D
e.Code = code.Code(v.C)
e.Message = v.M
e.Data = v.D
return nil
}

Expand Down Expand Up @@ -79,10 +76,10 @@ func Errorf(code code.Code, msg string, args ...interface{}) error {
// specified code, error data, and formatted message string.
// If v == nil this behaves identically to Errorf(code, msg, args...).
func DataErrorf(code code.Code, v interface{}, msg string, args ...interface{}) error {
e := &Error{code: code, message: fmt.Sprintf(msg, args...)}
e := &Error{Code: code, Message: fmt.Sprintf(msg, args...)}
if v != nil {
if data, err := json.Marshal(v); err == nil {
e.data = data
e.Data = data
}
}
return e
Expand Down
2 changes: 1 addition & 1 deletion internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func TestClientCancellation(t *testing.T) {
rsp := rsps[0]
rsp.wait()
if err := rsp.Error(); err != nil {
if err.code != code.Cancelled {
if err.Code != code.Cancelled {
t.Errorf("Response error for %q: got %v, want %v", rsp.ID(), err, code.Cancelled)
}
} else {
Expand Down
17 changes: 11 additions & 6 deletions jrpc2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ import (
"github.com/google/go-cmp/cmp"
)

// Static type assertions.
var (
_ code.ErrCoder = (*jrpc2.Error)(nil)
)

var notAuthorized = code.Register(-32095, "request not authorized")

var testOK = handler.New(func(ctx context.Context) (string, error) {
Expand Down Expand Up @@ -284,8 +289,8 @@ func TestErrorOnly(t *testing.T) {
t.Errorf("ErrorOnly: got %+v, want error", rsp)
} else if e, ok := err.(*jrpc2.Error); !ok {
t.Errorf("ErrorOnly: got %v, want *Error", err)
} else if e.Code() != 1 || e.Message() != errMessage {
t.Errorf("ErrorOnly: got (%s, %s), want (1, %s)", e.Code(), e.Message(), errMessage)
} else if e.Code != 1 || e.Message != errMessage {
t.Errorf("ErrorOnly: got (%s, %s), want (1, %s)", e.Code, e.Message, errMessage)
} else {
var data json.RawMessage
if err, want := e.UnmarshalData(&data), jrpc2.ErrNoData; err != want {
Expand Down Expand Up @@ -456,11 +461,11 @@ func TestErrors(t *testing.T) {
if got, err := c.Call(context.Background(), "Err", nil); err == nil {
t.Errorf("Call(Push): got %#v, wanted error", got)
} else if e, ok := err.(*jrpc2.Error); ok {
if e.Code() != errCode {
t.Errorf("Error code: got %d, want %d", e.Code(), errCode)
if e.Code != errCode {
t.Errorf("Error code: got %d, want %d", e.Code, errCode)
}
if e.Message() != errMessage {
t.Errorf("Error message: got %q, want %q", e.Message(), errMessage)
if e.Message != errMessage {
t.Errorf("Error message: got %q, want %q", e.Message, errMessage)
}
var data json.RawMessage
if err := e.UnmarshalData(&data); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func (c *ClientOptions) handleCallback() func(*jmessage) []byte {
if e, ok := err.(*Error); ok {
rsp.E = e
} else {
rsp.E = &Error{code: code.FromError(err), message: err.Error()}
rsp.E = &Error{Code: code.FromError(err), Message: err.Error()}
}
}
bits, _ := json.Marshal(rsp)
Expand Down
8 changes: 4 additions & 4 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ func (s *Server) stop(err error) {
for id, rsp := range s.call {
rsp.ch <- &jmessage{
ID: json.RawMessage(id),
E: &Error{message: "client channel terminated", code: code.Cancelled},
E: &Error{Message: "client channel terminated", Code: code.Cancelled},
}
delete(s.call, id)
}
Expand Down Expand Up @@ -638,7 +638,7 @@ func (s *Server) pushError(err error) {
if e, ok := err.(*Error); ok {
jerr = e
} else {
jerr = &Error{code: code.FromError(err), message: err.Error()}
jerr = &Error{Code: code.FromError(err), Message: err.Error()}
}

nw, err := encode(s.ch, jmessages{{
Expand Down Expand Up @@ -711,9 +711,9 @@ func (ts tasks) responses(rpcLog RPCLogger) jmessages {
} else if e, ok := task.err.(*Error); ok {
rsp.E = e
} else if c := code.FromError(task.err); c != code.NoError {
rsp.E = &Error{code: c, message: task.err.Error()}
rsp.E = &Error{Code: c, Message: task.err.Error()}
} else {
rsp.E = &Error{code: code.InternalError, message: task.err.Error()}
rsp.E = &Error{Code: code.InternalError, Message: task.err.Error()}
}
rpcLog.LogResponse(task.ctx, &Response{
id: string(rsp.ID),
Expand Down

0 comments on commit 5b6bbf0

Please sign in to comment.