diff --git a/api.go b/api.go index da5745ec..f9d0da6b 100644 --- a/api.go +++ b/api.go @@ -3,10 +3,12 @@ package toxiproxy import ( "context" "encoding/json" + "errors" "fmt" "net/http" "os" "strings" + "sync" "time" "github.com/gorilla/mux" @@ -34,7 +36,9 @@ type ApiServer struct { Collection *ProxyCollection Metrics *metricsContainer Logger *zerolog.Logger - http *http.Server + + initOnce sync.Once // guards shutdown + shutdown func(ctx context.Context) error } const ( @@ -50,42 +54,45 @@ func NewServer(m *metricsContainer, logger zerolog.Logger) *ApiServer { } } -func (server *ApiServer) Listen(addr string) error { - server.Logger. - Info(). - Str("address", addr). - Msg("Starting Toxiproxy HTTP server") +var errServerStarted = errors.New("server already started") - server.http = &http.Server{ - Addr: addr, - Handler: server.Routes(), - WriteTimeout: wait_timeout, - ReadTimeout: read_timeout, - IdleTimeout: 60 * time.Second, +func (server *ApiServer) Listen(addr string) error { + var s *http.Server + server.initOnce.Do(func() { + server.Logger. + Info(). + Str("address", addr). + Msg("Starting Toxiproxy HTTP server") + + s = &http.Server{ + Addr: addr, + Handler: server.Routes(), + WriteTimeout: wait_timeout, + ReadTimeout: read_timeout, + IdleTimeout: 60 * time.Second, + } + }) + if s == nil { + return errServerStarted } - err := server.http.ListenAndServe() - if err == http.ErrServerClosed { - err = nil + server.shutdown = s.Shutdown + err := s.ListenAndServe() + if errors.Is(err, http.ErrServerClosed) { + return nil } - return err } func (server *ApiServer) Shutdown() error { - if server.http == nil { - return nil + server.initOnce.Do(func() {}) + if server.shutdown == nil { + return nil // in desire state } ctx, cancel := context.WithTimeout(context.Background(), wait_timeout) defer cancel() - - err := server.http.Shutdown(ctx) - if err != nil { - return err - } - - return nil + return server.shutdown(ctx) } func (server *ApiServer) Routes() *mux.Router { diff --git a/api_test.go b/api_test.go index a13fcf35..0940c401 100644 --- a/api_test.go +++ b/api_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "os" + "strings" "testing" "time" @@ -1124,6 +1125,25 @@ func TestInvalidStream(t *testing.T) { }) } +// Issue https://github.com/Shopify/toxiproxy/issues/667 +func TestApiServerShutdownListen(t *testing.T) { + s := &toxiproxy.ApiServer{Logger: &zerolog.Logger{}} + defer s.Shutdown() + + got := s.Shutdown() + if got != nil { + t.Fatal("error shutting down unstarted server:", got) + } + + got = s.Listen(":0") + // Avoid inventing an exported sentinal error just for this test, which is heavy handed + // given this error is specific only to starting the server twice, via the Listen method, + // and is unlikely to change, we can just do a substring match. + if !strings.Contains(got.Error(), "server already started") { + t.Fatal("error starting server:", got) + } +} + func AssertToxicExists( t *testing.T, toxics tclient.Toxics,