Fix codecov for `rpc/jsonrpc/server: return an error in WriteRPCRespo…

…nseHTTP(Error) (bp #6204) (#6230)`

crypto/merkle/proof_value.go

@@ -85,8 +85,8 @@ func (op ValueOp) Run(args [][]byte) ([][]byte, error) {
 
 	bz := new(bytes.Buffer)
 	// Wrap <op.Key, vhash> to hash the KVPair.
-	encodeByteSlice(bz, op.key) // nolint: errcheck // does not error
-	encodeByteSlice(bz, vhash)  // nolint: errcheck // does not error
+	encodeByteSlice(bz, op.key) //nolint: errcheck // does not error
+	encodeByteSlice(bz, vhash)  //nolint: errcheck // does not error
 	kvhash := leafHash(bz.Bytes())
 
 	if !bytes.Equal(kvhash, op.Proof.LeafHash) {

rpc/jsonrpc/server/common_test.go

@@ -0,0 +1,114 @@
+package server
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"strconv"
+
+	"github.com/line/ostracon/libs/log"
+	"github.com/line/ostracon/rpc/jsonrpc/types"
+)
+
+var (
+	TestJSONIntID = types.JSONRPCIntID(-1)
+	TestRPCError  = &types.RPCError{}
+	TestRawMSG    = json.RawMessage(`{"p1":"v1"}`)
+	TestText      = "foo"
+	ErrFoo        = errors.New(TestText)
+
+	TestRPCFunc = NewRPCFunc(
+		func(ctx *types.Context, s string, i int) (string, error) { return TestText, nil }, "s,i")
+	TestRPCErrorFunc = NewRPCFunc(
+		func(ctx *types.Context, s string, i int) (string, error) { return "", ErrFoo }, "s,i")
+	TestWSRPCFunc = NewWSRPCFunc(
+		func(ctx *types.Context, s string, i int) (string, error) { return TestText, nil }, "s,i")
+
+	TestFuncMap   = map[string]*RPCFunc{"c": TestRPCFunc}
+	TestGoodBody  = `{"jsonrpc": "2.0", "method": "c", "id": "0", "params": null}`
+	TestBadParams = `{"jsonrpc": "2.0", "method": "c", "id": "0", "params": "s=a,i=b"}`
+)
+
+type FailManager struct {
+	counter       int
+	failedCounter int
+	throwPanic    bool
+}
+
+func (fm *FailManager) checkAndDo(
+	encounter func() (int, error),
+	throwing func(),
+) (int, error) {
+	if fm.counter == fm.failedCounter {
+		fmt.Println("FailManager:do encounter")
+		return encounter()
+	}
+	fm.counter++
+	if fm.throwPanic {
+		fmt.Println("FailManager:do throwing")
+		throwing()
+	}
+	return 0, nil
+}
+
+type FailedWriteResponseWriter struct {
+	header http.Header
+	fm     *FailManager
+	code   int
+	error  error
+}
+
+func NewFailedWriteResponseWriter() FailedWriteResponseWriter {
+	return FailedWriteResponseWriter{
+		header: make(http.Header),
+		fm:     &FailManager{},
+		code:   -1,
+		error:  fmt.Errorf("error"),
+	}
+}
+func (frw FailedWriteResponseWriter) Header() http.Header {
+	return frw.header
+}
+func (frw FailedWriteResponseWriter) Write(buf []byte) (int, error) {
+	fmt.Println("FailedWriteResponseWriter:" + strconv.Itoa(frw.fm.counter) + ":" + string(buf))
+	return frw.fm.checkAndDo(
+		func() (int, error) {
+			return frw.code, frw.error
+		},
+		func() {
+			res := types.RPCResponse{}
+			res.UnmarshalJSON(buf) // nolint: errcheck
+			panic(res)
+		},
+	)
+}
+func (frw FailedWriteResponseWriter) WriteHeader(code int) {
+	frw.header.Set(http.StatusText(code), strconv.Itoa(code))
+}
+
+type FailedLogger struct {
+	fm *FailManager
+}
+
+func NewFailedLogger() FailedLogger {
+	return FailedLogger{
+		fm: &FailManager{},
+	}
+}
+func (l *FailedLogger) Info(msg string, keyvals ...interface{}) {
+	fmt.Println("FailedLogger.Info:" + msg)
+}
+func (l *FailedLogger) Debug(msg string, keyvals ...interface{}) {
+	fmt.Println("FailedLogger.Debug:" + msg)
+}
+func (l *FailedLogger) Error(msg string, keyvals ...interface{}) {
+	fmt.Println("FailedLogger.Error:" + strconv.Itoa(l.fm.counter) + ":" + msg)
+	l.fm.checkAndDo( // nolint: errcheck
+		func() (int, error) { panic(l.fm.counter) },
+		func() {},
+	)
+}
+func (l *FailedLogger) With(keyvals ...interface{}) log.Logger {
+	return l
+}

rpc/jsonrpc/server/http_json_handler_test.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -227,3 +228,26 @@ func TestUnknownRPCPath(t *testing.T) {
 	require.Equal(t, http.StatusNotFound, res.StatusCode, "should always return 404")
 	res.Body.Close()
 }
+
+func TestMakeJSONRPCHandler_Unmarshal_WriteRPCResponseHTTPError_error(t *testing.T) {
+	handlerFunc := makeJSONRPCHandler(nil, log.TestingLogger())
+	// json.Unmarshal error
+	req, _ := http.NewRequest("GET", "http://localhost/", strings.NewReader("hoge"))
+	// WriteRPCResponseHTTPError error
+	rec := NewFailedWriteResponseWriter()
+	handlerFunc.ServeHTTP(rec, req)
+	assert.Equal(t,
+		strconv.Itoa(http.StatusInternalServerError),
+		rec.Header().Get(http.StatusText(http.StatusInternalServerError)))
+}
+
+func TestMakeJSONRPCHandler_last_WriteRPCResponseHTTP_error(t *testing.T) {
+	handlerFunc := makeJSONRPCHandler(TestFuncMap, log.TestingLogger())
+	req, _ := http.NewRequest("GET", "http://localhost/", strings.NewReader(TestGoodBody))
+	// WriteRPCResponseHTTP error
+	rec := NewFailedWriteResponseWriter()
+	handlerFunc.ServeHTTP(rec, req)
+	assert.Equal(t,
+		strconv.Itoa(http.StatusOK),
+		rec.Header().Get(http.StatusText(http.StatusOK)))
+}

rpc/jsonrpc/server/http_server_test.go

@@ -8,6 +8,8 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -182,3 +184,97 @@ func TestWriteRPCResponseHTTPError(t *testing.T) {
   }
 }`, string(body))
 }
+func TestWriteRPCResponseHTTP_Error_MarshalIndent(t *testing.T) {
+	w := NewFailedWriteResponseWriter()
+	result, _ := TestRawMSG.MarshalJSON()
+	// json.MarshalIndent error
+	err := WriteRPCResponseHTTP(w,
+		types.RPCResponse{Result: result[1:]}) // slice json for error
+	require.Error(t, err)
+	assert.NotEqual(t, w.error, err)
+}
+
+func TestWriteRPCResponseHTTP_Write_error(t *testing.T) {
+	w := NewFailedWriteResponseWriter()
+	result, _ := TestRawMSG.MarshalJSON()
+	// w.Write error
+	err := WriteRPCResponseHTTP(w,
+		types.NewRPCSuccessResponse(TestJSONIntID, result))
+	require.Error(t, err)
+	assert.Equal(t, w.error, err)
+}
+
+func TestWriteRPCResponseHTTPError_MarshallIndent_error(t *testing.T) {
+	w := NewFailedWriteResponseWriter()
+	// json.MarshalIndent error
+	result, _ := TestRawMSG.MarshalJSON()
+	err := WriteRPCResponseHTTPError(w, http.StatusInternalServerError,
+		types.RPCResponse{Result: result[1:], Error: TestRPCError}) // slice json for error
+	require.Error(t, err)
+	assert.NotEqual(t, w.error, err)
+}
+
+func TestWriteRPCResponseHTTPError_Write_error(t *testing.T) {
+	w := NewFailedWriteResponseWriter()
+	// w.Write error
+	err := WriteRPCResponseHTTPError(w, http.StatusInternalServerError,
+		types.RPCInternalError(TestJSONIntID, ErrFoo))
+	require.Error(t, err)
+	assert.Equal(t, w.error, err)
+}
+
+func TestWriteRPCResponseHTTPError_rerErrorIsNil_panic(t *testing.T) {
+	w := NewFailedWriteResponseWriter()
+	// panic: res.Error == nil
+	defer func() {
+		e := recover()
+		assert.True(t, e != nil)
+	}()
+	result, _ := TestRawMSG.MarshalJSON()
+	WriteRPCResponseHTTPError(w, http.StatusInternalServerError, types.RPCResponse{Result: result}) // nolint: errcheck
+}
+
+func TestRecoverAndLogHandler_RPCResponseOK_WriteRPCResponseHTTPError_error(t *testing.T) {
+	// RPCResponse == ok and WriteRPCResponseHTTPError is error
+	handlerFunc := makeJSONRPCHandler(TestFuncMap, log.TestingLogger())
+	handler := RecoverAndLogHandler(handlerFunc, log.TestingLogger())
+	assert.NotNil(t, handler)
+	req, _ := http.NewRequest("GET", "http://localhost/", strings.NewReader(TestGoodBody))
+	// throwing and encounter
+	rec := NewFailedWriteResponseWriter()
+	rec.fm.throwPanic = true
+	rec.fm.failedCounter = 1
+	handler.ServeHTTP(rec, req)
+	assert.Equal(t,
+		strconv.Itoa(http.StatusOK),
+		rec.Header().Get(http.StatusText(http.StatusOK)))
+}
+
+func TestRecoverAndLogHandler_RPCResponseNG_WriteRPCResponseHTTPError_error(t *testing.T) {
+	// RPCResponse != ok and WriteRPCResponseHTTPError is error
+	handler := RecoverAndLogHandler(nil, log.TestingLogger())
+	assert.NotNil(t, handler)
+	req, _ := http.NewRequest("GET", "http://localhost/", strings.NewReader(TestGoodBody))
+	// encounter
+	rec := NewFailedWriteResponseWriter()
+	handler.ServeHTTP(rec, req)
+	assert.Equal(t,
+		strconv.Itoa(http.StatusInternalServerError),
+		rec.Header().Get(http.StatusText(http.StatusInternalServerError)))
+}
+
+func TestRecoverAndLogHandler_RPCResponseNG_WriteRPCResponseHTTPError_error_panic(t *testing.T) {
+	// RPCResponse != ok and WriteRPCResponseHTTPError is error and logger.Error is panic on 2nd times
+	// encounter
+	logger := NewFailedLogger()
+	logger.fm.failedCounter = 1
+	handler := RecoverAndLogHandler(nil, &logger)
+	assert.NotNil(t, handler)
+	req, _ := http.NewRequest("GET", "http://localhost/", strings.NewReader(TestGoodBody))
+	// encounter
+	rec := NewFailedWriteResponseWriter()
+	handler.ServeHTTP(rec, req)
+	assert.Equal(t,
+		strconv.Itoa(http.StatusInternalServerError),
+		rec.Header().Get(http.StatusText(http.StatusInternalServerError)))
+}

rpc/jsonrpc/server/http_uri_handler_test.go

@@ -0,0 +1,68 @@
+package server
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/line/ostracon/libs/log"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMakeHTTPHandler(t *testing.T) {
+	handlerFunc := makeHTTPHandler(TestRPCFunc, log.TestingLogger())
+	req, _ := http.NewRequest("GET", "http://localhost/", strings.NewReader(TestGoodBody))
+	rec := httptest.NewRecorder()
+	handlerFunc(rec, req)
+	res := rec.Result()
+	require.Equal(t, http.StatusOK, res.StatusCode)
+	res.Body.Close()
+}
+
+func TestMakeHTTPHandler_WS_WriteRPCResponseHTTPError_error(t *testing.T) {
+	handlerFunc := makeHTTPHandler(TestWSRPCFunc, log.TestingLogger())
+	req, _ := http.NewRequest("GET", "http://localhost/", nil)
+	rec := NewFailedWriteResponseWriter()
+	handlerFunc(rec, req)
+	assert.Equal(t,
+		strconv.Itoa(http.StatusNotFound),
+		rec.Header().Get(http.StatusText(http.StatusNotFound)))
+}
+
+func TestMakeHTTPHandler_httpParamsToArgs_WriteRPCResponseHTTPError_error(t *testing.T) {
+	handlerFunc := makeHTTPHandler(TestRPCFunc, log.TestingLogger())
+	// httpParamsToArgs error
+	req, _ := http.NewRequest("GET", "http://localhost/c?s=1", nil)
+	// WriteRPCResponseHTTPError error
+	rec := NewFailedWriteResponseWriter()
+	handlerFunc(rec, req)
+	assert.Equal(t,
+		strconv.Itoa(http.StatusInternalServerError),
+		rec.Header().Get(http.StatusText(http.StatusInternalServerError)))
+}
+
+func TestMakeHTTPHandler_unreflectResult_WriteRPCResponseHTTPError_error(t *testing.T) {
+	// unreflectResult error
+	handlerFunc := makeHTTPHandler(TestRPCErrorFunc, log.TestingLogger())
+	req, _ := http.NewRequest("GET", "http://localhost/", nil)
+	// WriteRPCResponseHTTPError error
+	rec := NewFailedWriteResponseWriter()
+	handlerFunc(rec, req)
+	assert.Equal(t,
+		strconv.Itoa(http.StatusInternalServerError),
+		rec.Header().Get(http.StatusText(http.StatusInternalServerError)))
+}
+
+func TestMakeHTTPHandler_last_WriteRPCResponseHTTP_error(t *testing.T) {
+	handlerFunc := makeHTTPHandler(TestRPCFunc, log.TestingLogger())
+	req, _ := http.NewRequest("GET", "http://localhost/", strings.NewReader(TestGoodBody))
+	// WriteRPCResponseHTTP error
+	rec := NewFailedWriteResponseWriter()
+	handlerFunc(rec, req)
+	assert.Equal(t,
+		strconv.Itoa(http.StatusOK),
+		rec.Header().Get(http.StatusText(http.StatusOK)))
+}

rpc/jsonrpc/types/types_test.go

@@ -85,39 +85,39 @@ func TestRPCError(t *testing.T) {
 
 func TestRPCRequestUnmarshalJSON(t *testing.T) {
 	tcs := []struct {
-		assert bool
+		assert   bool
 		expected RPCRequest
- 	} {
+	}{
 		{
-			assert: true,
+			assert:   true,
 			expected: RPCRequest{ID: JSONRPCIntID(1), Method: "id int", Params: json.RawMessage(`{"p1":"v1"}`)},
 		}, {
-			assert: true,
+			assert:   true,
 			expected: RPCRequest{ID: JSONRPCStringID("s"), Method: "id string", Params: json.RawMessage(`{"p1":"v1"}`)},
-	 	}, {
-			assert: true,
+		}, {
+			assert:   true,
 			expected: RPCRequest{ID: JSONRPCStringID(""), Method: "id empty", Params: json.RawMessage(`{"p1":"v1"}`)},
 		}, {
 			// can't treat nil as jsonrpcid
-			assert: false,
+			assert:   false,
 			expected: RPCRequest{ID: nil, Method: "id nil", Params: json.RawMessage(`{"p1":"v1"}`)},
 		}, {
-			assert: true,
+			assert:   true,
 			expected: RPCRequest{ID: JSONRPCIntID(1), Method: "params null", Params: json.RawMessage(`null`)},
 		}, {
 			// can't treat nil as json.RawMessage: the value of `nil` become "null" string
-			assert: false,
+			assert:   false,
 			expected: RPCRequest{ID: JSONRPCIntID(1), Method: "params nil", Params: nil},
 		},
 	}
 	for _, tc := range tcs {
 		data, _ := json.Marshal(tc.expected)
 		actual := RPCRequest{}
-		json.Unmarshal(data, &actual)
+		json.Unmarshal(data, &actual) // nolint: errcheck
 		assert.Equal(t, reflect.DeepEqual(tc.expected, actual), tc.assert,
 			"expected:", tc.expected, "actual:", actual)
 		actual2 := RPCRequest{}
-		actual2.UnmarshalJSON(data)
+		actual2.UnmarshalJSON(data) // nolint: errcheck
 		assert.Equal(t, reflect.DeepEqual(tc.expected, actual2), tc.assert,
 			"expected:", tc.expected, "actual2:", actual2)
 	}