Remove need for using a pointer to an Error in the Response

example_funcs_test.go

@@ -9,7 +9,7 @@ import (
 	"io/ioutil"
 	"net/http"
 
-	"github.com/AdamSLevy/jsonrpc2/v7"
+	"github.com/AdamSLevy/jsonrpc2/v8"
 )
 
 // Use the http and json packages to send a Request object.

example_test.go

@@ -11,7 +11,7 @@ import (
 	"io/ioutil"
 	"net/http"
 
-	jrpc "github.com/AdamSLevy/jsonrpc2/v7"
+	jrpc "github.com/AdamSLevy/jsonrpc2/v8"
 )
 
 var endpoint = "http://localhost:18888"

go.mod

@@ -1,4 +1,4 @@
-module github.com/AdamSLevy/jsonrpc2/v7
+module github.com/AdamSLevy/jsonrpc2/v8
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect

handler.go

@@ -22,7 +22,7 @@ func HTTPRequestHandler(methods MethodMap) http.HandlerFunc {
 	if err := methods.IsValid(); err != nil {
 		panic(err)
 	}
-	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+	return func(w http.ResponseWriter, req *http.Request) {
 		// Read all bytes of HTTP request body.
 		reqBytes, err := ioutil.ReadAll(req.Body)
 		if err != nil {
@@ -73,7 +73,7 @@ func HTTPRequestHandler(methods MethodMap) http.HandlerFunc {
 				respond(w, responses[0])
 			}
 		}
-	})
+	}
 }
 
 // processRequest unmarshals and processes a single Request stored in rawReq

methods.go

