From 30c6eab22d462e0df4dde0f063302db98df50783 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 7 Mar 2024 17:35:10 -0500 Subject: [PATCH 1/2] add missing logs at startup --- http/server.go | 50 +++++++++++++++++++++++++++++---------------- http/server_test.go | 45 +++++++++++++++++++++++++++++++++------- net/peer.go | 1 + node/node.go | 9 +++++++- 4 files changed, 79 insertions(+), 26 deletions(-) diff --git a/http/server.go b/http/server.go index 80d46c6679..f975e200ad 100644 --- a/http/server.go +++ b/http/server.go @@ -110,8 +110,10 @@ func WithTLSKeyPath(path string) ServerOpt { // Server struct holds the Handler for the HTTP API. type Server struct { - options *ServerOptions - server *http.Server + options *ServerOptions + server *http.Server + listener net.Listener + isTLS bool } // NewServer instantiates a new server with the given http.Handler. @@ -148,25 +150,34 @@ func (s *Server) Shutdown(ctx context.Context) error { return s.server.Shutdown(ctx) } -// ListenAndServe listens for and serves incoming connections. -func (s *Server) ListenAndServe() error { +// SetListener sets a new listener on the Server. +func (s *Server) SetListener() (err error) { + s.listener, err = net.Listen("tcp", s.options.Address) + return err +} + +// Serve serves incoming connections. +func (s *Server) Serve() error { if s.options.TLSCertPath == "" && s.options.TLSKeyPath == "" { - return s.listenAndServe() + return s.serve() } - return s.listenAndServeTLS() + s.isTLS = true + return s.serveTLS() } -// listenAndServe listens for and serves http connections. -func (s *Server) listenAndServe() error { - listener, err := net.Listen("tcp", s.options.Address) - if err != nil { - return err +// serve serves http connections. +func (s *Server) serve() error { + if s.listener == nil { + return ErrNoListener } - return s.server.Serve(listener) + return s.server.Serve(s.listener) } -// listenAndServeTLS listens for and serves https connections. -func (s *Server) listenAndServeTLS() error { +// serveTLS serves https connections. +func (s *Server) serveTLS() error { + if s.listener == nil { + return ErrNoListener + } cert, err := tls.LoadX509KeyPair(s.options.TLSCertPath, s.options.TLSKeyPath) if err != nil { return err @@ -177,9 +188,12 @@ func (s *Server) listenAndServeTLS() error { CipherSuites: tlsCipherSuites, Certificates: []tls.Certificate{cert}, } - listener, err := net.Listen("tcp", s.options.Address) - if err != nil { - return err + return s.server.Serve(tls.NewListener(s.listener, config)) +} + +func (s *Server) Address() string { + if s.isTLS { + return "https://" + s.listener.Addr().String() } - return s.server.Serve(tls.NewListener(listener, config)) + return "http://" + s.listener.Addr().String() } diff --git a/http/server_test.go b/http/server_test.go index 4065267c26..7a2833f0a1 100644 --- a/http/server_test.go +++ b/http/server_test.go @@ -65,11 +65,28 @@ var testHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) w.WriteHeader(http.StatusOK) }) +func TestServerServeWithNoListener(t *testing.T) { + srv, err := NewServer(testHandler) + require.NoError(t, err) + + err = srv.Serve() + require.ErrorIs(t, err, ErrNoListener) +} + +func TestServerServeWithTLSAndNoListener(t *testing.T) { + certPath, keyPath := writeTestCerts(t) + srv, err := NewServer(testHandler, WithTLSCertPath(certPath), WithTLSKeyPath(keyPath)) + require.NoError(t, err) + + err = srv.Serve() + require.ErrorIs(t, err, ErrNoListener) +} + func TestServerListenAndServeWithInvalidAddress(t *testing.T) { srv, err := NewServer(testHandler, WithAddress("invalid")) require.NoError(t, err) - err = srv.ListenAndServe() + err = srv.SetListener() require.ErrorContains(t, err, "address invalid") } @@ -78,15 +95,23 @@ func TestServerListenAndServeWithTLSAndInvalidAddress(t *testing.T) { srv, err := NewServer(testHandler, WithAddress("invalid"), WithTLSCertPath(certPath), WithTLSKeyPath(keyPath)) require.NoError(t, err) - err = srv.ListenAndServe() + err = srv.SetListener() require.ErrorContains(t, err, "address invalid") } func TestServerListenAndServeWithTLSAndInvalidCerts(t *testing.T) { - srv, err := NewServer(testHandler, WithAddress("invalid"), WithTLSCertPath("invalid.crt"), WithTLSKeyPath("invalid.key")) + srv, err := NewServer( + testHandler, + WithAddress("invalid"), + WithTLSCertPath("invalid.crt"), + WithTLSKeyPath("invalid.key"), + WithAddress("127.0.0.1:30001"), + ) require.NoError(t, err) - err = srv.ListenAndServe() + err = srv.SetListener() + require.NoError(t, err) + err = srv.Serve() require.ErrorContains(t, err, "no such file or directory") } @@ -95,7 +120,9 @@ func TestServerListenAndServeWithAddress(t *testing.T) { require.NoError(t, err) go func() { - err := srv.ListenAndServe() + err := srv.SetListener() + require.NoError(t, err) + err = srv.Serve() require.ErrorIs(t, http.ErrServerClosed, err) }() @@ -118,7 +145,9 @@ func TestServerListenAndServeWithTLS(t *testing.T) { require.NoError(t, err) go func() { - err := srv.ListenAndServe() + err := srv.SetListener() + require.NoError(t, err) + err = srv.Serve() require.ErrorIs(t, http.ErrServerClosed, err) }() @@ -140,7 +169,9 @@ func TestServerListenAndServeWithAllowedOrigins(t *testing.T) { require.NoError(t, err) go func() { - err := srv.ListenAndServe() + err := srv.SetListener() + require.NoError(t, err) + err = srv.Serve() require.ErrorIs(t, http.ErrServerClosed, err) }() diff --git a/net/peer.go b/net/peer.go index acdba2e9c8..0c456d5b18 100644 --- a/net/peer.go +++ b/net/peer.go @@ -177,6 +177,7 @@ func (p *Peer) Start() error { go p.handleBroadcastLoop() } + log.FeedbackInfo(p.ctx, "Starting P2P node", logging.NewKV("P2P addresses", p.host.Addrs())) // register the P2P gRPC server go func() { pb.RegisterServiceServer(p.p2pRPC, p.server) diff --git a/node/node.go b/node/node.go index f13fd0b1c2..841a95554d 100644 --- a/node/node.go +++ b/node/node.go @@ -12,6 +12,8 @@ package node import ( "context" + "fmt" + gohttp "net/http" "github.com/libp2p/go-libp2p/core/peer" @@ -159,8 +161,13 @@ func (n *Node) Start(ctx context.Context) error { } } if n.Server != nil { + err := n.Server.SetListener() + if err != nil { + return err + } + log.FeedbackInfo(ctx, fmt.Sprintf("Providing HTTP API at %s.", n.Server.Address())) go func() { - if err := n.Server.ListenAndServe(); err != nil { + if err := n.Server.Serve(); err != nil && err != gohttp.ErrServerClosed { log.FeedbackErrorE(ctx, "HTTP server stopped", err) } }() From f4bec708d0611e7a3c263c0506b7f5f25cf36f96 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 7 Mar 2024 18:52:21 -0500 Subject: [PATCH 2/2] fix lint and hanging listener in test --- http/server_test.go | 2 ++ node/node.go | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/http/server_test.go b/http/server_test.go index 7a2833f0a1..ec9ab8ab75 100644 --- a/http/server_test.go +++ b/http/server_test.go @@ -113,6 +113,8 @@ func TestServerListenAndServeWithTLSAndInvalidCerts(t *testing.T) { require.NoError(t, err) err = srv.Serve() require.ErrorContains(t, err, "no such file or directory") + err = srv.listener.Close() + require.NoError(t, err) } func TestServerListenAndServeWithAddress(t *testing.T) { diff --git a/node/node.go b/node/node.go index 841a95554d..89bedd56ff 100644 --- a/node/node.go +++ b/node/node.go @@ -12,6 +12,7 @@ package node import ( "context" + "errors" "fmt" gohttp "net/http" @@ -167,7 +168,7 @@ func (n *Node) Start(ctx context.Context) error { } log.FeedbackInfo(ctx, fmt.Sprintf("Providing HTTP API at %s.", n.Server.Address())) go func() { - if err := n.Server.Serve(); err != nil && err != gohttp.ErrServerClosed { + if err := n.Server.Serve(); err != nil && !errors.Is(err, gohttp.ErrServerClosed) { log.FeedbackErrorE(ctx, "HTTP server stopped", err) } }()