Skip to content
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
26 changes: 24 additions & 2 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -421,11 +422,28 @@ func (s *server) runWSEventLoop(newSession NewSessionCallback) {
s.lock.Unlock()
err = server.Serve(s.streamListener)
if err != nil {
log.Errorf("http.server.Serve(addr{%s}) = err:%+v", s.addr, perrors.WithStack(err))
if isHTTPServeCloseError(err) {
s.logHTTPServeClose(err)
} else {
s.logHTTPServeError(err)
}
}
}()
}

func (s *server) logHTTPServeClose(err error) {
log.Infof("http.server.Serve(addr{%s}) closed: %v", s.addr, err)
}

func isHTTPServeCloseError(err error) bool {
return errors.Is(err, http.ErrServerClosed) ||
errors.Is(err, net.ErrClosed)
}

func (s *server) logHTTPServeError(err error) {
log.Errorf("http.server.Serve(addr{%s}) = err:%+v", s.addr, perrors.WithStack(err))
}

// serve websocket client request
// RunWSSEventLoop serve websocket client request
func (s *server) runWSSEventLoop(newSession NewSessionCallback) {
Expand Down Expand Up @@ -481,7 +499,11 @@ func (s *server) runWSSEventLoop(newSession NewSessionCallback) {
s.lock.Unlock()
err = server.Serve(tls.NewListener(s.streamListener, config))
if err != nil {
log.Errorf("http.server.Serve(addr{%s}) = err:%+v", s.addr, perrors.WithStack(err))
if isHTTPServeCloseError(err) {
s.logHTTPServeClose(err)
return
}
s.logHTTPServeError(err)
panic(err)
}
}()
Expand Down
184 changes: 184 additions & 0 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ import (
"bufio"
"bytes"
"errors"
"fmt"
"io"
"net"
"net/http"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
Expand Down Expand Up @@ -226,6 +229,100 @@ func TestWSServeWSRequestClosesSelfConnectConn(t *testing.T) {
}
}

func TestWSSServerCloseDoesNotPanic(t *testing.T) {
certFile := filepath.Join(t.TempDir(), "server.crt")
keyFile := filepath.Join(t.TempDir(), "server.key")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 补一条 unexpected Serve error 仍会失败的回归测试

当前用例只证明正常 Close() 不再触发 panic,没有保护本 PR 同时承诺保留的 fatal-error 路径。我在独立副本中把 isHTTPServeCloseError 变异为无条件返回 true 后,这个测试重复 20 次和全仓 go test ./... 仍全部通过,也就是所有 WSS Serve 故障被静默降级的回归无法被发现。请用可注入的 fake listener,必要时放到子进程中,分别验证 http.ErrServerClosed/wrapped net.ErrClosed 正常退出,而普通 fatal Accept/Serve error 必须进入 error 日志并保持 panic。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

收到,已补充

assert.NoError(t, DownloadFile(certFile, WssServerCRT))
assert.NoError(t, DownloadFile(keyFile, WssServerKEY))

srv := NewWSSServer(
WithLocalAddress("127.0.0.1:0"),
WithWebsocketServerPath("/hello"),
WithWebsocketServerCert(certFile),
WithWebsocketServerPrivateKey(keyFile),
)
defer srv.Close()

var serverMsgHandler MessageHandler
srv.RunEventLoop(func(session Session) error {
return newSessionCallback(session, &serverMsgHandler)
})

waitUntilHTTPServerReady(t, srv.(*server))
srv.Close()
assert.True(t, srv.IsClosed())
}

func TestHTTPServeCloseErrorClassification(t *testing.T) {
tests := []struct {
name string
err error
want bool
}{
{
name: "http server closed",
err: http.ErrServerClosed,
want: true,
},
{
name: "wrapped net closed",
err: fmt.Errorf("wrapped: %w", net.ErrClosed),
want: true,
},
{
name: "fatal error with closed listener text",
err: errors.New("fatal listener failure after use of closed network connection marker"),
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, isHTTPServeCloseError(tt.err))
})
}
}