@@ -44,25 +44,25 @@ func (methods MethodMap) IsValid() error {
 
 // MethodFunc is the function signature used for RPC methods. The raw JSON of
 // the params of a valid Request is passed to the MethodFunc for further
-// application specific unmarshaling and validation. When called by the
-// handler, the params json.RawMessage, if not nil, is guaranteed to be valid
-// JSON representing a structured JSON type.
+// application specific unmarshaling and validation. When a MethodFunc is
+// invoked by the handler, the params json.RawMessage, if not nil, is
+// guaranteed to be valid JSON representing a structured JSON type.
 //
 // A valid MethodFunc must return a valid Response object. If MethodFunc
 // panics, or if the returned Response is not valid for whatever reason, then
-// an InternalError with no Data will be returned.
+// an InternalError with no Data will be returned by the http.HandlerFunc
+// instead.
 //
 // A valid Response must have either a Result or Error populated.
 //
-// If Error is populated, the Result will be discarded and the Error will be
-// validated. Valid errors will always retain their Data.
+// An Error is considered populated if the Error.Message is not empty. If Error
+// is populated, any Result will be ignored and the Error will be validated.
 //
 // A valid Error must be either InvalidParams or must use an ErrorCode outside
 // of the reserved range. If the ErrorCode is InvalidParamsCode, then the
-// correct InvalidParamsMessage will be set, so the MethodFunc does not need to
-// ensure that the Message is populated in this case. Otherwise the Message
-// must be populated and the ErrorCode must not be within the reserved
-// ErrorCode range.
+// correct InvalidParamsMessage will be set. In this case the MethodFunc only
+// needs to ensure that the Message is not empty. MethodFuncs are encouraged to
+// use NewInvalidParamsErrorResponse() for these errors.
 //
 // If you are getting InternalErrors from your method, set DebugMethodFunc to
 // true for additional debug output about the cause of the internal error.
@@ -72,18 +72,6 @@ type MethodFunc func(params json.RawMessage) Response
 // wraps the actual invocation of the method so that it can recover from panics
 // and validate and sanitize the returned Response. If the method panics or
 // returns an invalid Response, an InternalError response is returned.
-//
-// Valid error Responses are stripped of any Result left over by the method,
-// and any user provided Data is Marshaled and replaced with the resulting
-// json.RawMessage.
-//
-// For valid Responses, the user provided Result is Marshaled and replaced with
-// the resulting json.RawMessage.
-//
-// If you are getting InternalErrors from your method, set DebugMethodFunc to
-// true for additional debug output about the cause of the internal error.
-//
-// See MethodFunc for more information on writing conforming methods.
 func (method MethodFunc) call(params json.RawMessage) (res Response) {
 	defer func() {
 		if r := recover(); r != nil {
@@ -96,28 +84,34 @@ func (method MethodFunc) call(params json.RawMessage) (res Response) {
 		}
 	}()
 	res = method(params)
-	if !res.IsValid() {
-		panic("Invalid Response")
-	}
-	if res.Error != nil {
-		// Discard any result that may have been saved.
-		res.Result = nil
+	if res.IsError() {
+		// This as an Error Response.
+		// InvalidParamsCode is the only reserved ErrorCode MethodFuncs
+		// are allowed to use.
 		if res.Error.Code == InvalidParamsCode {
+			// Ensure the correct message is used.
 			res.Message = InvalidParamsMessage
-		} else if len(res.Error.Message) == 0 || res.Error.Code.IsReserved() {
+		} else if res.Error.Code.IsReserved() {
 			panic("Invalid Response.Error")
 		}
+		// Marshal the user provided Data to catch any potential errors
+		// here instead of later in the http.HandlerFunc.
 		data, err := json.Marshal(res.Data)
 		if err != nil {
 			panic("Cannot marshal Response.Error.Data")
 		}
 		res.Data = json.RawMessage(data)
-	} else {
+	} else if res.Result != nil {
+		// This must be a valid Response.
+		// Marshal the user provided Result to catch any potential
+		// errors here instead of later in the http.HandlerFunc.
 		data, err := json.Marshal(res.Result)
 		if err != nil {
 			panic("Cannot marshal Response.Result")
 		}
 		res.Result = json.RawMessage(data)
+	} else {
+		panic("Both Response.Result and Response.Error are empty")
 	}
 	return
 }

methods_test.go

@@ -38,19 +38,18 @@ func TestMethodFuncCall(t *testing.T) {
 	}, func(_ json.RawMessage) Response {
 		return Response{}
 	}, func(_ json.RawMessage) Response {
-		return Response{JSONRPC: "2.0",
-			Error: &Error{Message: "e", Data: map[bool]bool{true: true}}}
+		return Response{Error: Error{Message: "e", Data: map[bool]bool{true: true}}}
 	}, func(_ json.RawMessage) Response {
-		return Response{JSONRPC: "2.0", Result: map[bool]bool{true: true}}
+		return Response{Result: map[bool]bool{true: true}}
 	})
 	for _, f := range fs {
 		res := f.call(nil)
 		if assert.NotNil(res.Error) {
-			assert.Equal(InternalError, *res.Error)
+			assert.Equal(InternalError, res.Error)
 		}
 		assert.Nil(res.Result)
 	}
-	assert.Equal("Internal error: \"Invalid Response.Error\"\nParams: \nResponse: <-- {\"jsonrpc\":\"2.0\",\"error\":{\"code\":-32601,\"message\":\"method not found\",\"data\":\"test data\"},\"id\":null}\nInternal error: \"Invalid Response\"\nParams: \nResponse: <-- {\"jsonrpc\":\"\",\"id\":null}\nInternal error: \"Cannot marshal Response.Error.Data\"\nParams: \nResponse: <-- \nInternal error: \"Cannot marshal Response.Result\"\nParams: \nResponse: <-- \n",
+	assert.Equal("Internal error: \"Invalid Response.Error\"\nParams: \nResponse: <-- {\"jsonrpc\":\"2.0\",\"error\":{\"code\":-32601,\"message\":\"method not found\",\"data\":\"test data\"},\"id\":null}\nInternal error: \"Both Response.Result and Response.Error are empty\"\nParams: \nResponse: <-- \nInternal error: \"Cannot marshal Response.Error.Data\"\nParams: \nResponse: <-- \nInternal error: \"Cannot marshal Response.Result\"\nParams: \nResponse: <-- \n",
 		string(buf.Bytes()))
 
 	var f MethodFunc = func(_ json.RawMessage) Response {
@@ -62,7 +61,7 @@ func TestMethodFuncCall(t *testing.T) {
 			Code:    100,
 			Message: "custom",
 			Data:    json.RawMessage(`"data"`),
-		}, *res.Error)
+		}, res.Error)
 	}
 	assert.Nil(res.Result)
 
@@ -73,7 +72,7 @@ func TestMethodFuncCall(t *testing.T) {
 	if assert.NotNil(res.Error) {
 		e := InvalidParams
 		e.Data = json.RawMessage(`"data"`)
-		assert.Equal(e, *res.Error)
+		assert.Equal(e, res.Error)
 	}
 	assert.Nil(res.Result)
 }

request.go

@@ -18,19 +18,31 @@ type Request struct {
 	ID      interface{} `json:"id,omitempty"`
 }
 
