serve http, webdav, restic: ensure rclone exits if the port is in use

cmd/serve/http/http.go

@@ -46,7 +46,11 @@ control the stats printing.
 		f := cmd.NewFsSrc(args)
 		cmd.Run(false, true, command, func() error {
 			s := newServer(f, &httpflags.Opt)
-			s.serve()
+			err := s.Serve()
+			if err != nil {
+				return err
+			}
+			s.Wait()
 			return nil
 		})
 	},
@@ -54,30 +58,32 @@ control the stats printing.
 
 // server contains everything to run the server
 type server struct {
+	*httplib.Server
 	f   fs.Fs
 	vfs *vfs.VFS
-	srv *httplib.Server
 }
 
 func newServer(f fs.Fs, opt *httplib.Options) *server {
 	mux := http.NewServeMux()
 	s := &server{
-		f:   f,
-		vfs: vfs.New(f, &vfsflags.Opt),
-		srv: httplib.NewServer(mux, opt),
+		Server: httplib.NewServer(mux, opt),
+		f:      f,
+		vfs:    vfs.New(f, &vfsflags.Opt),
 	}
 	mux.HandleFunc("/", s.handler)
 	return s
 }
 
-// serve runs the http server - doesn't return
-func (s *server) serve() {
-	err := s.srv.Serve()
+// Serve runs the http server in the background.
+//
+// Use s.Close() and s.Wait() to shutdown server
+func (s *server) Serve() error {
+	err := s.Server.Serve()
 	if err != nil {
-		fs.Errorf(s.f, "Opening listener: %v", err)
+		return err
 	}
-	fs.Logf(s.f, "Serving on %s", s.srv.URL())
-	s.srv.Wait()
+	fs.Logf(s.f, "Serving on %s", s.URL())
+	return nil
 }
 
 // handler reads incoming requests and dispatches them

cmd/serve/http/http_test.go

@@ -35,7 +35,7 @@ func startServer(t *testing.T, f fs.Fs) {
 	opt := httplib.DefaultOpt
 	opt.ListenAddr = testBindAddress
 	httpServer = newServer(f, &opt)
-	go httpServer.serve()
+	assert.NoError(t, httpServer.Serve())
 
 	// try to connect to the test server
 	pause := time.Millisecond
@@ -233,5 +233,6 @@ func TestAddEntry(t *testing.T) {
 }
 
 func TestFinalise(t *testing.T) {
-	httpServer.srv.Close()
+	httpServer.Close()
+	httpServer.Wait()
 }

cmd/serve/httplib/httplib.go

@@ -13,6 +13,7 @@ import (
 
 	auth "github.com/abbot/go-http-auth"
 	"github.com/ncw/rclone/fs"
+	"github.com/pkg/errors"
 )
 
 // Globals
@@ -188,7 +189,7 @@ func NewServer(handler http.Handler, opt *Options) *Server {
 func (s *Server) Serve() error {
 	ln, err := net.Listen("tcp", s.httpServer.Addr)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "start server failed")
 	}
 	s.listener = ln
 	s.waitChan = make(chan struct{})

cmd/serve/restic/restic.go

@@ -138,8 +138,11 @@ these **must** end with /.  Eg
 				httpSrv.ServeConn(conn, opts)
 				return nil
 			}
-
-			s.serve()
+			err := s.Serve()
+			if err != nil {
+				return err
+			}
+			s.Wait()
 			return nil
 		})
 	},
@@ -151,28 +154,30 @@ const (
 
 // server contains everything to run the server
 type server struct {
-	f   fs.Fs
-	srv *httplib.Server
+	*httplib.Server
+	f fs.Fs
 }
 
 func newServer(f fs.Fs, opt *httplib.Options) *server {
 	mux := http.NewServeMux()
 	s := &server{
-		f:   f,
-		srv: httplib.NewServer(mux, opt),
+		Server: httplib.NewServer(mux, opt),
+		f:      f,
 	}
 	mux.HandleFunc("/", s.handler)
 	return s
 }
 
-// serve runs the http server - doesn't return
-func (s *server) serve() {
-	err := s.srv.Serve()
+// Serve runs the http server in the background.
+//
+// Use s.Close() and s.Wait() to shutdown server
+func (s *server) Serve() error {
+	err := s.Server.Serve()
 	if err != nil {
-		fs.Errorf(s.f, "Opening listener: %v", err)
+		return err
 	}
-	fs.Logf(s.f, "Serving restic REST API on %s", s.srv.URL())
-	s.srv.Wait()
+	fs.Logf(s.f, "Serving restic REST API on %s", s.URL())
+	return nil
 }
 
 var matchData = regexp.MustCompile("(?:^|/)data/([^/]{2,})$")

cmd/serve/restic/restic_test.go

@@ -41,8 +41,11 @@ func TestRestic(t *testing.T) {
 
 	// Start the server
 	w := newServer(fremote, &opt)
-	go w.serve()
-	defer w.srv.Close()
+	assert.NoError(t, w.Serve())
+	defer func() {
+		w.Close()
+		w.Wait()
+	}()
 
 	// Change directory to run the tests
 	err = os.Chdir(resticSource)

cmd/serve/webdav/webdav.go

@@ -68,8 +68,12 @@ Use "rclone hashsum" to see the full list.
 			fs.Debugf(f, "Using hash %v for ETag", hashType)
 		}
 		cmd.Run(false, false, command, func() error {
-			w := newWebDAV(f, &httpflags.Opt)
-			w.serve()
+			s := newWebDAV(f, &httpflags.Opt)
+			err := s.serve()
+			if err != nil {
+				return err
+			}
+			s.Wait()
 			return nil
 		})
 		return nil
@@ -89,9 +93,9 @@ Use "rclone hashsum" to see the full list.
 // might apply". In particular, whether or not renaming a file or directory
 // overwriting another existing file or directory is an error is OS-dependent.
 type WebDAV struct {
+	*httplib.Server
 	f   fs.Fs
 	vfs *vfs.VFS
-	srv *httplib.Server
 }
 
 // check interface
@@ -110,18 +114,20 @@ func newWebDAV(f fs.Fs, opt *httplib.Options) *WebDAV {
 		Logger:     w.logRequest, // FIXME
 	}
 
-	w.srv = httplib.NewServer(handler, opt)
+	w.Server = httplib.NewServer(handler, opt)
 	return w
 }
 
-// serve runs the http server - doesn't return
-func (w *WebDAV) serve() {
-	err := w.srv.Serve()
+// serve runs the http server in the background.
+//
+// Use s.Close() and s.Wait() to shutdown server
+func (w *WebDAV) serve() error {
+	err := w.Serve()
 	if err != nil {
-		fs.Errorf(w.f, "Opening listener: %v", err)
+		return err
 	}
-	fs.Logf(w.f, "WebDav Server started on %s", w.srv.URL())
-	w.srv.Wait()
+	fs.Logf(w.f, "WebDav Server started on %s", w.URL())
+	return nil
 }
 
 // logRequest is called by the webdav module on every request

cmd/serve/webdav/webdav_test.go

@@ -48,8 +48,11 @@ func TestWebDav(t *testing.T) {
 
 	// Start the server
 	w := newWebDAV(fremote, &opt)
-	go w.serve()
-	defer w.srv.Close()
+	assert.NoError(t, w.serve())
+	defer func() {
+		w.Close()
+		w.Wait()
+	}()
 
 	// Change directory to run the tests
 	err = os.Chdir("../../../backend/webdav")