From f0230d955696d39cfc48b3efc753839fea3553ed Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Tue, 9 Nov 2021 21:46:17 -0800 Subject: [PATCH 01/10] Add exitcodeOption to shutdowner --- app.go | 1 + shutdown.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/app.go b/app.go index 28fa3bf08..fdfacf470 100644 --- a/app.go +++ b/app.go @@ -382,6 +382,7 @@ type App struct { donesMu sync.Mutex // guards dones and shutdownSig dones []chan os.Signal shutdownSig os.Signal + exitCode int osExit func(code int) // os.Exit override; used for testing only } diff --git a/shutdown.go b/shutdown.go index d5b8488c0..1ff1d8e5c 100644 --- a/shutdown.go +++ b/shutdown.go @@ -39,6 +39,18 @@ type ShutdownOption interface { apply(*shutdowner) } +type exitCodeOption int + +func (e exitCodeOption) apply(s *shutdowner) { + s.app.exitCode = int(e) +} + +// WithExitCode allows the user to configure the exitCode upon application +// shutdown. +func WithExitCode(exitCode int) ShutdownOption { + return exitCodeOption(exitCode) +} + type shutdowner struct { app *App } @@ -49,6 +61,10 @@ type shutdowner struct { // In practice this means Shutdowner.Shutdown should not be called from an // fx.Invoke, but from a fx.Lifecycle.OnStart hook. func (s *shutdowner) Shutdown(opts ...ShutdownOption) error { + // Apply the options to shutdowner + for _, opt := range opts { + opt.apply(s) + } return s.app.broadcastSignal(_sigTERM) } From e5a682af99e029dae8f5c8cf70549cfa968e1a48 Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Tue, 9 Nov 2021 21:46:55 -0800 Subject: [PATCH 02/10] Update app.Run() to look for an exit code if one exists --- app.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app.go b/app.go index fdfacf470..768063ece 100644 --- a/app.go +++ b/app.go @@ -666,7 +666,11 @@ func (app *App) Run() { // Historically, we do not os.Exit(0) even though most applications // cede control to Fx with they call app.Run. To avoid a breaking // change, never os.Exit for success. - if code := app.run(app.Done()); code != 0 { + code := app.run(app.Done()) + if app.exitCode != 0 { + code = app.exitCode + } + if code != 0 { app.exit(code) } } From c482a9a7ff9310b1bf244120d857b82f3d930842 Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Wed, 10 Nov 2021 13:44:55 -0800 Subject: [PATCH 03/10] Add String() implementation for exitCodeOption --- shutdown.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shutdown.go b/shutdown.go index 1ff1d8e5c..b44505d6f 100644 --- a/shutdown.go +++ b/shutdown.go @@ -45,6 +45,10 @@ func (e exitCodeOption) apply(s *shutdowner) { s.app.exitCode = int(e) } +func (e exitCodeOption) String() string { + return fmt.Sprintf("s.Shutdown(%v)", int(e)) +} + // WithExitCode allows the user to configure the exitCode upon application // shutdown. func WithExitCode(exitCode int) ShutdownOption { From 85ed3ace3124f99b3056669049b72beac416abe5 Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Wed, 10 Nov 2021 13:47:50 -0800 Subject: [PATCH 04/10] Add test for shutdown with exit code when app is started with app.Run() --- shutdown_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/shutdown_test.go b/shutdown_test.go index b6af93f13..3883d1ca2 100644 --- a/shutdown_test.go +++ b/shutdown_test.go @@ -87,6 +87,37 @@ func TestShutdown(t *testing.T) { assert.NotNil(t, <-done1, "done channel 1 did not receive signal") assert.NotNil(t, <-done2, "done channel 2 did not receive signal") }) + + t.Run("shutdown app with non-zero, user-specified exit code with app.Run()", func(t *testing.T) { + t.Parallel() + + var ( + exited bool + exitCode int + ) + exit := func(code int) { + exited = true + exitCode = code + } + + var s fx.Shutdowner + expectedExitCode := 2 + shutdown := func(s fx.Shutdowner) error { + return s.Shutdown(fx.WithExitCode(expectedExitCode)) + } + + app := fxtest.New( + t, + fx.Populate(&s), + fx.WithExit(exit), + fx.Invoke(shutdown), + ) + + app.Run() + + assert.Equal(t, expectedExitCode, exitCode) + assert.True(t, exited) + }) } func TestDataRace(t *testing.T) { From b3f67f478c363994a83cbdbe3a89fad503ae848a Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Wed, 10 Nov 2021 14:18:23 -0800 Subject: [PATCH 05/10] Remove comment --- shutdown.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shutdown.go b/shutdown.go index b44505d6f..151ad59cd 100644 --- a/shutdown.go +++ b/shutdown.go @@ -34,7 +34,7 @@ type Shutdowner interface { } // ShutdownOption provides a way to configure properties of the shutdown -// process. Currently, no options have been implemented. +// process. type ShutdownOption interface { apply(*shutdowner) } From afe8f0e2126d5290d6852d00786ee3d1f42bbfcb Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Wed, 10 Nov 2021 16:21:47 -0800 Subject: [PATCH 06/10] Add DoneWithCode() for use when an app does not use app.Run() --- app.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/app.go b/app.go index 768063ece..ab49cea13 100644 --- a/app.go +++ b/app.go @@ -809,6 +809,25 @@ func (app *App) Done() <-chan os.Signal { return c } +// Shutdown captures a signal and exit code. +type Shutdown struct { + Signal os.Signal + ExitCode int +} + +// DoneWithCode returns a channel of the Shutdown struct to block on after starting +// the application. This behaves exactly as Done() does with the addition of +// capturing an application exit code, if one exists. +func (app *App) DoneWithCode() <-chan Shutdown { + c := make(chan Shutdown, 1) + // farm out a goroutine to wait for + // app.Done() channel to be populated + go func() { + c <- Shutdown{Signal: <-app.Done(), ExitCode: app.exitCode} + }() + return c +} + // StartTimeout returns the configured startup timeout. Apps default to using // DefaultTimeout, but users can configure this behavior using the // StartTimeout option. From 4b32dc66e005b294d64d69f5d1cc07c0c6558825 Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Wed, 10 Nov 2021 16:22:02 -0800 Subject: [PATCH 07/10] Add test for shutdown with DoneWithCode() --- shutdown_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/shutdown_test.go b/shutdown_test.go index 3883d1ca2..debb8c965 100644 --- a/shutdown_test.go +++ b/shutdown_test.go @@ -118,6 +118,28 @@ func TestShutdown(t *testing.T) { assert.Equal(t, expectedExitCode, exitCode) assert.True(t, exited) }) + + t.Run("shutdown app with non-zero, user-specified exit code without app.Run()", func(t *testing.T) { + t.Parallel() + + var s fx.Shutdowner + expectedExitCode := 2 + app := fxtest.New( + t, + fx.Populate(&s), + ) + + require.NoError(t, app.Start(context.Background()), "error starting app") + assert.NoError(t, s.Shutdown(fx.WithExitCode(expectedExitCode)), "error in app shutdown") + done1, done2 := app.DoneWithCode(), app.DoneWithCode() + done := app.Done() + defer app.Stop(context.Background()) + result1, result2 := <-done1, <-done2 + + assert.NotNil(t, <-done, "done channel did not receive signal") + assert.Equal(t, expectedExitCode, result1.ExitCode, "mismatched exit code") + assert.Equal(t, expectedExitCode, result2.ExitCode, "mismatched exit code") + }) } func TestDataRace(t *testing.T) { From ade54abbc6d45745b3e0fbc8fb03703ecdff6a2f Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Thu, 11 Nov 2021 00:24:30 -0800 Subject: [PATCH 08/10] Reword comment --- app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app.go b/app.go index ab49cea13..4295be626 100644 --- a/app.go +++ b/app.go @@ -817,7 +817,7 @@ type Shutdown struct { // DoneWithCode returns a channel of the Shutdown struct to block on after starting // the application. This behaves exactly as Done() does with the addition of -// capturing an application exit code, if one exists. +// capturing the application's exit code, if one is set. func (app *App) DoneWithCode() <-chan Shutdown { c := make(chan Shutdown, 1) // farm out a goroutine to wait for From 978250bad31f77f587a119100447b574fc542620 Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Thu, 11 Nov 2021 00:24:47 -0800 Subject: [PATCH 09/10] Remove redundant comment --- shutdown.go | 1 - 1 file changed, 1 deletion(-) diff --git a/shutdown.go b/shutdown.go index 151ad59cd..b00462a0c 100644 --- a/shutdown.go +++ b/shutdown.go @@ -65,7 +65,6 @@ type shutdowner struct { // In practice this means Shutdowner.Shutdown should not be called from an // fx.Invoke, but from a fx.Lifecycle.OnStart hook. func (s *shutdowner) Shutdown(opts ...ShutdownOption) error { - // Apply the options to shutdowner for _, opt := range opts { opt.apply(s) } From 5d714d6bbd3ce77ff0803657f13ac260fbd85b6e Mon Sep 17 00:00:00 2001 From: Manjari Akella Date: Thu, 11 Nov 2021 02:37:07 -0800 Subject: [PATCH 10/10] Move exit code computation to app.run() --- app.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/app.go b/app.go index 4295be626..35c6b13b0 100644 --- a/app.go +++ b/app.go @@ -666,11 +666,7 @@ func (app *App) Run() { // Historically, we do not os.Exit(0) even though most applications // cede control to Fx with they call app.Run. To avoid a breaking // change, never os.Exit for success. - code := app.run(app.Done()) - if app.exitCode != 0 { - code = app.exitCode - } - if code != 0 { + if code := app.run(app.Done()); code != 0 { app.exit(code) } } @@ -679,8 +675,13 @@ func (app *App) run(done <-chan os.Signal) (exitCode int) { startCtx, cancel := context.WithTimeout(context.Background(), app.StartTimeout()) defer cancel() + exitCode = 1 + if app.exitCode != 0 { + exitCode = app.exitCode + } + if err := app.Start(startCtx); err != nil { - return 1 + return exitCode } sig := <-done @@ -690,10 +691,10 @@ func (app *App) run(done <-chan os.Signal) (exitCode int) { defer cancel() if err := app.Stop(stopCtx); err != nil { - return 1 + return exitCode } - return 0 + return app.exitCode } // Err returns any error encountered during New's initialization. See the