From b12298cbbbcb8fd38248577f52bc283d32fcef6e Mon Sep 17 00:00:00 2001 From: Dan Carley Date: Fri, 12 Jul 2024 12:42:37 +0100 Subject: [PATCH 1/2] Pass errors down from ProjectRunner In preparation for being able to gracefully shutdown the HTTP server. The existing exit code is wrapped in a new type that satisfies `error` and lets us pull the code out for `os.Exit`. --- src/app/project_runner.go | 17 ++++++++++++++--- src/app/system_test.go | 8 ++++---- src/cmd/project_runner.go | 21 ++++++++++----------- src/cmd/root.go | 19 +++++++++++++++---- src/cmd/run.go | 3 ++- src/cmd/up.go | 3 ++- 6 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/app/project_runner.go b/src/app/project_runner.go index 919bc1a..05d8700 100644 --- a/src/app/project_runner.go +++ b/src/app/project_runner.go @@ -15,6 +15,14 @@ import ( "github.com/rs/zerolog/log" ) +type ExitError struct { + Code int +} + +func (e *ExitError) Error() string { + return fmt.Sprintf("project non-zero exit code: %d", e.Code) +} + type ProjectRunner struct { procConfMutex sync.Mutex project *types.Project @@ -47,7 +55,7 @@ func (p *ProjectRunner) init() { p.initProcessLogs() } -func (p *ProjectRunner) Run() int { +func (p *ProjectRunner) Run() error { p.runningProcesses = make(map[string]*Process) runOrder := []types.ProcessConfig{} err := p.project.WithProcesses([]string{}, func(process types.ProcessConfig) error { @@ -55,7 +63,7 @@ func (p *ProjectRunner) Run() int { return nil }) if err != nil { - log.Error().Msgf("Failed to build project run order: %s", err.Error()) + return fmt.Errorf("failed to build project run order: %e", err) } var nameOrder []string for _, v := range runOrder { @@ -75,7 +83,10 @@ func (p *ProjectRunner) Run() int { } p.waitGroup.Wait() log.Info().Msg("Project completed") - return p.exitCode + if p.exitCode != 0 { + err = &ExitError{p.exitCode} + } + return err } func (p *ProjectRunner) runProcess(config *types.ProcessConfig) { diff --git a/src/app/system_test.go b/src/app/system_test.go index e868dc8..80cde96 100644 --- a/src/app/system_test.go +++ b/src/app/system_test.go @@ -146,10 +146,10 @@ func TestSystem_TestComposeChainExit(t *testing.T) { t.Errorf(err.Error()) return } - exitCode := runner.Run() - want := 42 - if want != exitCode { - t.Errorf("Project.Run() = %v, want %v", exitCode, want) + err = runner.Run() + want := "project non-zero exit code: 42" + if want != err.Error() { + t.Errorf("Project.Run() = %v, want %v", err, want) } }) } diff --git a/src/cmd/project_runner.go b/src/cmd/project_runner.go index 6390a62..bfe3f05 100644 --- a/src/cmd/project_runner.go +++ b/src/cmd/project_runner.go @@ -38,16 +38,16 @@ func getProjectRunner(process []string, noDeps bool, mainProcess string, mainPro return runner } -func runProject(runner *app.ProjectRunner) { - exitCode := 0 +func runProject(runner *app.ProjectRunner) error { + var err error if *pcFlags.IsTuiEnabled { - exitCode = runTui(runner) + err = runTui(runner) } else { - exitCode = runHeadless(runner) + err = runHeadless(runner) } os.Remove(*pcFlags.UnixSocketPath) log.Info().Msg("Thank you for using process-compose") - os.Exit(exitCode) + return err } func setSignal(signalHandler func()) { @@ -60,23 +60,22 @@ func setSignal(signalHandler func()) { }() } -func runHeadless(project *app.ProjectRunner) int { +func runHeadless(project *app.ProjectRunner) error { setSignal(func() { _ = project.ShutDownProject() }) - exitCode := project.Run() - return exitCode + return project.Run() } -func runTui(project *app.ProjectRunner) int { +func runTui(project *app.ProjectRunner) error { go startTui(project) - exitCode := project.Run() + err := project.Run() if !*pcFlags.KeepTuiOn { tui.Stop() } else { tui.Wait() } - return exitCode + return err } func startTui(runner app.IProject) { diff --git a/src/cmd/root.go b/src/cmd/root.go index 07b0561..bb5cf4b 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -36,18 +36,18 @@ var ( pcFlags.PcThemeChanged = cmd.Flags().Changed(flagTheme) pcFlags.SortColumnChanged = cmd.Flags().Changed(flagSort) }, - RunE: run, + Run: run, } ) -func run(cmd *cobra.Command, args []string) error { +func run(cmd *cobra.Command, args []string) { defer func() { _ = logFile.Close() }() runner := getProjectRunner([]string{}, false, "", []string{}) startHttpServerIfEnabled(!*pcFlags.IsTuiEnabled, runner) - runProject(runner) - return nil + err := runProject(runner) + handleErrorAndExit(err) } // Execute adds all child commands to the root command and sets flags appropriately. @@ -137,6 +137,17 @@ func setupLogger() *os.File { return file } +// Logs and exits with a non-zero code if there are any errors. +func handleErrorAndExit(err error) { + if err != nil { + log.Error().Err(err) + if exitErr, ok := err.(*app.ExitError); ok { + os.Exit(exitErr.Code) + } + os.Exit(1) + } +} + func startHttpServerIfEnabled(useLogger bool, runner *app.ProjectRunner) { if !*pcFlags.NoServer { if *pcFlags.IsUnixSocket { diff --git a/src/cmd/run.go b/src/cmd/run.go index ebab098..b74fd32 100644 --- a/src/cmd/run.go +++ b/src/cmd/run.go @@ -40,7 +40,8 @@ Command line arguments, provided after --, are passed to the PROCESS.`, ) startHttpServerIfEnabled(false, runner) - runProject(runner) + err := runProject(runner) + handleErrorAndExit(err) }, } diff --git a/src/cmd/up.go b/src/cmd/up.go index f40ce83..8850c13 100644 --- a/src/cmd/up.go +++ b/src/cmd/up.go @@ -15,7 +15,8 @@ will start them and their dependencies only`, Run: func(cmd *cobra.Command, args []string) { runner := getProjectRunner(args, *pcFlags.NoDependencies, "", []string{}) startHttpServerIfEnabled(!*pcFlags.IsTuiEnabled, runner) - runProject(runner) + err := runProject(runner) + handleErrorAndExit(err) }, } From e8f7f1d095aa933692e2dc5c35bed7a6336a1fe5 Mon Sep 17 00:00:00 2001 From: Dan Carley Date: Fri, 12 Jul 2024 12:44:15 +0100 Subject: [PATCH 2/2] Gracefully shutdown HTTP server To prevent a race condition seen in #197: 1. On the client: a. User runs `process-compose down` b. Client makes request to `/project/stop` 2. On the server: a. `ProjectRunner.ShutDownProject()` stops all processes b. `cmd.runProject()` stops blocking c. Exits before `PcApi.ShutDownProject` has written the response 3. On the client: a. Gets `EOF` reading the response because the server has gone away I can't think of a way to reproduce this in a test, except for something convoluted like creating a custom client that introduces a delay in reading the response. It can be "manually" simulated by adding a small delay here though: diff --git a/src/api/pc_api.go b/src/api/pc_api.go index d123257..d069399 100644 --- a/src/api/pc_api.go +++ b/src/api/pc_api.go @@ -3,6 +3,7 @@ package api import ( "net/http" "strconv" + "time" "github.com/f1bonacc1/process-compose/src/app" "github.com/gin-gonic/gin" @@ -268,6 +269,7 @@ func (api *PcApi) GetProcessPorts(c *gin.Context) { // @Router /project/stop [post] func (api *PcApi) ShutDownProject(c *gin.Context) { api.project.ShutDownProject() + time.Sleep(10 * time.Millisecond) c.JSON(http.StatusOK, gin.H{"status": "stopped"}) } And using this config: processes: one: command: sleep infinity two: command: sleep infinity Before this change: bash-5.2$ go run src/main.go up --config services.yaml --tui=false & [1] 8171 bash-5.2$ go run src/main.go down 24-07-12 15:38:40.332 FTL failed to stop project error="Post \"http://localhost:8080/project/stop\": EOF" exit status 1 [1]+ Done go run src/main.go up --config services.yaml --tui=false After this change: bash-5.2$ go run src/main.go up --config services.yaml --tui=false & [1] 8432 bash-5.2$ go run src/main.go down --- src/api/server.go | 36 +++++++++++++++++++++++++++++------- src/cmd/root.go | 35 +++++++++++++++++++++++++++++------ src/cmd/run.go | 3 +-- src/cmd/up.go | 3 +-- 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/src/api/server.go b/src/api/server.go index 41de494..de32945 100644 --- a/src/api/server.go +++ b/src/api/server.go @@ -5,34 +5,56 @@ import ( "github.com/f1bonacc1/process-compose/src/app" "github.com/gin-gonic/gin" "github.com/rs/zerolog/log" + "net" + "net/http" "os" ) const EnvDebugMode = "PC_DEBUG_MODE" -func StartHttpServerWithUnixSocket(useLogger bool, unixSocket string, project app.IProject) { +func StartHttpServerWithUnixSocket(useLogger bool, unixSocket string, project app.IProject) (*http.Server, error) { router := getRouter(useLogger, project) log.Info().Msgf("start UDS http server listening %s", unixSocket) + + os.Remove(unixSocket) + server := &http.Server{ + Handler: router.Handler(), + } + + listener, err := net.Listen("unix", unixSocket) + if err != nil { + return server, err + } + go func() { - os.Remove(unixSocket) - err := router.RunUnix(unixSocket) - if err != nil { + defer listener.Close() + defer os.Remove(unixSocket) + + if err := server.Serve(listener); err != nil && err != http.ErrServerClosed { log.Fatal().Err(err).Msgf("start UDS http server on %s failed", unixSocket) } }() + + return server, nil } -func StartHttpServerWithTCP(useLogger bool, port int, project app.IProject) { +func StartHttpServerWithTCP(useLogger bool, port int, project app.IProject) (*http.Server, error) { router := getRouter(useLogger, project) endPoint := fmt.Sprintf(":%d", port) log.Info().Msgf("start http server listening %s", endPoint) + + server := &http.Server{ + Addr: endPoint, + Handler: router.Handler(), + } + go func() { - err := router.Run(endPoint) - if err != nil { + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { log.Fatal().Err(err).Msgf("start http server on %s failed", endPoint) } }() + return server, nil } func getRouter(useLogger bool, project app.IProject) *gin.Engine { diff --git a/src/cmd/root.go b/src/cmd/root.go index bb5cf4b..2e369d9 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -1,6 +1,7 @@ package cmd import ( + "context" "fmt" "github.com/f1bonacc1/process-compose/src/admitter" "github.com/f1bonacc1/process-compose/src/api" @@ -12,6 +13,7 @@ import ( "github.com/rs/zerolog/log" "github.com/spf13/cobra" "io" + "net/http" "os" "path" "runtime" @@ -45,8 +47,7 @@ func run(cmd *cobra.Command, args []string) { _ = logFile.Close() }() runner := getProjectRunner([]string{}, false, "", []string{}) - startHttpServerIfEnabled(!*pcFlags.IsTuiEnabled, runner) - err := runProject(runner) + err := waitForProjectAndServer(!*pcFlags.IsTuiEnabled, runner) handleErrorAndExit(err) } @@ -148,14 +149,36 @@ func handleErrorAndExit(err error) { } } -func startHttpServerIfEnabled(useLogger bool, runner *app.ProjectRunner) { +func startHttpServerIfEnabled(useLogger bool, runner *app.ProjectRunner) (*http.Server, error) { if !*pcFlags.NoServer { if *pcFlags.IsUnixSocket { - api.StartHttpServerWithUnixSocket(useLogger, *pcFlags.UnixSocketPath, runner) - return + return api.StartHttpServerWithUnixSocket(useLogger, *pcFlags.UnixSocketPath, runner) } - api.StartHttpServerWithTCP(useLogger, *pcFlags.PortNum, runner) + return api.StartHttpServerWithTCP(useLogger, *pcFlags.PortNum, runner) } + + return nil, nil +} + +func waitForProjectAndServer(useLogger bool, runner *app.ProjectRunner) error { + server, err := startHttpServerIfEnabled(useLogger, runner) + if err != nil { + return err + } + // Blocks until shutdown. + if err = runProject(runner); err != nil { + return err + } + if server != nil { + shutdownTimeout := 5 * time.Second + ctx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) + defer cancel() + if err := server.Shutdown(ctx); err != nil { + return err + } + } + + return nil } func getClient() *client.PcClient { diff --git a/src/cmd/run.go b/src/cmd/run.go index b74fd32..45ad707 100644 --- a/src/cmd/run.go +++ b/src/cmd/run.go @@ -39,8 +39,7 @@ Command line arguments, provided after --, are passed to the PROCESS.`, args, ) - startHttpServerIfEnabled(false, runner) - err := runProject(runner) + err := waitForProjectAndServer(!*pcFlags.IsTuiEnabled, runner) handleErrorAndExit(err) }, } diff --git a/src/cmd/up.go b/src/cmd/up.go index 8850c13..670ba59 100644 --- a/src/cmd/up.go +++ b/src/cmd/up.go @@ -14,8 +14,7 @@ If one or more process names are passed as arguments, will start them and their dependencies only`, Run: func(cmd *cobra.Command, args []string) { runner := getProjectRunner(args, *pcFlags.NoDependencies, "", []string{}) - startHttpServerIfEnabled(!*pcFlags.IsTuiEnabled, runner) - err := runProject(runner) + err := waitForProjectAndServer(!*pcFlags.IsTuiEnabled, runner) handleErrorAndExit(err) }, }