+// MarshalJSON outputs a valid JSON RPC Request object. The Response.JSONRPC
+// field is always output as Version ("2.0").
+func (r Request) MarshalJSON() ([]byte, error) {
+	req := struct {
+		JSONRPC string      `json:"jsonrpc"`
+		Method  string      `json:"method"`
+		Params  interface{} `json:"params,omitempty"`
+		ID      interface{} `json:"id,omitempty"`
+	}{JSONRPC: Version, ID: r.ID, Method: r.Method, Params: r.Params}
+	return json.Marshal(req)
+}
+
 // NewRequest is a convenience function that returns a new Request with the
 // "jsonrpc" field already populated with the required value, "2.0". If nil id
 // is provided, it will be considered a Notification object and not receive a
 // response. Use NewNotification if you want a simpler function call to form a
 // JSON-RPC 2.0 Notification object.
 func NewRequest(method string, id, params interface{}) Request {
-	return Request{JSONRPC: "2.0", ID: id, Method: method, Params: params}
+	return Request{ID: id, Method: method, Params: params}
 }
 
 // IsValid returns true if r has a valid JSONRPC value of "2.0" and a non-empty
 // Method. Params and ID are not validated
 func (r Request) IsValid() bool {
-	return r.JSONRPC == "2.0" && len(r.Method) > 0
+	return r.JSONRPC == Version && len(r.Method) > 0
 }
 
 // String returns a JSON string with "--> " prefixed to represent a Request

response.go

@@ -4,16 +4,51 @@
 
 package jsonrpc2
 
-import "encoding/json"
+import (
+	"encoding/json"
+	"fmt"
+)
+
+// Version is the valid version string for the "jsonrpc" field required in all
+// JSON RPC 2.0 objects.
+const Version = "2.0"
 
 // Response represents a JSON-RPC 2.0 Response object.
+//
+// The json:",omitempty" are simply here for clarity. Although Error is a
+// concrete type and the json package will not ever detect it as being empty,
+// the json.Marhsaler interface is implemented to use the Error.Message length
+// to determine whether the Error should be considered empty.
 type Response struct {
 	JSONRPC string      `json:"jsonrpc"`
 	Result  interface{} `json:"result,omitempty"`
-	*Error  `json:"error,omitempty"`
+	Error   `json:"error,omitempty"`
 	ID      interface{} `json:"id"`
 }
 
+// MarshalJSON outputs a valid JSON RPC Response object. It returns an error if
+// Response.Result and Response.Error are both empty. The Error is considered
+// empty if the Error.Message is empty. If the Error is not empty, any Result
+// will be omitted. If the Error is empty, then the Error is omitted. The
+// Response.JSONRPC field is always output as Version ("2.0").
+func (r Response) MarshalJSON() ([]byte, error) {
+	res := struct {
+		JSONRPC string      `json:"jsonrpc"`
+		Result  interface{} `json:"result,omitempty"`
+		Error   interface{} `json:"error,omitempty"`
+		ID      interface{} `json:"id"`
+	}{JSONRPC: Version, ID: r.ID}
+	if len(r.Error.Message) != 0 {
+		res.Error = r.Error
+		return json.Marshal(res)
+	}
+	if r.Result == nil {
+		return nil, fmt.Errorf("Result and Error are both empty")
+	}
+	res.Result = r.Result
+	return json.Marshal(res)
+}
+
 // NewResponse is a convenience function that returns a new success Response
 // with JSONRPC already populated with the required value, "2.0".
 func NewResponse(result interface{}) Response {
@@ -35,17 +70,22 @@ func NewInvalidParamsErrorResponse(data interface{}) Response {
 }
 
 func newResponse(id, result interface{}) Response {
-	return Response{JSONRPC: "2.0", ID: id, Result: result}
+	return Response{ID: id, Result: result}
 }
 
 func newErrorResponse(id interface{}, err Error) Response {
-	return Response{JSONRPC: "2.0", ID: id, Error: &err}
+	return Response{ID: id, Error: err}
 }
 
-// IsValid returns true when r has a valid JSONRPC value of "2.0", ID is not
-// nil, and either Result or Error is not nil.
+// IsValid returns true if either Response.Result is not nil or
+// Response.Error.Message is not empty.
 func (r Response) IsValid() bool {
-	return r.JSONRPC == "2.0" && (r.Result != nil || r.Error != nil)
+	return r.JSONRPC == Version && (r.Result != nil || len(r.Error.Message) != 0)
+}
+
+// IsError returns true if r.Error.Message is not empty.
+func (r Response) IsError() bool {
+	return len(r.Error.Message) > 0
 }
 
 // String returns a string of the JSON with "<-- " prefixed to represent a