Skip to content

Commit 41cdd7a

Browse files
yarikbratashchukcoderabbitai[bot]MSeveyManav-Aggarwal
authored
feat: start mockda server only if it's not running already (#1882)
Closes [#1818](#1818) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced error handling for starting the mock DA server. - Added mock server capabilities for improved testing. - **Bug Fixes** - Improved robustness in handling scenarios where the mock DA server is already running, allowing the application to continue without crashing. - Expanded error handling scenarios in testing to cover various outcomes, including server start and invalid URL handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Matthew Sevey <[email protected]> Co-authored-by: Manav Aggarwal <[email protected]>
1 parent fc65faf commit 41cdd7a

File tree

2 files changed

+119
-7
lines changed

2 files changed

+119
-7
lines changed

cmd/rollkit/commands/run_node.go

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package commands
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"math/rand"
78
"net"
89
"net/url"
910
"os"
11+
"syscall"
1012
"time"
1113

1214
cmtcmd "github.com/cometbft/cometbft/cmd/cometbft/commands"
@@ -24,6 +26,7 @@ import (
2426
"github.com/mitchellh/mapstructure"
2527
"google.golang.org/grpc"
2628

29+
"github.com/rollkit/go-da"
2730
proxy "github.com/rollkit/go-da/proxy/jsonrpc"
2831
goDATest "github.com/rollkit/go-da/test"
2932
seqGRPC "github.com/rollkit/go-sequencing/proxy/grpc"
@@ -47,6 +50,8 @@ var (
4750

4851
// initialize the logger with the cometBFT defaults
4952
logger = cometlog.NewTMLogger(cometlog.NewSyncWriter(os.Stdout))
53+
54+
errDAServerAlreadyRunning = errors.New("DA server already running")
5055
)
5156

5257
// MockSequencerAddress is a sample address used by the mock sequencer
@@ -127,12 +132,16 @@ func NewRunNodeCmd() *cobra.Command {
127132

128133
// use mock jsonrpc da server by default
129134
if !cmd.Flags().Lookup("rollkit.da_address").Changed {
130-
srv, err := startMockDAServJSONRPC(cmd.Context())
131-
if err != nil {
135+
srv, err := startMockDAServJSONRPC(cmd.Context(), nodeConfig.DAAddress, proxy.NewServer)
136+
if err != nil && !errors.Is(err, errDAServerAlreadyRunning) {
132137
return fmt.Errorf("failed to launch mock da server: %w", err)
133138
}
134139
// nolint:errcheck,gosec
135-
defer func() { srv.Stop(cmd.Context()) }()
140+
defer func() {
141+
if srv != nil {
142+
srv.Stop(cmd.Context())
143+
}
144+
}()
136145
}
137146

138147
// use mock grpc sequencer server by default
@@ -236,13 +245,27 @@ func addNodeFlags(cmd *cobra.Command) {
236245
}
237246

238247
// startMockDAServJSONRPC starts a mock JSONRPC server
239-
func startMockDAServJSONRPC(ctx context.Context) (*proxy.Server, error) {
240-
addr, _ := url.Parse(nodeConfig.DAAddress)
241-
srv := proxy.NewServer(addr.Hostname(), addr.Port(), goDATest.NewDummyDA())
242-
err := srv.Start(ctx)
248+
func startMockDAServJSONRPC(
249+
ctx context.Context,
250+
daAddress string,
251+
newServer func(string, string, da.DA) *proxy.Server,
252+
) (*proxy.Server, error) {
253+
addr, err := url.Parse(daAddress)
243254
if err != nil {
244255
return nil, err
245256
}
257+
258+
srv := newServer(addr.Hostname(), addr.Port(), goDATest.NewDummyDA())
259+
if err := srv.Start(ctx); err != nil {
260+
if errors.Is(err, syscall.EADDRINUSE) {
261+
logger.Info("DA server is already running", "address", nodeConfig.DAAddress)
262+
return nil, errDAServerAlreadyRunning
263+
}
264+
return nil, err
265+
}
266+
267+
logger.Info("Starting mock DA server", "address", nodeConfig.DAAddress)
268+
246269
return srv, nil
247270
}
248271

cmd/rollkit/commands/run_node_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11
package commands
22

33
import (
4+
"context"
5+
"errors"
6+
"net/url"
47
"reflect"
8+
"syscall"
59
"testing"
610
"time"
11+
12+
"github.com/stretchr/testify/assert"
13+
14+
"github.com/rollkit/go-da"
15+
proxy "github.com/rollkit/go-da/proxy/jsonrpc"
716
)
817

918
func TestParseFlags(t *testing.T) {
@@ -133,3 +142,83 @@ func TestAggregatorFlagInvariants(t *testing.T) {
133142
}
134143
}
135144
}
145+
146+
// MockServer wraps proxy.Server to allow us to control its behavior in tests
147+
type MockServer struct {
148+
*proxy.Server
149+
StartFunc func(context.Context) error
150+
StopFunc func(context.Context) error
151+
}
152+
153+
func (m *MockServer) Start(ctx context.Context) error {
154+
if m.StartFunc != nil {
155+
return m.StartFunc(ctx)
156+
}
157+
return m.Server.Start(ctx)
158+
}
159+
160+
func (m *MockServer) Stop(ctx context.Context) error {
161+
if m.StopFunc != nil {
162+
return m.StopFunc(ctx)
163+
}
164+
return m.Server.Stop(ctx)
165+
}
166+
167+
func TestStartMockDAServJSONRPC(t *testing.T) {
168+
tests := []struct {
169+
name string
170+
daAddress string
171+
mockServerErr error
172+
expectedErr error
173+
}{
174+
{
175+
name: "Success",
176+
daAddress: "http://localhost:26657",
177+
mockServerErr: nil,
178+
expectedErr: nil,
179+
},
180+
{
181+
name: "Invalid URL",
182+
daAddress: "://invalid",
183+
mockServerErr: nil,
184+
expectedErr: &url.Error{},
185+
},
186+
{
187+
name: "Server Already Running",
188+
daAddress: "http://localhost:26657",
189+
mockServerErr: syscall.EADDRINUSE,
190+
expectedErr: errDAServerAlreadyRunning,
191+
},
192+
{
193+
name: "Other Server Error",
194+
daAddress: "http://localhost:26657",
195+
mockServerErr: errors.New("other error"),
196+
expectedErr: errors.New("other error"),
197+
},
198+
}
199+
200+
for _, tt := range tests {
201+
t.Run(tt.name, func(t *testing.T) {
202+
newServerFunc := func(hostname, port string, da da.DA) *proxy.Server {
203+
mockServer := &MockServer{
204+
Server: proxy.NewServer(hostname, port, da),
205+
StartFunc: func(ctx context.Context) error {
206+
return tt.mockServerErr
207+
},
208+
}
209+
return mockServer.Server
210+
}
211+
212+
srv, err := startMockDAServJSONRPC(context.Background(), tt.daAddress, newServerFunc)
213+
214+
if tt.expectedErr != nil {
215+
assert.Error(t, err)
216+
assert.IsType(t, tt.expectedErr, err)
217+
assert.Nil(t, srv)
218+
} else {
219+
assert.NoError(t, err)
220+
assert.NotNil(t, srv)
221+
}
222+
})
223+
}
224+
}

0 commit comments

Comments
 (0)