func TestWSSEventLoopServeCloseErrorDoesNotPanic(t *testing.T) {
srv := newTestWSSServerWithListener(t, &serveErrorListener{
err: fmt.Errorf("listener closed: %w", net.ErrClosed),
})

srv.runWSSEventLoop(func(Session) error {
return nil
})
waitUntilHTTPServerReady(t, srv)
waitUntilServerLoopDone(t, srv)
}

func TestWSSEventLoopUnexpectedServeErrorPanics(t *testing.T) {
if os.Getenv("GETTY_WSS_FATAL_SERVE_ERROR") == "1" {
SetLogger(stderrTestLogger{})
srv := newTestWSSServerWithListener(t, &serveErrorListener{
err: errors.New("fatal listener failure after use of closed network connection marker"),
})

srv.runWSSEventLoop(func(Session) error {
return nil
})
srv.wg.Wait()
t.Fatal("expected WSS Serve error to panic")
}

cmd := exec.Command(os.Args[0], "-test.run=^TestWSSEventLoopUnexpectedServeErrorPanics$")
cmd.Env = append(os.Environ(), "GETTY_WSS_FATAL_SERVE_ERROR=1")
output, err := cmd.CombinedOutput()
if err == nil {
t.Fatalf("expected child process to fail from WSS Serve panic, got success:\n%s", output)
}
if !strings.Contains(string(output), "http.server.Serve") {
t.Fatalf("expected child process output to include WSS Serve error log, got:\n%s", output)
}
if !strings.Contains(string(output), "panic: fatal listener failure after use of closed network connection marker") {
t.Fatalf("expected child process output to include WSS Serve panic, got:\n%s", output)
}
}

type hijackResponseWriter struct {
header http.Header
conn *selfConnectConn
Expand Down Expand Up @@ -286,3 +383,90 @@ func (c *selfConnectConn) SetReadDeadline(time.Time) error {
func (c *selfConnectConn) SetWriteDeadline(time.Time) error {
return nil
}

type serveErrorListener struct {
err error
}

func (l *serveErrorListener) Accept() (net.Conn, error) {
return nil, l.err
}

func (l *serveErrorListener) Close() error {
return nil
}

func (l *serveErrorListener) Addr() net.Addr {
return &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0}
}

func newTestWSSServerWithListener(t *testing.T, listener net.Listener) *server {
t.Helper()

certFile := filepath.Join(t.TempDir(), "server.crt")
keyFile := filepath.Join(t.TempDir(), "server.key")
assert.NoError(t, DownloadFile(certFile, WssServerCRT))
assert.NoError(t, DownloadFile(keyFile, WssServerKEY))

srv := NewWSSServer(
WithLocalAddress(listener.Addr().String()),
WithWebsocketServerPath("/hello"),
WithWebsocketServerCert(certFile),
WithWebsocketServerPrivateKey(keyFile),
).(*server)
srv.streamListener = listener

return srv
}

func waitUntilHTTPServerReady(t *testing.T, server *server) {
t.Helper()

timeout := time.After(3 * time.Second)
tick := time.NewTicker(10 * time.Millisecond)
defer tick.Stop()

for {
server.lock.Lock()
ready := server.server != nil
server.lock.Unlock()
if ready {
return
}

select {
case <-timeout:
t.Fatal("timeout waiting for http server to start")
case <-tick.C:
}
}
}

func waitUntilServerLoopDone(t *testing.T, server *server) {
t.Helper()

done := make(chan struct{})
go func() {
server.wg.Wait()
close(done)
}()

select {
case <-done:
case <-time.After(3 * time.Second):
t.Fatal("timeout waiting for server event loop to exit")
}
}

type stderrTestLogger struct{}

func (stderrTestLogger) Info(args ...any) {}
func (stderrTestLogger) Warn(args ...any) {}
func (stderrTestLogger) Error(args ...any) {}
func (stderrTestLogger) Debug(args ...any) {}
func (stderrTestLogger) Infof(string, ...any) {}
func (stderrTestLogger) Warnf(string, ...any) {}
func (stderrTestLogger) Errorf(format string, args ...any) {
fmt.Fprintf(os.Stderr, format+"\n", args...)
}
func (stderrTestLogger) Debugf(string, ...any) {}
Loading