Skip to content

Commit

Permalink
internal/transport: skip log on EOF when reading client preface (#4458)
Browse files Browse the repository at this point in the history
  • Loading branch information
easwars committed Jun 2, 2021
1 parent e7b12ef commit 174b1c2
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 16 deletions.
2 changes: 1 addition & 1 deletion call_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func (s *server) start(t *testing.T, port int, maxStreams uint32) {
config := &transport.ServerConfig{
MaxStreams: maxStreams,
}
st, err := transport.NewServerTransport("http2", conn, config)
st, err := transport.NewServerTransport(conn, config)
if err != nil {
continue
}
Expand Down
18 changes: 15 additions & 3 deletions internal/transport/http2_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,14 @@ type http2Server struct {
connectionID uint64
}

// newHTTP2Server constructs a ServerTransport based on HTTP2. ConnectionError is
// returned if something goes wrong.
func newHTTP2Server(conn net.Conn, config *ServerConfig) (_ ServerTransport, err error) {
// NewServerTransport creates a http2 transport with conn and configuration
// options from config.
//
// It returns a non-nil transport and a nil error on success. On failure, it
// returns a non-nil transport and a nil-error. For a special case where the
// underlying conn gets closed before the client preface could be read, it
// returns a nil transport and a nil error.
func NewServerTransport(conn net.Conn, config *ServerConfig) (_ ServerTransport, err error) {
writeBufSize := config.WriteBufferSize
readBufSize := config.ReadBufferSize
maxHeaderListSize := defaultServerMaxHeaderListSize
Expand Down Expand Up @@ -266,6 +271,13 @@ func newHTTP2Server(conn net.Conn, config *ServerConfig) (_ ServerTransport, err
// Check the validity of client preface.
preface := make([]byte, len(clientPreface))
if _, err := io.ReadFull(t.conn, preface); err != nil {
// In deployments where a gRPC server runs behind a cloud load balancer
// which performs regular TCP level health checks, the connection is
// closed immediately by the latter. Skipping the error here will help
// reduce log clutter.
if err == io.EOF {
return nil, nil
}
return nil, connectionErrorf(false, err, "transport: http2Server.HandleStreams failed to receive the preface from client: %v", err)
}
if !bytes.Equal(preface, clientPreface) {
Expand Down
6 changes: 0 additions & 6 deletions internal/transport/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,12 +532,6 @@ type ServerConfig struct {
HeaderTableSize *uint32
}

// NewServerTransport creates a ServerTransport with conn or non-nil error
// if it fails.
func NewServerTransport(protocol string, conn net.Conn, config *ServerConfig) (ServerTransport, error) {
return newHTTP2Server(conn, config)
}

// ConnectOptions covers all relevant options for communicating with the server.
type ConnectOptions struct {
// UserAgent is the application user agent.
Expand Down
2 changes: 1 addition & 1 deletion internal/transport/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func (s *server) start(t *testing.T, port int, serverConfig *ServerConfig, ht hT
if err != nil {
return
}
transport, err := NewServerTransport("http2", conn, serverConfig)
transport, err := NewServerTransport(conn, serverConfig)
if err != nil {
return
}
Expand Down
16 changes: 11 additions & 5 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -844,10 +844,16 @@ func (s *Server) handleRawConn(lisAddr string, rawConn net.Conn) {
// ErrConnDispatched means that the connection was dispatched away from
// gRPC; those connections should be left open.
if err != credentials.ErrConnDispatched {
s.mu.Lock()
s.errorf("ServerHandshake(%q) failed: %v", rawConn.RemoteAddr(), err)
s.mu.Unlock()
channelz.Warningf(logger, s.channelzID, "grpc: Server.Serve failed to complete security handshake from %q: %v", rawConn.RemoteAddr(), err)
// In deployments where a gRPC server runs behind a cloud load
// balancer which performs regular TCP level health checks, the
// connection is closed immediately by the latter. Skipping the
// error here will help reduce log clutter.
if err != io.EOF {
s.mu.Lock()
s.errorf("ServerHandshake(%q) failed: %v", rawConn.RemoteAddr(), err)
s.mu.Unlock()
channelz.Warningf(logger, s.channelzID, "grpc: Server.Serve failed to complete security handshake from %q: %v", rawConn.RemoteAddr(), err)
}
rawConn.Close()
}
rawConn.SetDeadline(time.Time{})
Expand Down Expand Up @@ -897,7 +903,7 @@ func (s *Server) newHTTP2Transport(c net.Conn, authInfo credentials.AuthInfo) tr
MaxHeaderListSize: s.opts.maxHeaderListSize,
HeaderTableSize: s.opts.headerTableSize,
}
st, err := transport.NewServerTransport("http2", c, config)
st, err := transport.NewServerTransport(c, config)
if err != nil {
s.mu.Lock()
s.errorf("NewServerTransport(%q) failed: %v", c.RemoteAddr(), err)
Expand Down

0 comments on commit 174b1c2

Please sign in to comment.