Skip to content

Commit ff4a2d4

Browse files
tzdybalgupadhyayaGanesha Upadhyaya
authored
fix WebSocket serialization (#1425)
Unit test was updated to assert valid encoding of data. Hacky approach to fix the issue is not working. Replacing normal JSON serialization with JSON RPC serialization is required, but there still is an issue. <!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> ## Overview Draft for visibility. CC: @Manav-Aggarwal Relates to: #1268 <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. --> ## Checklist <!-- Please complete the checklist to ensure that the PR is ready to be reviewed. IMPORTANT: PRs should be left in Draft until the below checklist is completed. --> - [ ] New and updated code has appropriate documentation - [ ] New and updated code has new and/or updated testing - [ ] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [ ] Linked issues closed with keywords <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced WebSocket connection handling in JSON-RPC services. - **Bug Fixes** - Improved subscription response encoding in JSON-RPC. - **Refactor** - Updated WebSocket-related code to improve RPC request management. - **Tests** - Altered test cases to reflect new JSON unmarshalling approach. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Ganesha Upadhyaya <ganeshrvce@gmail.com> Co-authored-by: Ganesha Upadhyaya <gupadhyaya@Ganeshas-MacBook-Pro-2.local>
1 parent 51dae0e commit ff4a2d4

4 files changed

Lines changed: 36 additions & 7 deletions

File tree

rpc/json/handler.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ func (h *handler) serveJSONRPC(w http.ResponseWriter, r *http.Request) {
5757
func (h *handler) serveJSONRPCforWS(w http.ResponseWriter, r *http.Request, wsConn *wsConn) {
5858
// Create a new codec request.
5959
codecReq := h.codec.NewRequest(r)
60+
if wsConn != nil {
61+
wsConn.codecReq = codecReq
62+
}
6063
// Get service method to be called.
6164
method, err := codecReq.Method()
6265
if err != nil {

rpc/json/service.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
package json
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/json"
67
"fmt"
78
"net/http"
89
"reflect"
910
"time"
1011

12+
cmjson "github.com/cometbft/cometbft/libs/json"
13+
1114
rpcclient "github.com/cometbft/cometbft/rpc/client"
1215
ctypes "github.com/cometbft/cometbft/rpc/core/types"
16+
"github.com/gorilla/rpc/v2"
1317
"github.com/gorilla/rpc/v2/json2"
1418

1519
"github.com/rollkit/rollkit/third_party/log"
@@ -101,12 +105,25 @@ func (s *service) Subscribe(req *http.Request, args *subscribeArgs, wsConn *wsCo
101105
}
102106

103107
go func() {
108+
var codecReq rpc.CodecRequest
109+
if wsConn != nil {
110+
codecReq = wsConn.codecReq
111+
} else {
112+
codecReq = json2.NewCodec().NewRequest(req)
113+
}
114+
104115
for msg := range sub {
105-
data, err := json.Marshal(msg.Data)
116+
var raw json.RawMessage
117+
raw, err = cmjson.Marshal(msg.Data)
118+
btz := new(bytes.Buffer)
119+
w := newResponseWriter(btz)
106120
if err != nil {
107-
s.logger.Error("failed to marshal response data", "error", err)
108-
continue
121+
codecReq.WriteError(w, http.StatusInternalServerError, err)
122+
return
109123
}
124+
codecReq.WriteResponse(w, raw)
125+
126+
data := btz.Bytes()
110127
if wsConn != nil {
111128
wsConn.queue <- data
112129
}

rpc/json/ws.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,18 @@ import (
55
"io"
66
"net/http"
77

8+
"github.com/gorilla/rpc/v2"
9+
810
"github.com/gorilla/websocket"
911

1012
"github.com/rollkit/rollkit/third_party/log"
1113
)
1214

1315
type wsConn struct {
14-
conn *websocket.Conn
15-
queue chan []byte
16-
logger log.Logger
16+
conn *websocket.Conn
17+
codecReq rpc.CodecRequest
18+
queue chan []byte
19+
logger log.Logger
1720
}
1821

1922
func (wsc *wsConn) sendLoop() {

rpc/json/ws_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import (
99
"testing"
1010
"time"
1111

12+
cmjson "github.com/cometbft/cometbft/libs/json"
13+
"github.com/go-kit/kit/transport/http/jsonrpc"
14+
1215
"github.com/cometbft/cometbft/libs/log"
1316
cmtypes "github.com/cometbft/cometbft/types"
1417
"github.com/gorilla/rpc/v2/json2"
@@ -63,8 +66,11 @@ func TestWebSockets(t *testing.T) {
6366
require.NoError(err)
6467
assert.Equal(websocket.TextMessage, typ)
6568
assert.NotEmpty(msg)
69+
var jsrpcResp jsonrpc.Response
70+
err = json.Unmarshal(msg, &jsrpcResp)
71+
require.NoError(err)
6672
var payload cmtypes.EventDataNewBlock
67-
err = json.Unmarshal(msg, &payload)
73+
err = cmjson.Unmarshal(jsrpcResp.Result, &payload)
6874
require.NoError(err)
6975
assert.NotNil(payload.ResultFinalizeBlock)
7076
assert.NotNil(payload.Block)

0 commit comments

Comments
 (0)