From 84493b7c55da9ec215227e2eae31b0de48a0bf76 Mon Sep 17 00:00:00 2001 From: moreirathomas Date: Wed, 2 Mar 2022 18:19:19 +0100 Subject: [PATCH 1/5] feat: more granular server bootstrap --- cmd/main.go | 104 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 86 insertions(+), 18 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index e1e8d16..f4f476c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -2,8 +2,9 @@ package main import ( "context" + "errors" "flag" - "log" + "fmt" "os" "github.com/joho/godotenv" @@ -12,35 +13,102 @@ import ( "github.com/benchttp/server/firestore" ) -const defaultPort = "9998" +const ( + defaultConfigPath = ".env" + defaultPort = "9998" +) func main() { - port := flag.String("port", defaultPort, "Address for the server to listen on.") - flag.Parse() - addr := ":" + *port + closeHandle, err := run() - err := godotenv.Load(".env") if err != nil { - log.Fatalf("Error loading .env file") + closeHandle.close() + fmt.Fprintln(os.Stderr, err) + os.Exit(1) } - projectID := os.Getenv("GOOGLE_PROJECT_ID") - if projectID == "" { - log.Fatalf("GOOGLE_PROJECT_ID variable is not defined") + if err := closeHandle.close(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) } +} + +// shutdownHandle +type shutdownHandle struct { + closeFunc func() error +} + +func (c *shutdownHandle) close() error { + return c.closeFunc() +} + +func run() (*shutdownHandle, error) { + c := &shutdownHandle{} - collectionID := os.Getenv("FIRESTORE_COLLECTION_ID") - if projectID == "" { - log.Fatalf("FIRESTORE_COLLECTION_ID variable is not defined") + configPath := flag.String("config", defaultConfigPath, "") + + flag.Parse() + + config, err := readConfigFile(*configPath) + if err != nil { + return c, err } - rs, err := firestore.NewReportService(context.Background(), projectID, collectionID) + rs, err := firestore.NewReportService(context.Background(), config.project, config.collection) if err != nil { - log.Fatal(err) + return c, err + } + + srv := server.New(config.addr, rs) + + c.closeFunc = func() error { + if err := srv.Close(); err != nil { + return err + } + + if err := rs.Close(); err != nil { + return err + } + + return nil + } + + return c, srv.Start() +} + +type config struct { + addr string + project string + collection string +} + +func defaultConfig() config { + return config{ + addr: ":" + defaultPort, + } +} + +func readConfigFile(file string) (config, error) { + config := defaultConfig() + + if err := godotenv.Load(file); err != nil { + return config, err + } + + port := os.Getenv("PORT") + if port != "" { + config.addr = ":" + port + } + + config.project = os.Getenv("GOOGLE_PROJECT_ID") + if config.project == "" { + return config, errors.New("env variable GOOGLE_PROJECT_ID is not defined") } - srv := server.New(addr, rs) - if err := srv.Start(); err != nil { - log.Fatal(err) + config.collection = os.Getenv("FIRESTORE_COLLECTION_ID") + if config.collection == "" { + return config, errors.New("env variable FIRESTORE_COLLECTION_ID is not defined") } + + return config, nil } From 94b6b256817ffc49f1e2cbfb48c9b04adab112c7 Mon Sep 17 00:00:00 2001 From: moreirathomas Date: Wed, 2 Mar 2022 18:19:40 +0100 Subject: [PATCH 2/5] feat: extract shutdown handle in its own pacakge --- cmd/main.go | 37 ++++++++++++++++--------------------- shutdown/handle.go | 22 ++++++++++++++++++++++ shutdown/signal.go | 17 +++++++++++++++++ 3 files changed, 55 insertions(+), 21 deletions(-) create mode 100644 shutdown/handle.go create mode 100644 shutdown/signal.go diff --git a/cmd/main.go b/cmd/main.go index f4f476c..f3051ed 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -11,6 +11,7 @@ import ( "github.com/benchttp/server" "github.com/benchttp/server/firestore" + "github.com/benchttp/server/shutdown" ) const ( @@ -19,50 +20,44 @@ const ( ) func main() { - closeHandle, err := run() + ctx, cancel := context.WithCancel(context.Background()) + go shutdown.ListenInterrupt(cancel) + shutdownHandle, err := run(ctx) if err != nil { - closeHandle.close() + shutdownHandle.Call() // nolint fmt.Fprintln(os.Stderr, err) os.Exit(1) } - if err := closeHandle.close(); err != nil { + // Wait for interrupt. + <-ctx.Done() + + if err := shutdownHandle.Call(); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } } -// shutdownHandle -type shutdownHandle struct { - closeFunc func() error -} - -func (c *shutdownHandle) close() error { - return c.closeFunc() -} - -func run() (*shutdownHandle, error) { - c := &shutdownHandle{} - +func run(ctx context.Context) (*shutdown.Handle, error) { configPath := flag.String("config", defaultConfigPath, "") flag.Parse() config, err := readConfigFile(*configPath) if err != nil { - return c, err + return &shutdown.Handle{}, err } rs, err := firestore.NewReportService(context.Background(), config.project, config.collection) if err != nil { - return c, err + return &shutdown.Handle{}, err } srv := server.New(config.addr, rs) - c.closeFunc = func() error { - if err := srv.Close(); err != nil { + handle := shutdown.NewHandle(func() error { + if err := srv.Shutdown(ctx); err != nil { return err } @@ -71,9 +66,9 @@ func run() (*shutdownHandle, error) { } return nil - } + }) - return c, srv.Start() + return handle, srv.Start() } type config struct { diff --git a/shutdown/handle.go b/shutdown/handle.go new file mode 100644 index 0000000..dc7833e --- /dev/null +++ b/shutdown/handle.go @@ -0,0 +1,22 @@ +package shutdown + +// Handle is a configurable shutdown handle that executes a +// custom shutdown procedure. +type Handle struct { + handleFunc func() error +} + +func NewHandle(f func() error) *Handle { + return &Handle{ + handleFunc: f, + } +} + +// Call calls the registered handlFunc on Handle. +// Invoking Call is noop if handlFunc is nil. +func (h *Handle) Call() error { + if h.handleFunc == nil { + return nil + } + return h.handleFunc() +} diff --git a/shutdown/signal.go b/shutdown/signal.go new file mode 100644 index 0000000..a430992 --- /dev/null +++ b/shutdown/signal.go @@ -0,0 +1,17 @@ +package shutdown + +import ( + "os" + "os/signal" + "syscall" +) + +// ListenInterrupt listens for OS interrupt signals and calls f on receive. +// Calling ListenInterrupt blocks until a signal is received and thus must +// called inside a goroutine separated from the main program. +func ListenInterrupt(f func()) { + sig := make(chan os.Signal, 1) + signal.Notify(sig, os.Interrupt, syscall.SIGTERM, syscall.SIGINT) + <-sig + f() +} From fdd39445516ab6998612f8eb63b248e8e82283dd Mon Sep 17 00:00:00 2001 From: moreirathomas Date: Fri, 4 Mar 2022 15:37:52 +0100 Subject: [PATCH 3/5] fix: calling Server.Start would block main program --- cmd/main.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index f3051ed..cffe05e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -5,6 +5,7 @@ import ( "errors" "flag" "fmt" + "net/http" "os" "github.com/joho/godotenv" @@ -23,7 +24,7 @@ func main() { ctx, cancel := context.WithCancel(context.Background()) go shutdown.ListenInterrupt(cancel) - shutdownHandle, err := run(ctx) + shutdownHandle, err := run(ctx, cancel) if err != nil { shutdownHandle.Call() // nolint fmt.Fprintln(os.Stderr, err) @@ -39,7 +40,13 @@ func main() { } } -func run(ctx context.Context) (*shutdown.Handle, error) { +// run executes the main program. It starts a server.Server inside +// a goroutine and returns a configured shutdown.Handle for gracefully +// shutting down the program. +// +// If the bootstrap of the server fails, run returns a non-nil error. +// Otherwise the caller must use the shutdown.Handle to stop the server. +func run(ctx context.Context, cancel context.CancelFunc) (*shutdown.Handle, error) { configPath := flag.String("config", defaultConfigPath, "") flag.Parse() @@ -57,7 +64,7 @@ func run(ctx context.Context) (*shutdown.Handle, error) { srv := server.New(config.addr, rs) handle := shutdown.NewHandle(func() error { - if err := srv.Shutdown(ctx); err != nil { + if err := srv.Shutdown(ctx); err != nil && err != context.Canceled { return err } @@ -68,7 +75,14 @@ func run(ctx context.Context) (*shutdown.Handle, error) { return nil }) - return handle, srv.Start() + go func() { + if err := srv.Start(); err != nil && err != http.ErrServerClosed { + fmt.Fprintln(os.Stderr, err) + cancel() + } + }() + + return handle, nil } type config struct { From 66444e91c3a8bf23524287b2c521df04e7aab9d0 Mon Sep 17 00:00:00 2001 From: moreirathomas Date: Fri, 4 Mar 2022 16:07:45 +0100 Subject: [PATCH 4/5] misc: update golangci config --- .golangci.yml | 154 +++++++++++++++++++++++++------------------------- 1 file changed, 77 insertions(+), 77 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index b119c91..cef3d06 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -11,92 +11,92 @@ linters-settings: - (net/http.ResponseWriter).Write gocognit: - min-complexity: 8 + min-complexity: 15 gocritic: enabled-checks: - - appendassign - - appendcombine - - argorder - - assignop - - badcall - - badcond - - badlock - - badregexp - - boolexprsimplify - - builtinshadow - - builtinshadowdecl - - captlocal - - caseorder - - codegencomment - - commentedoutcode - - commentedoutimport - - commentformatting - - defaultcaseorder - - deferunlambda - - deprecatedcomment - - docstub - - duparg - - dupbranchbody - - dupcase - - dupimport - - dupsubexpr + - appendAssign + - appendCombine + - argOrder + - assignOp + - badCall + - badCond + - badLock + - badRegexp + - boolExprSimplify + - builtinShadow + - builtinShadowDecl + - captLocal + - caseOrder + - codegenComment + - commentedOutCode + - commentedOutImport + - commentFormatting + - defaultCaseOrder + - deferUnlambda + - deprecatedComment + - docStub + - dupArg + - dupBranchBody + - dupCase + - dupImport + - dupSubExpr - elseif - - emptyfallthrough - - emptystringtest - - equalfold - - evalorder - - exitafterdefer - - filepathjoin - - flagderef - - flagname - - hexliteral - - hugeparam - - ifelsechain - - importshadow - - indexalloc - - initclause - - mapkey - - methodexprcall - - nestingreduce - - newderef - - nilvalreturn - - octalliteral - - offby1 - - paramtypecombine - - ptrtorefparam - - rangeexprcopy - - rangevalcopy - - regexpmust - - regexppattern - - regexpsimplify + - emptyFallthrough + - emptyStringTest + - equalFold + - evalOrder + - exitAfterDefer + - filepathJoin + - flagDeref + - flagName + - hexLiteral + - hugeParam + - ifElseChain + - importShadow + - indexAlloc + - initClause + - mapKey + - methodExprCall + - nestingReduce + - newDeref + - nilValReturn + - octalLiteral + - offBy1 + - paramTypeCombine + - ptrToRefParam + - rangeExprCopy + - rangeValCopy + - regexpMust + - regexpPattern + - regexpSimplify - ruleguard - - singlecaseswitch - - sloppylen - - sloppyreassign - - sloppytypeassert - - sortslice - - sqlquery - - stringxbytes - - switchtrue - - toomanyresultschecker - - truncatecmp - - typeassertchain - - typedeffirst - - typeswitchvar - - typeunparen + - singleCaseSwitch + - sloppyLen + - sloppyReassign + - sloppyTypeAssert + - sortSlice + - sqlQuery + - stringXbytes + - switchTrue + - tooManyResultsChecker + - truncateCmp + - typeAssertChain + - typeDefFirst + - typeSwitchVar + - typeUnparen - underef - # - unamedresult - - unlabelstmt + # - unamedResult + - unlabelStmt - unlambda - - unnecessaryblock - - unnecessarydefer + - unnecessaryBlock + - unnecessaryDefer - unslice - - valswap - - weakcond + - valSwap + - weakCond # - whynolint - - wrapperfunc - - yodastyleexpr + - wrapperFunc + - yodaStyleExpr settings: hugeParam: From 064bfd7387e06a923ede7e29056084bed7dd23e8 Mon Sep 17 00:00:00 2001 From: moreirathomas Date: Fri, 4 Mar 2022 16:17:12 +0100 Subject: [PATCH 5/5] doc: add config flag usage description --- cmd/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main.go b/cmd/main.go index cffe05e..b282ec0 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -47,7 +47,7 @@ func main() { // If the bootstrap of the server fails, run returns a non-nil error. // Otherwise the caller must use the shutdown.Handle to stop the server. func run(ctx context.Context, cancel context.CancelFunc) (*shutdown.Handle, error) { - configPath := flag.String("config", defaultConfigPath, "") + configPath := flag.String("config", defaultConfigPath, "Path to the configuration file (for example --config .env)") flag.Parse()