Merge pull request #2221 from gobuffalo/development

v0.18.5

events.go

@@ -1,5 +1,14 @@
 package buffalo
 
+// TODO: TODO-v1 check if they are really need to be exported.
+/* The event id should be unique across packages as the format of
+   "<package-name>:<additional-names>:<optional-error>" as documented. They
+   should not be used by another packages to keep it informational. To make
+   it sure, they need to be internal.
+   Especially for plugable conponents like servers or workers, they can have
+   their own event definition if they need but the buffalo runtime can emit
+   generalize events when e.g. the runtime calls configured worker.
+*/
 const (
 	// EvtAppStart is emitted when buffalo.App#Serve is called
 	EvtAppStart = "buffalo:app:start"
@@ -18,12 +27,20 @@ const (
 	// EvtRouteErr is emitted when there is a problem handling processing a route
 	EvtRouteErr = "buffalo:route:err"
 
-	// EvtWorkerStart is emitted when buffalo.App#Serve is called and workers are started
+	// EvtServerStart is emitted when buffalo is about to start servers
+	EvtServerStart = "buffalo:server:start"
+	// EvtServerStartErr is emitted when an error occurs when starting servers
+	EvtServerStartErr = "buffalo:server:start:err"
+	// EvtServerStop is emitted when buffalo is about to stop servers
+	EvtServerStop = "buffalo:server:stop"
+	// EvtServerStopErr is emitted when an error occurs when stopping servers
+	EvtServerStopErr = "buffalo:server:stop:err"
+
+	// EvtWorkerStart is emitted when buffalo is about to start workers
 	EvtWorkerStart = "buffalo:worker:start"
 	// EvtWorkerStartErr is emitted when an error occurs when starting workers
 	EvtWorkerStartErr = "buffalo:worker:start:err"
-
-	// EvtWorkerStop is emitted when buffalo.App#Stop is called and workers are stopped
+	// EvtWorkerStop is emitted when buffalo is about to stop workers
 	EvtWorkerStop = "buffalo:worker:stop"
 	// EvtWorkerStopErr is emitted when an error occurs when stopping workers
 	EvtWorkerStopErr = "buffalo:worker:stop:err"

go.mod

@@ -18,6 +18,7 @@ require (
 	github.com/gobuffalo/plush/v4 v4.1.9
 	github.com/gobuffalo/pop/v6 v6.0.1
 	github.com/gobuffalo/tags/v3 v3.1.2
+	github.com/google/go-cmp v0.5.6 // indirect
 	github.com/gorilla/handlers v1.5.1
 	github.com/gorilla/mux v1.8.0
 	github.com/gorilla/sessions v1.2.1
@@ -26,11 +27,14 @@ require (
 	github.com/markbates/oncer v1.0.0
 	github.com/markbates/refresh v1.12.0
 	github.com/markbates/safe v1.0.1
-	github.com/markbates/sigtx v1.0.0
+	github.com/mattn/go-colorable v0.1.12 // indirect
 	github.com/monoculum/formam v3.5.5+incompatible
 	github.com/psanford/memfs v0.0.0-20210214183328-a001468d78ef
 	github.com/sirupsen/logrus v1.8.1
-	github.com/spf13/cobra v1.3.0
-	github.com/stretchr/testify v1.7.0
+	github.com/spf13/cobra v1.4.0
+	github.com/stretchr/testify v1.7.1
+	golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 // indirect
+	golang.org/x/sys v0.0.0-20211205182925-97ca703d548d // indirect
+	golang.org/x/text v0.3.7 // indirect
 	gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc
 )

options.go

@@ -48,6 +48,10 @@ type Options struct {
 	// to "_buffalo_session".
 	SessionName string `json:"session_name"`
 
+	// Timeout in second for ongoing requests when shutdown the server.
+	// The default value is 60.
+	TimeoutSecondShutdown int `json:"timeout_second_shutdown"`
+
 	// Worker implements the Worker interface and can process tasks in the background.
 	// Default is "github.com/gobuffalo/worker.Simple.
 	Worker worker.Worker `json:"-"`
@@ -99,6 +103,7 @@ func optionsWithDefaults(opts Options) Options {
 		// TCP case
 		opts.Addr = defaults.String(opts.Addr, fmt.Sprintf("%s:%s", envAddr, envy.Get("PORT", "3000")))
 	}
+	opts.Host = defaults.String(opts.Host, envy.Get("HOST", fmt.Sprintf("http://127.0.0.1:%s", envy.Get("PORT", "3000"))))
 
 	if opts.PreWares == nil {
 		opts.PreWares = []PreWare{}
@@ -176,12 +181,15 @@ func optionsWithDefaults(opts Options) Options {
 
 		opts.SessionStore = cookieStore
 	}
+	opts.SessionName = defaults.String(opts.SessionName, "_buffalo_session")
+
 	if opts.Worker == nil {
 		w := worker.NewSimpleWithContext(opts.Context)
 		w.Logger = opts.Logger
 		opts.Worker = w
 	}
-	opts.SessionName = defaults.String(opts.SessionName, "_buffalo_session")
-	opts.Host = defaults.String(opts.Host, envy.Get("HOST", fmt.Sprintf("http://127.0.0.1:%s", envy.Get("PORT", "3000"))))
+
+	opts.TimeoutSecondShutdown = defaults.Int(opts.TimeoutSecondShutdown, 60)
+
 	return opts
 }

runtime/version.go

@@ -1,4 +1,4 @@
 package runtime
 
 // Version is the current version of the buffalo binary
-var Version = "v0.18.4"
+var Version = "v0.18.5"

server.go

@@ -5,25 +5,33 @@ import (
 	"errors"
 	"net/http"
 	"os"
+	"os/signal"
 	"strings"
+	"sync"
 	"syscall"
+	"time"
 
 	"github.com/gobuffalo/buffalo/servers"
 	"github.com/gobuffalo/events"
 	"github.com/markbates/refresh/refresh/web"
-	"github.com/markbates/sigtx"
 )
 
 // Serve the application at the specified address/port and listen for OS
 // interrupt and kill signals and will attempt to stop the application
 // gracefully. This will also start the Worker process, unless WorkerOff is enabled.
 func (a *App) Serve(srvs ...servers.Server) error {
-	a.Logger.Infof("Starting application at http://%s", a.Options.Addr)
+	var wg sync.WaitGroup
+
+	// FIXME: this information is not correct.
+	// It needs to be fixed as we support multiple servers.
+	a.Logger.Infof("starting application at http://%s", a.Options.Addr)
 
 	payload := events.Payload{
 		"app": a,
 	}
 	if err := events.EmitPayload(EvtAppStart, payload); err != nil {
+		// just to make sure if events work properly?
+		a.Logger.Error("unable to emit event. something went wrong internally")
 		return err
 	}
 
@@ -39,70 +47,88 @@ func (a *App) Serve(srvs ...servers.Server) error {
 		}
 	}
 
-	ctx, cancel := sigtx.WithCancel(a.Context, syscall.SIGTERM, os.Interrupt)
+	ctx, cancel := signal.NotifyContext(a.Context, syscall.SIGTERM, os.Interrupt)
 	defer cancel()
 
+	wg.Add(1)
 	go func() {
 		// gracefully shut down the application when the context is cancelled
+		defer wg.Done()
+		// channel waiter should not be called any other place
 		<-ctx.Done()
-		a.Logger.Info("Shutting down application")
 
-		events.EmitError(EvtAppStop, ctx.Err(), payload)
+		a.Logger.Info("shutting down application")
 
-		if err := a.Stop(ctx.Err()); err != nil {
-			events.EmitError(EvtAppStopErr, err, payload)
-			a.Logger.Error(err)
+		// shutting down listeners first, to make sure no more new request
+		a.Logger.Info("shutting down servers")
+		for _, s := range srvs {
+			timeout := time.Duration(a.Options.TimeoutSecondShutdown) * time.Second
+			ctx, cfn := context.WithTimeout(context.Background(), timeout)
+			defer cfn()
+			events.EmitPayload(EvtServerStop, payload)
+			if err := s.Shutdown(ctx); err != nil {
+				events.EmitError(EvtServerStopErr, err, payload)
+				a.Logger.Error("shutting down server: ", err)
+			}
+			cfn()
 		}
 
 		if !a.WorkerOff {
-			// stop the workers
-			a.Logger.Info("Shutting down worker")
+			a.Logger.Info("shutting down worker")
 			events.EmitPayload(EvtWorkerStop, payload)
 			if err := a.Worker.Stop(); err != nil {
 				events.EmitError(EvtWorkerStopErr, err, payload)
-				a.Logger.Error(err)
-			}
-		}
-
-		for _, s := range srvs {
-			if err := s.Shutdown(ctx); err != nil {
-				a.Logger.Error(err)
+				a.Logger.Error("error while shutting down worker: ", err)
 			}
 		}
-
 	}()
 
 	// if configured to do so, start the workers
 	if !a.WorkerOff {
+		wg.Add(1)
 		go func() {
+			defer wg.Done()
 			events.EmitPayload(EvtWorkerStart, payload)
 			if err := a.Worker.Start(ctx); err != nil {
+				events.EmitError(EvtWorkerStartErr, err, payload)
 				a.Stop(err)
 			}
 		}()
 	}
 
 	for _, s := range srvs {
 		s.SetAddr(a.Addr)
+		wg.Add(1)
 		go func(s servers.Server) {
-			if err := s.Start(ctx, a); err != nil {
-				a.Stop(err)
-			}
+			defer wg.Done()
+			events.EmitPayload(EvtServerStart, payload)
+			// s.Start always returns non-nil error
+			a.Stop(s.Start(ctx, a))
 		}(s)
 	}
 
-	<-ctx.Done()
+	wg.Wait()
+	a.Logger.Info("shutdown completed")
 
-	return a.Context.Err()
+	err := ctx.Err()
+	if errors.Is(err, context.Canceled) {
+		return nil
+	}
+	return err
 }
 
 // Stop the application and attempt to gracefully shutdown
 func (a *App) Stop(err error) error {
-	a.cancel()
-	if err != nil && !errors.Is(err, context.Canceled) {
-		a.Logger.Error(err)
-		return err
+	events.EmitError(EvtAppStop, err, events.Payload{"app": a})
+
+	ce := a.Context.Err()
+	if ce != nil {
+		a.Logger.Warn("application context has already been canceled: ", ce)
+		return errors.New("application has already been canceled")
 	}
+
+	a.Logger.Warn("stopping application: ", err)
+	a.cancel()
 	return nil
 }