rpc/jsonrpc/server: return an error in WriteRPCResponseHTTP(Error) (bp #6204) (#6230)

* rpc/jsonrpc/server: return an error in WriteRPCResponseHTTP(Error) (#6204)

instead of panicking
Closes #5529

(cherry picked from commit 00b9524168)

# Conflicts:
#	CHANGELOG_PENDING.md
#	rpc/jsonrpc/server/http_json_handler.go
#	rpc/jsonrpc/server/http_server.go
#	rpc/jsonrpc/server/http_server_test.go
#	rpc/jsonrpc/server/http_uri_handler.go

* resolve conflicts

* fix linting

* fix conflict

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
Co-authored-by: Marko Baricevic <marbar3778@yahoo.com>
This commit is contained in:
mergify[bot]
2021-03-17 14:55:05 +00:00
committed by GitHub
parent d004a584f8
commit 4e25703d58
12 changed files with 76 additions and 66 deletions

View File

@@ -21,14 +21,12 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han
return func(w http.ResponseWriter, r *http.Request) {
b, err := ioutil.ReadAll(r.Body)
if err != nil {
WriteRPCResponseHTTPError(
w,
http.StatusBadRequest,
types.RPCInvalidRequestError(
nil,
fmt.Errorf("error reading request body: %w", err),
),
res := types.RPCInvalidRequestError(nil,
fmt.Errorf("error reading request body: %w", err),
)
if wErr := WriteRPCResponseHTTPError(w, http.StatusBadRequest, res); wErr != nil {
logger.Error("failed to write response", "res", res, "err", wErr)
}
return
}
@@ -48,13 +46,10 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han
// next, try to unmarshal as a single request
var request types.RPCRequest
if err := json.Unmarshal(b, &request); err != nil {
WriteRPCResponseHTTPError(
w,
http.StatusInternalServerError,
types.RPCParseError(
fmt.Errorf("error unmarshalling request: %w", err),
),
)
res := types.RPCParseError(fmt.Errorf("error unmarshaling request: %w", err))
if wErr := WriteRPCResponseHTTPError(w, http.StatusInternalServerError, res); wErr != nil {
logger.Error("failed to write response", "res", res, "err", wErr)
}
return
}
requests = []types.RPCRequest{request}
@@ -108,7 +103,9 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han
}
if len(responses) > 0 {
WriteRPCResponseHTTP(w, responses...)
if wErr := WriteRPCResponseHTTP(w, responses...); wErr != nil {
logger.Error("failed to write responses", "res", responses, "err", wErr)
}
}
}
}

View File

@@ -89,30 +89,32 @@ func ServeTLS(
return err
}
// WriteRPCResponseHTTPError marshals res as JSON and writes it to w.
// WriteRPCResponseHTTPError marshals res as JSON (with indent) and writes it
// to w.
//
// Panics if it can't Marshal res or write to w.
// source: https://www.jsonrpc.org/historical/json-rpc-over-http.html
func WriteRPCResponseHTTPError(
w http.ResponseWriter,
httpCode int,
res types.RPCResponse,
) {
) error {
if res.Error == nil {
panic("tried to write http error response without RPC error")
}
jsonBytes, err := json.MarshalIndent(res, "", " ")
if err != nil {
panic(err)
return fmt.Errorf("json marshal: %w", err)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(httpCode)
if _, err := w.Write(jsonBytes); err != nil {
panic(err)
}
_, err = w.Write(jsonBytes)
return err
}
// WriteRPCResponseHTTP marshals res as JSON and writes it to w.
//
// Panics if it can't Marshal res or write to w.
func WriteRPCResponseHTTP(w http.ResponseWriter, res ...types.RPCResponse) {
// WriteRPCResponseHTTP marshals res as JSON (with indent) and writes it to w.
func WriteRPCResponseHTTP(w http.ResponseWriter, res ...types.RPCResponse) error {
var v interface{}
if len(res) == 1 {
v = res[0]
@@ -122,13 +124,12 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, res ...types.RPCResponse) {
jsonBytes, err := json.MarshalIndent(v, "", " ")
if err != nil {
panic(err)
return fmt.Errorf("json marshal: %w", err)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(200)
if _, err := w.Write(jsonBytes); err != nil {
panic(err)
}
_, err = w.Write(jsonBytes)
return err
}
//-----------------------------------------------------------------------------
@@ -166,7 +167,9 @@ func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler
// If RPCResponse
if res, ok := e.(types.RPCResponse); ok {
WriteRPCResponseHTTP(rww, res)
if wErr := WriteRPCResponseHTTP(rww, res); wErr != nil {
logger.Error("failed to write response", "res", res, "err", wErr)
}
} else {
// Panics can contain anything, attempt to normalize it as an error.
var err error
@@ -180,15 +183,12 @@ func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler
default:
}
logger.Error(
"Panic in RPC HTTP handler", "err", e, "stack",
string(debug.Stack()),
)
WriteRPCResponseHTTPError(
rww,
http.StatusInternalServerError,
types.RPCInternalError(types.JSONRPCIntID(-1), err),
)
logger.Error("panic in RPC HTTP handler", "err", e, "stack", string(debug.Stack()))
res := types.RPCInternalError(types.JSONRPCIntID(-1), err)
if wErr := WriteRPCResponseHTTPError(rww, http.StatusInternalServerError, res); wErr != nil {
logger.Error("failed to write response", "res", res, "err", wErr)
}
}
}

View File

@@ -112,7 +112,8 @@ func TestWriteRPCResponseHTTP(t *testing.T) {
// one argument
w := httptest.NewRecorder()
WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(id, &sampleResult{"hello"}))
err := WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(id, &sampleResult{"hello"}))
require.NoError(t, err)
resp := w.Result()
body, err := ioutil.ReadAll(resp.Body)
_ = resp.Body.Close()
@@ -129,9 +130,10 @@ func TestWriteRPCResponseHTTP(t *testing.T) {
// multiple arguments
w = httptest.NewRecorder()
WriteRPCResponseHTTP(w,
err = WriteRPCResponseHTTP(w,
types.NewRPCSuccessResponse(id, &sampleResult{"hello"}),
types.NewRPCSuccessResponse(id, &sampleResult{"world"}))
require.NoError(t, err)
resp = w.Result()
body, err = ioutil.ReadAll(resp.Body)
_ = resp.Body.Close()
@@ -159,9 +161,11 @@ func TestWriteRPCResponseHTTP(t *testing.T) {
func TestWriteRPCResponseHTTPError(t *testing.T) {
w := httptest.NewRecorder()
WriteRPCResponseHTTPError(w,
err := WriteRPCResponseHTTPError(
w,
http.StatusInternalServerError,
types.RPCInternalError(types.JSONRPCIntID(-1), errors.New("foo")))
require.NoError(t, err)
resp := w.Result()
body, err := ioutil.ReadAll(resp.Body)
_ = resp.Body.Close()

View File

@@ -25,8 +25,10 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit
// Exception for websocket endpoints
if rpcFunc.ws {
return func(w http.ResponseWriter, r *http.Request) {
WriteRPCResponseHTTPError(w, http.StatusNotFound,
types.RPCMethodNotFoundError(dummyID))
res := types.RPCMethodNotFoundError(dummyID)
if wErr := WriteRPCResponseHTTPError(w, http.StatusNotFound, res); wErr != nil {
logger.Error("failed to write response", "res", res, "err", wErr)
}
}
}
@@ -39,14 +41,12 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit
fnArgs, err := httpParamsToArgs(rpcFunc, r)
if err != nil {
WriteRPCResponseHTTPError(
w,
http.StatusInternalServerError,
types.RPCInvalidParamsError(
dummyID,
fmt.Errorf("error converting http params to arguments: %w", err),
),
res := types.RPCInvalidParamsError(dummyID,
fmt.Errorf("error converting http params to arguments: %w", err),
)
if wErr := WriteRPCResponseHTTPError(w, http.StatusInternalServerError, res); wErr != nil {
logger.Error("failed to write response", "res", res, "err", wErr)
}
return
}
args = append(args, fnArgs...)
@@ -56,11 +56,17 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit
logger.Debug("HTTPRestRPC", "method", r.URL.Path, "args", args, "returns", returns)
result, err := unreflectResult(returns)
if err != nil {
WriteRPCResponseHTTPError(w, http.StatusInternalServerError,
types.RPCInternalError(dummyID, err))
if err := WriteRPCResponseHTTPError(w, http.StatusInternalServerError,
types.RPCInternalError(dummyID, err)); err != nil {
logger.Error("failed to write response", "res", result, "err", err)
return
}
return
}
if err := WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(dummyID, result)); err != nil {
logger.Error("failed to write response", "res", result, "err", err)
return
}
WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(dummyID, result))
}
}