Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Add missing logs at startup #2391

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 32 additions & 18 deletions http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,10 @@

// 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.
Expand Down Expand Up @@ -148,25 +150,34 @@
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
Expand All @@ -177,9 +188,12 @@
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()

Check warning on line 196 in http/server.go

View check run for this annotation

Codecov / codecov/patch

http/server.go#L196

Added line #L196 was not covered by tests
}
return s.server.Serve(tls.NewListener(listener, config))
return "http://" + s.listener.Addr().String()
}
47 changes: 40 additions & 7 deletions http/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand All @@ -78,24 +95,36 @@ 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")
err = srv.listener.Close()
require.NoError(t, err)
}

func TestServerListenAndServeWithAddress(t *testing.T) {
srv, err := NewServer(testHandler, WithAddress("127.0.0.1:30001"))
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)
}()

Expand All @@ -118,7 +147,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)
}()

Expand All @@ -140,7 +171,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)
}()

Expand Down
1 change: 1 addition & 0 deletions net/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 9 additions & 1 deletion node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

import (
"context"
"errors"
"fmt"
gohttp "net/http"

"github.com/libp2p/go-libp2p/core/peer"

Expand Down Expand Up @@ -159,8 +162,13 @@
}
}
if n.Server != nil {
err := n.Server.SetListener()
if err != nil {
return err
}

Check warning on line 168 in node/node.go

View check run for this annotation

Codecov / codecov/patch

node/node.go#L167-L168

Added lines #L167 - L168 were not covered by tests
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 && !errors.Is(err, gohttp.ErrServerClosed) {
log.FeedbackErrorE(ctx, "HTTP server stopped", err)
}
}()
Expand Down
Loading