From 647a2e0c2d687683899f615a1c4999acf2620a04 Mon Sep 17 00:00:00 2001 From: tnasu Date: Wed, 8 Dec 2021 11:41:16 +0900 Subject: [PATCH] Fix codecov for `rpc/jsonrpc/server: return an error in WriteRPCResponseHTTP(Error) (bp #6204) (#6230)` --- blockchain/v2/reactor_test.go | 3 - crypto/merkle/proof_value.go | 4 +- rpc/jsonrpc/server/common_test.go | 114 +++++++++++++++++++ rpc/jsonrpc/server/http_json_handler_test.go | 24 ++++ rpc/jsonrpc/server/http_server_test.go | 96 ++++++++++++++++ rpc/jsonrpc/server/http_uri_handler_test.go | 68 +++++++++++ 6 files changed, 304 insertions(+), 5 deletions(-) create mode 100644 rpc/jsonrpc/server/common_test.go create mode 100644 rpc/jsonrpc/server/http_uri_handler_test.go diff --git a/blockchain/v2/reactor_test.go b/blockchain/v2/reactor_test.go index bfd549a2b..6dfacf49b 100644 --- a/blockchain/v2/reactor_test.go +++ b/blockchain/v2/reactor_test.go @@ -65,17 +65,14 @@ type mockBlockStore struct { blocks map[int64]*types.Block } -// nolint:unused // ignore func (ml *mockBlockStore) Height() int64 { return int64(len(ml.blocks)) } -// nolint:unused // ignore func (ml *mockBlockStore) LoadBlock(height int64) *types.Block { return ml.blocks[height] } -// nolint:unused // ignore func (ml *mockBlockStore) SaveBlock(block *types.Block, part *types.PartSet, commit *types.Commit) { ml.blocks[block.Height] = block } diff --git a/crypto/merkle/proof_value.go b/crypto/merkle/proof_value.go index 49f22ee94..428f29943 100644 --- a/crypto/merkle/proof_value.go +++ b/crypto/merkle/proof_value.go @@ -85,8 +85,8 @@ func (op ValueOp) Run(args [][]byte) ([][]byte, error) { bz := new(bytes.Buffer) // Wrap 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) { diff --git a/rpc/jsonrpc/server/common_test.go b/rpc/jsonrpc/server/common_test.go new file mode 100644 index 000000000..27520dbdc --- /dev/null +++ b/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 +} diff --git a/rpc/jsonrpc/server/http_json_handler_test.go b/rpc/jsonrpc/server/http_json_handler_test.go index 8928ee6a7..2d46a6c6f 100644 --- a/rpc/jsonrpc/server/http_json_handler_test.go +++ b/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))) +} diff --git a/rpc/jsonrpc/server/http_server_test.go b/rpc/jsonrpc/server/http_server_test.go index 890d0d299..f52d2c83c 100644 --- a/rpc/jsonrpc/server/http_server_test.go +++ b/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_MarshalIndent_error(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))) +} diff --git a/rpc/jsonrpc/server/http_uri_handler_test.go b/rpc/jsonrpc/server/http_uri_handler_test.go new file mode 100644 index 000000000..6111d91af --- /dev/null +++ b/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))) +}