From 21f6b6ba67a4c78ee8395a0c330f5639aa896ae0 Mon Sep 17 00:00:00 2001 From: Ross McFarlane Date: Wed, 7 Mar 2018 10:28:20 +0000 Subject: [PATCH 1/3] Handle JSON RPC errors. Resolves issue #672. --- transport/http/jsonrpc/client.go | 24 ++++++++ transport/http/jsonrpc/client_test.go | 85 +++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/transport/http/jsonrpc/client.go b/transport/http/jsonrpc/client.go index ca57bbf1d..bd80edf96 100644 --- a/transport/http/jsonrpc/client.go +++ b/transport/http/jsonrpc/client.go @@ -27,6 +27,7 @@ type Client struct { dec DecodeResponseFunc before []httptransport.RequestFunc after []httptransport.ClientResponseFunc + errf ErrorFunc finalizer httptransport.ClientFinalizerFunc requestID RequestIDGenerator bufferedStream bool @@ -53,6 +54,7 @@ func NewClient( dec: DefaultResponseDecoder, before: []httptransport.RequestFunc{}, after: []httptransport.ClientResponseFunc{}, + errf: DefaultErrorFunc, requestID: NewAutoIncrementID(0), bufferedStream: false, } @@ -117,6 +119,23 @@ func ClientResponseDecoder(dec DecodeResponseFunc) ClientOption { return func(c *Client) { c.dec = dec } } +// ErrorFunc intercepts RPC errors from responses before they are returned from +// the endpoint. This gives the server an opportunity to modify the error, or +// suppress it altogether. The return values of the ErrorFunc are used as the +// return values of the Endpoint. +type ErrorFunc func(Error) (interface{}, error) + +// ClientErrorFunc sets the func used to intercept response errors. +// If not set, DefaultErrorFunc is used. +func ClientErrorFunc(errf ErrorFunc) ClientOption { + return func(c *Client) { c.errf = errf } +} + +// DefaultErrorFunc passes its error through without modification. +func DefaultErrorFunc(err Error) (interface{}, error) { + return nil, err +} + // RequestIDGenerator returns an ID for the request. type RequestIDGenerator interface { Generate() interface{} @@ -199,6 +218,11 @@ func (c Client) Endpoint() endpoint.Endpoint { return nil, err } + // Handle the RPC error, if any. + if rpcRes.Error != nil { + return c.errf(*rpcRes.Error) + } + for _, f := range c.after { ctx = f(ctx, resp) } diff --git a/transport/http/jsonrpc/client_test.go b/transport/http/jsonrpc/client_test.go index fe42f239d..d80bb1773 100644 --- a/transport/http/jsonrpc/client_test.go +++ b/transport/http/jsonrpc/client_test.go @@ -206,6 +206,91 @@ func TestCanUseDefaults(t *testing.T) { } } +func TestClientCanHandleJSONRPCError(t *testing.T) { + var testbody = `{ + "jsonrpc": "2.0", + "error": { + "code": -32603, + "message": "Bad thing happened." + } + }` + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(testbody)) + })) + + sut := jsonrpc.NewClient(mustParse(server.URL), "add") + + _, err := sut.Endpoint()(context.Background(), 5) + if err == nil { + t.Fatal("Expected error, got none.") + } + + { + want := "Bad thing happened." + got := err.Error() + if got != want { + t.Fatalf("error message: want=%s, got=%s", want, got) + } + } + + type errorCoder interface { + ErrorCode() int + } + ec, ok := err.(errorCoder) + if !ok { + t.Fatal("Error is not errorCoder") + } + + { + want := -32603 + got := ec.ErrorCode() + if got != want { + t.Fatalf("error code: want=%d, got=%d", want, got) + } + } +} + +func TestClientCanHandleJSONRPCErrorWithErrorFunc(t *testing.T) { + var testbody = `{ + "jsonrpc": "2.0", + "error": { + "code": -32603, + "message": "Bad thing happened." + } + }` + + errfunc := func(err jsonrpc.Error) (interface{}, error) { + want := -32603 + got := err.Code + if got != want { + t.Fatalf("error code: want=%d, got=%d", want, got) + } + + return 5, nil + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(testbody)) + })) + + sut := jsonrpc.NewClient( + mustParse(server.URL), + "add", + jsonrpc.ClientErrorFunc(errfunc), + ) + + got, err := sut.Endpoint()(context.Background(), 5) + if err != nil { + t.Fatal(err) + } + + want := 5 + if got != want { + t.Fatalf("result: want=%d, got=%d", want, got) + } +} + func TestDefaultAutoIncrementer(t *testing.T) { sut := jsonrpc.NewAutoIncrementID(0) var want uint64 From 47e81863bfc849ab949e38ddae53f9da87d415e6 Mon Sep 17 00:00:00 2001 From: Ross McFarlane Date: Mon, 12 Mar 2018 09:51:20 +0000 Subject: [PATCH 2/3] Refactor decode func to receive JSON RPC response. Remove error func. --- examples/addsvc/pkg/addtransport/jsonrpc.go | 18 ++++++--- transport/http/jsonrpc/client.go | 36 ++++------------- transport/http/jsonrpc/client_test.go | 44 +-------------------- transport/http/jsonrpc/encode_decode.go | 6 +-- 4 files changed, 25 insertions(+), 79 deletions(-) diff --git a/examples/addsvc/pkg/addtransport/jsonrpc.go b/examples/addsvc/pkg/addtransport/jsonrpc.go index 9508e81e1..a23ceadfe 100644 --- a/examples/addsvc/pkg/addtransport/jsonrpc.go +++ b/examples/addsvc/pkg/addtransport/jsonrpc.go @@ -137,9 +137,12 @@ func encodeSumResponse(_ context.Context, obj interface{}) (json.RawMessage, err return b, nil } -func decodeSumResponse(_ context.Context, msg json.RawMessage) (interface{}, error) { - var res addendpoint.SumResponse - err := json.Unmarshal(msg, &res) +func decodeSumResponse(_ context.Context, res jsonrpc.Response) (interface{}, error) { + if res.Error != nil { + return nil, *res.Error + } + var sumres addendpoint.SumResponse + err := json.Unmarshal(res.Result, &sumres) if err != nil { return nil, fmt.Errorf("couldn't unmarshal body to SumResponse: %s", err) } @@ -185,9 +188,12 @@ func encodeConcatResponse(_ context.Context, obj interface{}) (json.RawMessage, return b, nil } -func decodeConcatResponse(_ context.Context, msg json.RawMessage) (interface{}, error) { - var res addendpoint.ConcatResponse - err := json.Unmarshal(msg, &res) +func decodeConcatResponse(_ context.Context, res jsonrpc.Response) (interface{}, error) { + if res.Error != nil { + return nil, *res.Error + } + var concatres addendpoint.ConcatResponse + err := json.Unmarshal(res.Result, &concatres) if err != nil { return nil, fmt.Errorf("couldn't unmarshal body to ConcatResponse: %s", err) } diff --git a/transport/http/jsonrpc/client.go b/transport/http/jsonrpc/client.go index bd80edf96..de17fd5bd 100644 --- a/transport/http/jsonrpc/client.go +++ b/transport/http/jsonrpc/client.go @@ -27,7 +27,6 @@ type Client struct { dec DecodeResponseFunc before []httptransport.RequestFunc after []httptransport.ClientResponseFunc - errf ErrorFunc finalizer httptransport.ClientFinalizerFunc requestID RequestIDGenerator bufferedStream bool @@ -54,7 +53,6 @@ func NewClient( dec: DefaultResponseDecoder, before: []httptransport.RequestFunc{}, after: []httptransport.ClientResponseFunc{}, - errf: DefaultErrorFunc, requestID: NewAutoIncrementID(0), bufferedStream: false, } @@ -69,10 +67,14 @@ func DefaultRequestEncoder(_ context.Context, req interface{}) (json.RawMessage, return json.Marshal(req) } -// DefaultResponseDecoder unmarshals the given JSON to interface{}. -func DefaultResponseDecoder(_ context.Context, res json.RawMessage) (interface{}, error) { +// DefaultResponseDecoder unmarshals the result to interface{}, or returns an +// error, if found. +func DefaultResponseDecoder(_ context.Context, res Response) (interface{}, error) { + if res.Error != nil { + return nil, *res.Error + } var result interface{} - err := json.Unmarshal(res, &result) + err := json.Unmarshal(res.Result, &result) if err != nil { return nil, err } @@ -119,23 +121,6 @@ func ClientResponseDecoder(dec DecodeResponseFunc) ClientOption { return func(c *Client) { c.dec = dec } } -// ErrorFunc intercepts RPC errors from responses before they are returned from -// the endpoint. This gives the server an opportunity to modify the error, or -// suppress it altogether. The return values of the ErrorFunc are used as the -// return values of the Endpoint. -type ErrorFunc func(Error) (interface{}, error) - -// ClientErrorFunc sets the func used to intercept response errors. -// If not set, DefaultErrorFunc is used. -func ClientErrorFunc(errf ErrorFunc) ClientOption { - return func(c *Client) { c.errf = errf } -} - -// DefaultErrorFunc passes its error through without modification. -func DefaultErrorFunc(err Error) (interface{}, error) { - return nil, err -} - // RequestIDGenerator returns an ID for the request. type RequestIDGenerator interface { Generate() interface{} @@ -218,16 +203,11 @@ func (c Client) Endpoint() endpoint.Endpoint { return nil, err } - // Handle the RPC error, if any. - if rpcRes.Error != nil { - return c.errf(*rpcRes.Error) - } - for _, f := range c.after { ctx = f(ctx, resp) } - return c.dec(ctx, rpcRes.Result) + return c.dec(ctx, rpcRes) } } diff --git a/transport/http/jsonrpc/client_test.go b/transport/http/jsonrpc/client_test.go index d80bb1773..3b4f1ea51 100644 --- a/transport/http/jsonrpc/client_test.go +++ b/transport/http/jsonrpc/client_test.go @@ -62,12 +62,12 @@ func TestClientHappyPath(t *testing.T) { fin = func(ctx context.Context, err error) { finalizerCalled = true } - decode = func(ctx context.Context, res json.RawMessage) (interface{}, error) { + decode = func(ctx context.Context, res jsonrpc.Response) (interface{}, error) { if ac := ctx.Value(afterCalledKey); ac == nil { t.Fatal("after not called") } var result int - err := json.Unmarshal(res, &result) + err := json.Unmarshal(res.Result, &result) if err != nil { return nil, err } @@ -251,46 +251,6 @@ func TestClientCanHandleJSONRPCError(t *testing.T) { } } -func TestClientCanHandleJSONRPCErrorWithErrorFunc(t *testing.T) { - var testbody = `{ - "jsonrpc": "2.0", - "error": { - "code": -32603, - "message": "Bad thing happened." - } - }` - - errfunc := func(err jsonrpc.Error) (interface{}, error) { - want := -32603 - got := err.Code - if got != want { - t.Fatalf("error code: want=%d, got=%d", want, got) - } - - return 5, nil - } - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - w.Write([]byte(testbody)) - })) - - sut := jsonrpc.NewClient( - mustParse(server.URL), - "add", - jsonrpc.ClientErrorFunc(errfunc), - ) - - got, err := sut.Endpoint()(context.Background(), 5) - if err != nil { - t.Fatal(err) - } - - want := 5 - if got != want { - t.Fatalf("result: want=%d, got=%d", want, got) - } -} - func TestDefaultAutoIncrementer(t *testing.T) { sut := jsonrpc.NewAutoIncrementID(0) var want uint64 diff --git a/transport/http/jsonrpc/encode_decode.go b/transport/http/jsonrpc/encode_decode.go index ab7612e5b..39618a1d5 100644 --- a/transport/http/jsonrpc/encode_decode.go +++ b/transport/http/jsonrpc/encode_decode.go @@ -40,9 +40,9 @@ type EncodeResponseFunc func(context.Context, interface{}) (response json.RawMes // JSON encodes the object directly. type EncodeRequestFunc func(context.Context, interface{}) (request json.RawMessage, err error) -// DecodeResponseFunc extracts a user-domain response object from an HTTP -// request object. It's designed to be used in JSON RPC clients, for +// DecodeResponseFunc extracts a user-domain response object from an JSONRPC +// response object. It's designed to be used in JSON RPC clients, for // client-side endpoints. One straightforward DecodeRequestFunc could be // something that JSON decodes from the request body to the concrete // response type. -type DecodeResponseFunc func(context.Context, json.RawMessage) (response interface{}, err error) +type DecodeResponseFunc func(context.Context, Response) (response interface{}, err error) From 0870aaa6cd6adef155322eee67a4682534a06712 Mon Sep 17 00:00:00 2001 From: Ross McFarlane Date: Mon, 12 Mar 2018 17:34:31 +0000 Subject: [PATCH 3/3] Comment tweaks. --- transport/http/jsonrpc/encode_decode.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/transport/http/jsonrpc/encode_decode.go b/transport/http/jsonrpc/encode_decode.go index 39618a1d5..1a02ec19e 100644 --- a/transport/http/jsonrpc/encode_decode.go +++ b/transport/http/jsonrpc/encode_decode.go @@ -20,13 +20,13 @@ type EndpointCodec struct { // EndpointCodecMap maps the Request.Method to the proper EndpointCodec type EndpointCodecMap map[string]EndpointCodec -// DecodeRequestFunc extracts a user-domain request object from an raw JSON -// It's designed to be used in HTTP servers, for server-side endpoints. +// DecodeRequestFunc extracts a user-domain request object from raw JSON +// It's designed to be used in JSON RPC servers, for server-side endpoints. // One straightforward DecodeRequestFunc could be something that unmarshals // JSON from the request body to the concrete request type. type DecodeRequestFunc func(context.Context, json.RawMessage) (request interface{}, err error) -// EncodeResponseFunc encodes the passed response object to a JSON RPC response. +// EncodeResponseFunc encodes the passed response object to a JSON RPC result. // It's designed to be used in HTTP servers, for server-side endpoints. // One straightforward EncodeResponseFunc could be something that JSON encodes // the object directly. @@ -34,15 +34,15 @@ type EncodeResponseFunc func(context.Context, interface{}) (response json.RawMes // Client-Side Codec -// EncodeRequestFunc encodes the passed request object to raw JSON. +// EncodeRequestFunc encodes the given request object to raw JSON. // It's designed to be used in JSON RPC clients, for client-side // endpoints. One straightforward EncodeResponseFunc could be something that // JSON encodes the object directly. type EncodeRequestFunc func(context.Context, interface{}) (request json.RawMessage, err error) -// DecodeResponseFunc extracts a user-domain response object from an JSONRPC +// DecodeResponseFunc extracts a user-domain response object from an JSON RPC // response object. It's designed to be used in JSON RPC clients, for -// client-side endpoints. One straightforward DecodeRequestFunc could be -// something that JSON decodes from the request body to the concrete -// response type. +// client-side endpoints. It is the responsibility of this function to decide +// whether any error present in the JSON RPC response should be surfaced to the +// client endpoint. type DecodeResponseFunc func(context.Context, Response) (response interface{}, err error)