rpc: standardize error codes (#6019)

CHANGELOG_PENDING.md

@@ -13,6 +13,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
   - [config] \#5728 `fast_sync = "v1"` is no longer supported (@melekes)
   - [cli] \#5772 `gen_node_key` prints JSON-encoded `NodeKey` rather than ID and does not save it to `node_key.json` (@melekes)
   - [cli] \#5777 use hyphen-case instead of snake_case for all cli commands and config parameters (@cmwaters)
+  - [rpc] \#6019 standardise RPC errors and return the correct status code (@bipulprasad & @cmwaters)
 
 - Apps
   - [ABCI] \#5447 Remove `SetOption` method from `ABCI.Client` interface

light/provider/http/http.go

@@ -2,27 +2,24 @@ package http
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math/rand"
-	"regexp"
 	"strings"
 	"time"
 
 	"github.com/tendermint/tendermint/light/provider"
 	rpcclient "github.com/tendermint/tendermint/rpc/client"
 	rpchttp "github.com/tendermint/tendermint/rpc/client/http"
+	ctypes "github.com/tendermint/tendermint/rpc/core/types"
+	rpctypes "github.com/tendermint/tendermint/rpc/jsonrpc/types"
 	"github.com/tendermint/tendermint/types"
 )
 
-var (
-	// This is very brittle, see: https://github.com/tendermint/tendermint/issues/4740
-	regexpMissingHeight = regexp.MustCompile(`height \d+ (must be less than or equal to|is not available)`)
-
-	defaultOptions = Options{
-		MaxRetryAttempts: 10,
-		Timeout:          5 * time.Second,
-	}
-)
+var defaultOptions = Options{
+	MaxRetryAttempts: 10,
+	Timeout:          5 * time.Second,
+}
 
 // http provider uses an RPC client to obtain the necessary information.
 type http struct {
@@ -134,34 +131,48 @@ func (p *http) validatorSet(ctx context.Context, height *int64) (*types.Validato
 		// is negative we will keep repeating.
 		for attempt := 0; attempt != p.maxRetryAttempts+1; attempt++ {
 			res, err := p.client.Validators(ctx, height, &page, &perPage)
-			if err != nil {
-				// TODO: standardize errors on the RPC side
-				if regexpMissingHeight.MatchString(err.Error()) {
+			switch e := err.(type) {
+			case nil: // success!! Now we validate the response
+				if len(res.Validators) == 0 {
+					return nil, provider.ErrBadLightBlock{
+						Reason: fmt.Errorf("validator set is empty (height: %d, page: %d, per_page: %d)",
+							height, page, perPage),
+					}
+				}
+				if res.Total <= 0 {
+					return nil, provider.ErrBadLightBlock{
+						Reason: fmt.Errorf("total number of vals is <= 0: %d (height: %d, page: %d, per_page: %d)",
+							res.Total, height, page, perPage),
+					}
+				}
+
+			case *rpctypes.RPCError:
+				// Check if we got something other than internal error. This shouldn't happen unless the RPC module
+				// or light client has been tampered with. If we do get this error, stop the connection with the
+				// peer and return an error
+				if e.Code != -32603 {
+					return nil, provider.ErrBadLightBlock{Reason: errors.New(e.Data)}
+				}
+
+				// check if the error indicates that the peer doesn't have the block
+				if strings.Contains(e.Data, ctypes.ErrHeightNotAvailable.Error()) ||
+					strings.Contains(e.Data, ctypes.ErrHeightExceedsChainHead.Error()) {
 					return nil, provider.ErrLightBlockNotFound
 				}
-				// if we have exceeded retry attempts then return no response error
-				if attempt == p.maxRetryAttempts {
-					return nil, provider.ErrNoResponse
-				}
-				// else we wait and try again with exponential backoff
+
+				// we wait and try again with exponential backoff
+				// TODO: If we can, we should check if the error is purely because the node failed to respond in
+				// time. If this is the case then we continue, else we should stop straight away
 				time.Sleep(backoffTimeout(uint16(attempt)))
 				continue
+
+			default:
+				// something has happened to the RPC module if we are not receiving errors of type RPCError
+				panic(fmt.Errorf("unexpected error type: %w", err))
 			}
 
-			// Validate response.
-			if len(res.Validators) == 0 {
-				return nil, provider.ErrBadLightBlock{
-					Reason: fmt.Errorf("validator set is empty (height: %d, page: %d, per_page: %d)",
-						height, page, perPage),
-				}
-			}
-			if res.Total <= 0 {
-				return nil, provider.ErrBadLightBlock{
-					Reason: fmt.Errorf("total number of vals is <= 0: %d (height: %d, page: %d, per_page: %d)",
-						res.Total, height, page, perPage),
-				}
-			}
-
+			// update the total and increment the page index so we can fetch the
+			// next page of validators if need be
 			total = res.Total
 			vals = append(vals, res.Validators...)
 			page++
@@ -181,16 +192,34 @@ func (p *http) signedHeader(ctx context.Context, height *int64) (*types.SignedHe
 	// is negative we will keep repeating.
 	for attempt := 0; attempt != p.maxRetryAttempts+1; attempt++ {
 		commit, err := p.client.Commit(ctx, height)
-		if err != nil {
-			// TODO: standardize errors on the RPC side
-			if regexpMissingHeight.MatchString(err.Error()) {
+		switch e := err.(type) {
+		case nil: // success!!
+			return &commit.SignedHeader, nil
+
+		case *rpctypes.RPCError:
+			// Check if we got something other than internal error. This shouldn't happen unless the RPC module
+			// or light client has been tampered with. If we do get this error, stop the connection with the
+			// peer and return an error
+			if e.Code != -32603 {
+				return nil, provider.ErrBadLightBlock{Reason: errors.New(e.Data)}
+			}
+
+			// check if the error indicates that the peer doesn't have the block
+			if strings.Contains(err.Error(), ctypes.ErrHeightNotAvailable.Error()) ||
+				strings.Contains(err.Error(), ctypes.ErrHeightExceedsChainHead.Error()) {
 				return nil, provider.ErrLightBlockNotFound
 			}
+
 			// we wait and try again with exponential backoff
+			// TODO: If we can, we should check if the error is purely because the node failed to respond in
+			// time. If this is the case then we continue, else we should stop straight away
 			time.Sleep(backoffTimeout(uint16(attempt)))
 			continue
+
+		default:
+			// something has happened to the RPC module if we are not receiving errors of type RPCError
+			panic(fmt.Errorf("unexpected error type: %w", err))
 		}
-		return &commit.SignedHeader, nil
 	}
 	return nil, provider.ErrNoResponse
 }

light/provider/http/http_test.go

@@ -79,7 +79,7 @@ func TestProvider(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, lower, sh.Height)
 
-	// fetching missing heights (both future and pruned) should return appropriate errors
+	// // fetching missing heights (both future and pruned) should return appropriate errors
 	_, err = p.LightBlock(context.Background(), 1000)
 	require.Error(t, err)
 	assert.Equal(t, provider.ErrLightBlockNotFound, err)

light/rpc/client.go

@@ -20,8 +20,6 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
-var errNegOrZeroHeight = errors.New("negative or zero height")
-
 // KeyPathFunc builds a merkle path out of the given path and key.
 type KeyPathFunc func(path string, key []byte) (merkle.KeyPath, error)
 
@@ -127,7 +125,7 @@ func (c *Client) ABCIQueryWithOptions(ctx context.Context, path string, data tmb
 		return nil, errors.New("no proof ops")
 	}
 	if resp.Height <= 0 {
-		return nil, errNegOrZeroHeight
+		return nil, ctypes.ErrZeroOrNegativeHeight
 	}
 
 	// Update the light client if we're behind.
@@ -212,7 +210,7 @@ func (c *Client) ConsensusParams(ctx context.Context, height *int64) (*ctypes.Re
 		return nil, err
 	}
 	if res.BlockHeight <= 0 {
-		return nil, errNegOrZeroHeight
+		return nil, ctypes.ErrZeroOrNegativeHeight
 	}
 
 	// Update the light client if we're behind.
@@ -370,7 +368,7 @@ func (c *Client) BlockResults(ctx context.Context, height *int64) (*ctypes.Resul
 
 	// Validate res.
 	if res.Height <= 0 {
-		return nil, errNegOrZeroHeight
+		return nil, ctypes.ErrZeroOrNegativeHeight
 	}
 
 	// Update the light client if we're behind.
@@ -435,7 +433,7 @@ func (c *Client) Tx(ctx context.Context, hash []byte, prove bool) (*ctypes.Resul
 
 	// Validate res.
 	if res.Height <= 0 {
-		return nil, errNegOrZeroHeight
+		return nil, ctypes.ErrZeroOrNegativeHeight
 	}
 
 	// Update the light client if we're behind.
@@ -576,7 +574,7 @@ const (
 
 func validatePage(pagePtr *int, perPage, totalCount int) (int, error) {
 	if perPage < 1 {
-		panic(fmt.Sprintf("zero or negative perPage: %d", perPage))
+		panic(fmt.Errorf("%w (%d)", ctypes.ErrZeroOrNegativePerPage, perPage))
 	}
 
 	if pagePtr == nil { // no page parameter
@@ -589,7 +587,7 @@ func validatePage(pagePtr *int, perPage, totalCount int) (int, error) {
 	}
 	page := *pagePtr
 	if page <= 0 || page > pages {
-		return 1, fmt.Errorf("page should be within [1, %d] range, given %d", pages, page)
+		return 1, fmt.Errorf("%w expected range: [1, %d], given %d", ctypes.ErrPageOutOfRange, pages, page)
 	}
 
 	return page, nil

rpc/core/blocks.go

@@ -53,7 +53,7 @@ func BlockchainInfo(ctx *rpctypes.Context, minHeight, maxHeight int64) (*ctypes.
 func filterMinMax(base, height, min, max, limit int64) (int64, int64, error) {
 	// filter negatives
 	if min < 0 || max < 0 {
-		return min, max, fmt.Errorf("heights must be non-negative")
+		return min, max, ctypes.ErrZeroOrNegativeHeight
 	}
 
 	// adjust for default values
@@ -75,7 +75,8 @@ func filterMinMax(base, height, min, max, limit int64) (int64, int64, error) {
 	min = tmmath.MaxInt64(min, max-limit+1)
 
 	if min > max {
-		return min, max, fmt.Errorf("min height %d can't be greater than max height %d", min, max)
+		return min, max, fmt.Errorf("%w: min height %d can't be greater than max height %d",
+			ctypes.ErrInvalidRequest, min, max)
 	}
 	return min, max, nil
 }

rpc/core/env.go

@@ -11,6 +11,7 @@ import (
 	mempl "github.com/tendermint/tendermint/mempool"
 	"github.com/tendermint/tendermint/p2p"
 	"github.com/tendermint/tendermint/proxy"
+	ctypes "github.com/tendermint/tendermint/rpc/core/types"
 	sm "github.com/tendermint/tendermint/state"
 	"github.com/tendermint/tendermint/state/txindex"
 	"github.com/tendermint/tendermint/types"
@@ -94,8 +95,9 @@ type Environment struct {
 //----------------------------------------------
 
 func validatePage(pagePtr *int, perPage, totalCount int) (int, error) {
+	// this can only happen if we haven't first run validatePerPage
 	if perPage < 1 {
-		panic(fmt.Sprintf("zero or negative perPage: %d", perPage))
+		panic(fmt.Errorf("%w (%d)", ctypes.ErrZeroOrNegativePerPage, perPage))
 	}
 
 	if pagePtr == nil { // no page parameter
@@ -108,7 +110,7 @@ func validatePage(pagePtr *int, perPage, totalCount int) (int, error) {
 	}
 	page := *pagePtr
 	if page <= 0 || page > pages {
-		return 1, fmt.Errorf("page should be within [1, %d] range, given %d", pages, page)
+		return 1, fmt.Errorf("%w expected range: [1, %d], given %d", ctypes.ErrPageOutOfRange, pages, page)
 	}
 
 	return page, nil
@@ -142,16 +144,15 @@ func getHeight(latestHeight int64, heightPtr *int64) (int64, error) {
 	if heightPtr != nil {
 		height := *heightPtr
 		if height <= 0 {
-			return 0, fmt.Errorf("height must be greater than 0, but got %d", height)
+			return 0, fmt.Errorf("%w (requested height: %d)", ctypes.ErrZeroOrNegativeHeight, height)
 		}
 		if height > latestHeight {
-			return 0, fmt.Errorf("height %d must be less than or equal to the current blockchain height %d",
-				height, latestHeight)
+			return 0, fmt.Errorf("%w (requested height: %d, blockchain height: %d)",
+				ctypes.ErrHeightExceedsChainHead, height, latestHeight)
 		}
 		base := env.BlockStore.Base()
 		if height < base {
-			return 0, fmt.Errorf("height %v is not available, lowest height is %v",
-				height, base)
+			return 0, fmt.Errorf("%w (requested height: %d, base height: %d)", ctypes.ErrHeightExceedsChainHead, height, base)
 		}
 		return height, nil
 	}

rpc/core/evidence.go

@@ -1,7 +1,6 @@
 package core
 
 import (
-	"errors"
 	"fmt"
 
 	ctypes "github.com/tendermint/tendermint/rpc/core/types"
@@ -13,7 +12,7 @@ import (
 // More: https://docs.tendermint.com/master/rpc/#/Evidence/broadcast_evidence
 func BroadcastEvidence(ctx *rpctypes.Context, ev types.Evidence) (*ctypes.ResultBroadcastEvidence, error) {
 	if ev == nil {
-		return nil, errors.New("no evidence was provided")
+		return nil, fmt.Errorf("%w: no evidence was provided", ctypes.ErrInvalidRequest)
 	}
 
 	if err := ev.ValidateBasic(); err != nil {

rpc/core/net.go

@@ -1,7 +1,7 @@
 package core
 
 import (
-	"errors"
+	"fmt"
 	"strings"
 
 	"github.com/tendermint/tendermint/p2p"
@@ -36,7 +36,7 @@ func NetInfo(ctx *rpctypes.Context) (*ctypes.ResultNetInfo, error) {
 // UnsafeDialSeeds dials the given seeds (comma-separated id@IP:PORT).
 func UnsafeDialSeeds(ctx *rpctypes.Context, seeds []string) (*ctypes.ResultDialSeeds, error) {
 	if len(seeds) == 0 {
-		return &ctypes.ResultDialSeeds{}, errors.New("no seeds provided")
+		return &ctypes.ResultDialSeeds{}, fmt.Errorf("%w: no seeds provided", ctypes.ErrInvalidRequest)
 	}
 	env.Logger.Info("DialSeeds", "seeds", seeds)
 	if err := env.P2PPeers.DialPeersAsync(seeds); err != nil {
@@ -50,7 +50,7 @@ func UnsafeDialSeeds(ctx *rpctypes.Context, seeds []string) (*ctypes.ResultDialS
 func UnsafeDialPeers(ctx *rpctypes.Context, peers []string, persistent, unconditional, private bool) (
 	*ctypes.ResultDialPeers, error) {
 	if len(peers) == 0 {
-		return &ctypes.ResultDialPeers{}, errors.New("no peers provided")
+		return &ctypes.ResultDialPeers{}, fmt.Errorf("%w: no peers provided", ctypes.ErrInvalidRequest)
 	}
 
 	ids, err := getIDs(peers)

rpc/core/tx.go

@@ -88,7 +88,7 @@ func TxSearch(ctx *rpctypes.Context, query string, prove bool, pagePtr, perPageP
 			return results[i].Height < results[j].Height
 		})
 	default:
-		return nil, errors.New("expected order_by to be either `asc` or `desc` or empty")
+		return nil, fmt.Errorf("%w: expected order_by to be either `asc` or `desc` or empty", ctypes.ErrInvalidRequest)
 	}
 
 	// paginate results

rpc/core/types/responses.go

@@ -2,6 +2,7 @@ package coretypes
 
 import (
 	"encoding/json"
+	"errors"
 	"time"
 
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -12,6 +13,18 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
+// List of standardized errors used across RPC
+var (
+	ErrZeroOrNegativePerPage  = errors.New("zero or negative per_page")
+	ErrPageOutOfRange         = errors.New("page should be within range")
+	ErrZeroOrNegativeHeight   = errors.New("height must be greater than zero")
+	ErrHeightExceedsChainHead = errors.New("height must be less than or equal to the head of the node's blockchain")
+	ErrHeightNotAvailable     = errors.New("height is not available")
+	// ErrInvalidRequest is used as a wrapper to cover more specific cases where the user has
+	// made an invalid request
+	ErrInvalidRequest = errors.New("invalid request")
+)
+
 // List of blocks
 type ResultBlockchainInfo struct {
 	LastHeight int64              `json:"last_height"`

rpc/jsonrpc/client/http_json_client.go

@@ -173,7 +173,7 @@ func (c *Client) Call(
 	requestBuf := bytes.NewBuffer(requestBytes)
 	httpRequest, err := http.NewRequestWithContext(ctx, http.MethodPost, c.address, requestBuf)
 	if err != nil {
-		return nil, fmt.Errorf("request failed: %w", err)
+		return nil, fmt.Errorf("request setup failed: %w", err)
 	}
 
 	httpRequest.Header.Set("Content-Type", "application/json")
@@ -183,8 +183,11 @@ func (c *Client) Call(
 	}
 
 	httpResponse, err := c.client.Do(httpRequest)
+	if e, ok := err.(*url.Error); ok && e.Timeout() {
+		panic("Hello world")
+	}
 	if err != nil {
-		return nil, fmt.Errorf("post failed: %w", err)
+		return nil, fmt.Errorf("request failed: %w", err)
 	}
 
 	defer httpResponse.Body.Close()

rpc/jsonrpc/server/http_json_handler.go

@@ -3,6 +3,7 @@ package server
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -11,7 +12,8 @@ import (
 
 	tmjson "github.com/tendermint/tendermint/libs/json"
 	"github.com/tendermint/tendermint/libs/log"
-	types "github.com/tendermint/tendermint/rpc/jsonrpc/types"
+	ctypes "github.com/tendermint/tendermint/rpc/core/types"
+	"github.com/tendermint/tendermint/rpc/jsonrpc/types"
 )
 
 // HTTP + JSON handler
@@ -23,7 +25,6 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han
 		if err != nil {
 			WriteRPCResponseHTTPError(
 				w,
-				http.StatusBadRequest,
 				types.RPCInvalidRequestError(
 					nil,
 					fmt.Errorf("error reading request body: %w", err),
@@ -50,7 +51,6 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han
 			if err := json.Unmarshal(b, &request); err != nil {
 				WriteRPCResponseHTTPError(
 					w,
-					http.StatusInternalServerError,
 					types.RPCParseError(
 						fmt.Errorf("error unmarshaling request: %w", err),
 					),
@@ -100,11 +100,27 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han
 			returns := rpcFunc.f.Call(args)
 			logger.Info("HTTPJSONRPC", "method", request.Method, "args", args, "returns", returns)
 			result, err := unreflectResult(returns)
-			if err != nil {
-				responses = append(responses, types.RPCInternalError(request.ID, err))
-				continue
+			switch e := err.(type) {
+			// if no error then return a success response
+			case nil:
+				responses = append(responses, types.NewRPCSuccessResponse(request.ID, result))
+
+			// if this already of type RPC error then forward that error
+			case *types.RPCError:
+				responses = append(responses, types.NewRPCErrorResponse(request.ID, e.Code, e.Message, e.Data))
+
+			default: // we need to unwrap the error and parse it accordingly
+				switch errors.Unwrap(err) {
+				// check if the error was due to an invald request
+				case ctypes.ErrZeroOrNegativeHeight, ctypes.ErrZeroOrNegativePerPage,
+					ctypes.ErrPageOutOfRange, ctypes.ErrInvalidRequest:
+					responses = append(responses, types.RPCInvalidRequestError(request.ID, err))
+
+				// lastly default all remaining errors as internal errors
+				default: // includes ctypes.ErrHeightNotAvailable and ctypes.ErrHeightExceedsChainHead
+					responses = append(responses, types.RPCInternalError(request.ID, err))
+				}
 			}
-			responses = append(responses, types.NewRPCSuccessResponse(request.ID, result))
 		}
 		if len(responses) > 0 {
 			WriteRPCResponseHTTP(w, responses...)

rpc/jsonrpc/server/http_server.go

@@ -91,17 +91,42 @@ func ServeTLS(
 
 // WriteRPCResponseHTTPError marshals res as JSON and writes it to w.
 //
+// Maps JSON RPC error codes to HTTP Status codes as follows:
+//
+// HTTP Status	code	message
+// 500	-32700	Parse error.
+// 400	-32600	Invalid Request.
+// 404	-32601	Method not found.
+// 500	-32602	Invalid params.
+// 500	-32603	Internal error.
+// 500	-32099..-32000	Server error.
+//
+// source: https://www.jsonrpc.org/historical/json-rpc-over-http.html
+//
 // Panics if it can't Marshal res or write to w.
 func WriteRPCResponseHTTPError(
 	w http.ResponseWriter,
-	httpCode int,
 	res types.RPCResponse,
 ) {
+	if res.Error == nil {
+		panic("tried to write http error response without RPC error")
+	}
+
 	jsonBytes, err := json.MarshalIndent(res, "", "  ")
 	if err != nil {
 		panic(err)
 	}
 
+	var httpCode int
+	switch res.Error.Code {
+	case -32600:
+		httpCode = http.StatusBadRequest
+	case -32601:
+		httpCode = http.StatusNotFound
+	default:
+		httpCode = http.StatusInternalServerError
+	}
+
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(httpCode)
 	if _, err := w.Write(jsonBytes); err != nil {
@@ -186,7 +211,6 @@ func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler
 					)
 					WriteRPCResponseHTTPError(
 						rww,
-						http.StatusInternalServerError,
 						types.RPCInternalError(types.JSONRPCIntID(-1), err),
 					)
 				}

rpc/jsonrpc/server/http_server_test.go

@@ -159,9 +159,7 @@ func TestWriteRPCResponseHTTP(t *testing.T) {
 
 func TestWriteRPCResponseHTTPError(t *testing.T) {
 	w := httptest.NewRecorder()
-	WriteRPCResponseHTTPError(w,
-		http.StatusInternalServerError,
-		types.RPCInternalError(types.JSONRPCIntID(-1), errors.New("foo")))
+	WriteRPCResponseHTTPError(w, types.RPCInternalError(types.JSONRPCIntID(-1), errors.New("foo")))
 	resp := w.Result()
 	body, err := ioutil.ReadAll(resp.Body)
 	_ = resp.Body.Close()

rpc/jsonrpc/server/http_uri_handler.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"net/http"
 	"reflect"
@@ -10,6 +11,7 @@ import (
 
 	tmjson "github.com/tendermint/tendermint/libs/json"
 	"github.com/tendermint/tendermint/libs/log"
+	ctypes "github.com/tendermint/tendermint/rpc/core/types"
 	types "github.com/tendermint/tendermint/rpc/jsonrpc/types"
 )
 
@@ -25,8 +27,7 @@ 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))
+			WriteRPCResponseHTTPError(w, types.RPCMethodNotFoundError(dummyID))
 		}
 	}
 
@@ -41,7 +42,6 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit
 		if err != nil {
 			WriteRPCResponseHTTPError(
 				w,
-				http.StatusInternalServerError,
 				types.RPCInvalidParamsError(
 					dummyID,
 					fmt.Errorf("error converting http params to arguments: %w", err),
@@ -55,12 +55,27 @@ 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))
-			return
+		switch e := err.(type) {
+		// if no error then return a success response
+		case nil:
+			WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(dummyID, result))
+
+		// if this already of type RPC error then forward that error.
+		case *types.RPCError:
+			WriteRPCResponseHTTPError(w, types.NewRPCErrorResponse(dummyID, e.Code, e.Message, e.Data))
+
+		default: // we need to unwrap the error and parse it accordingly
+
+			switch errors.Unwrap(err) {
+			case ctypes.ErrZeroOrNegativeHeight, ctypes.ErrZeroOrNegativePerPage,
+				ctypes.ErrPageOutOfRange, ctypes.ErrInvalidRequest:
+				WriteRPCResponseHTTPError(w, types.RPCInvalidRequestError(dummyID, err))
+
+			default: // ctypes.ErrHeightNotAvailable, ctypes.ErrHeightExceedsChainHead:
+				WriteRPCResponseHTTPError(w, types.RPCInternalError(dummyID, err))
+			}
 		}
-		WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(dummyID, result))
+
 	}
 }
 

rpc/jsonrpc/server/rpc_func.go

@@ -1,7 +1,6 @@
 package server
 
 import (
-	"fmt"
 	"net/http"
 	"reflect"
 	"strings"
@@ -86,8 +85,8 @@ func funcReturnTypes(f interface{}) []reflect.Type {
 // NOTE: assume returns is result struct and error. If error is not nil, return it
 func unreflectResult(returns []reflect.Value) (interface{}, error) {
 	errV := returns[1]
-	if errV.Interface() != nil {
-		return nil, fmt.Errorf("%v", errV.Interface())
+	if err, ok := errV.Interface().(error); ok && err != nil {
+		return nil, err
 	}
 	rv := returns[0]
 	// the result is a registered interface,

rpc/jsonrpc/server/ws_handler.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/tendermint/tendermint/libs/log"
 	"github.com/tendermint/tendermint/libs/service"
+	ctypes "github.com/tendermint/tendermint/rpc/core/types"
 	types "github.com/tendermint/tendermint/rpc/jsonrpc/types"
 )
 
@@ -369,7 +370,7 @@ func (wsc *wsConnection) readRoutine() {
 				fnArgs, err := jsonParamsToArgs(rpcFunc, request.Params)
 				if err != nil {
 					if err := wsc.WriteRPCResponse(writeCtx,
-						types.RPCInternalError(request.ID, fmt.Errorf("error converting json params to arguments: %w", err)),
+						types.RPCInvalidParamsError(request.ID, fmt.Errorf("error converting json params to arguments: %w", err)),
 					); err != nil {
 						wsc.Logger.Error("Error writing RPC response", "err", err)
 					}
@@ -383,17 +384,34 @@ func (wsc *wsConnection) readRoutine() {
 			// TODO: Need to encode args/returns to string if we want to log them
 			wsc.Logger.Info("WSJSONRPC", "method", request.Method)
 
+			var resp types.RPCResponse
 			result, err := unreflectResult(returns)
-			if err != nil {
-				if err := wsc.WriteRPCResponse(writeCtx, types.RPCInternalError(request.ID, err)); err != nil {
-					wsc.Logger.Error("Error writing RPC response", "err", err)
+			switch e := err.(type) {
+			// if no error then return a success response
+			case nil:
+				resp = types.NewRPCSuccessResponse(request.ID, result)
+
+			// if this already of type RPC error then forward that error
+			case *types.RPCError:
+				resp = types.NewRPCErrorResponse(request.ID, e.Code, e.Message, e.Data)
+
+			default: // we need to unwrap the error and parse it accordingly
+				switch errors.Unwrap(err) {
+				// check if the error was due to an invald request
+				case ctypes.ErrZeroOrNegativeHeight, ctypes.ErrZeroOrNegativePerPage,
+					ctypes.ErrPageOutOfRange, ctypes.ErrInvalidRequest:
+					resp = types.RPCInvalidRequestError(request.ID, err)
+
+				// lastly default all remaining errors as internal errors
+				default: // includes ctypes.ErrHeightNotAvailable and ctypes.ErrHeightExceedsChainHead
+					resp = types.RPCInternalError(request.ID, err)
 				}
-				continue
 			}
 
-			if err := wsc.WriteRPCResponse(writeCtx, types.NewRPCSuccessResponse(request.ID, result)); err != nil {
+			if err := wsc.WriteRPCResponse(writeCtx, resp); err != nil {
 				wsc.Logger.Error("Error writing RPC response", "err", err)
 			}
+
 		}
 	}
 }

rpc/jsonrpc/types/types.go

@@ -214,7 +214,7 @@ func (resp RPCResponse) String() string {
 //	If there was an error in detecting the id in the Request object (e.g. Parse
 // 	error/Invalid Request), it MUST be Null.
 func RPCParseError(err error) RPCResponse {
-	return NewRPCErrorResponse(nil, -32700, "Parse error. Invalid JSON", err.Error())
+	return NewRPCErrorResponse(nil, -32700, "Parse error", err.Error())
 }
 
 // From the JSON-RPC 2.0 spec:

rpc/jsonrpc/types/types_test.go

@@ -40,7 +40,7 @@ func TestResponses(t *testing.T) {
 
 		d := RPCParseError(errors.New("hello world"))
 		e, _ := json.Marshal(d)
-		f := `{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error. Invalid JSON","data":"hello world"}}`
+		f := `{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error","data":"hello world"}}`
 		assert.Equal(f, string(e))
 
 		g := RPCMethodNotFoundError(jsonid)