Skip to content

Commit

Permalink
App.Run: Respect Shutdowner ExitCode (#1075)
Browse files Browse the repository at this point in the history
We added support for changing the exit code for a Shutdowner with the
ExitCode option in #989, but this was somehow not respected by App.Run.

This changes App.Run to use the same underlying machinery (`Wait()`) to
decide on the exit code to use.

To test this, we add a new internal/e2e submodule that will hold full,
end-to-end integration tests.
These can be full Fx applications that we run tests against.
This is a submodule so that it can have dependencies that are not
desirable as direct dependencies of Fx,
and it's inside the internal/ directory so that it can consume
Fx-internal packages (like testutil).

The included regression test verifies the behavior described in #1074.
An Fx program using App.Run, and shut down with Shutdowner.Shutdown
and an explicit exit code, does not exit with the requested exit code.
Failure before the fix:

```
% go test
--- FAIL: TestShutdownExitCode (0.01s)
    writer.go:40: [Fx] PROVIDE  fx.Lifecycle <= go.uber.org/fx.New.func1()
    writer.go:40: [Fx] PROVIDE  fx.Shutdowner <= go.uber.org/fx.(*App).shutdowner-fm()
    writer.go:40: [Fx] PROVIDE  fx.DotGraph <= go.uber.org/fx.(*App).dotGraph-fm()
    writer.go:40: [Fx] INVOKE           go.uber.org/fx/internal/e2e/shutdowner_run_exitcode.main.func1()
    writer.go:40: [Fx] RUNNING
    writer.go:40: [Fx] TERMINATED
    main_test.go:46:
                Error Trace:    [..]/fx/internal/e2e/shutdowner_run_exitcode/main_test.go:46
                Error:          An error is expected but got nil.
                Test:           TestShutdownExitCode
FAIL
exit status 1
FAIL    go.uber.org/fx/internal/e2e/shutdowner_run_exitcode     0.016s
```

Resolves #1074

---

There's a follow up to this: abhinav#1.
It depends on the e2e test machinery, so I'll make a PR out of it once
this is merged.
  • Loading branch information
abhinav authored Apr 29, 2023
1 parent 590809b commit 8f5597f
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 8 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GO_FILES = $(shell \
find . '(' -path '*/.*' -o -path './vendor' -o -path '*/testdata/*' ')' -prune \
-o -name '*.go' -print | cut -b3-)

MODULES = . ./tools ./docs
MODULES = . ./tools ./docs ./internal/e2e

# 'make cover' should not run on docs by default.
# We run that separately explicitly on a specific platform.
Expand Down
9 changes: 5 additions & 4 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,12 +574,12 @@ 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 {
if code := app.run(app.Wait()); code != 0 {
app.exit(code)
}
}

func (app *App) run(done <-chan os.Signal) (exitCode int) {
func (app *App) run(done <-chan ShutdownSignal) (exitCode int) {
startCtx, cancel := app.clock.WithTimeout(context.Background(), app.StartTimeout())
defer cancel()

Expand All @@ -588,7 +588,8 @@ func (app *App) run(done <-chan os.Signal) (exitCode int) {
}

sig := <-done
app.log().LogEvent(&fxevent.Stopping{Signal: sig})
app.log().LogEvent(&fxevent.Stopping{Signal: sig.Signal})
exitCode = sig.ExitCode

stopCtx, cancel := app.clock.WithTimeout(context.Background(), app.StopTimeout())
defer cancel()
Expand All @@ -597,7 +598,7 @@ func (app *App) run(done <-chan os.Signal) (exitCode int) {
return 1
}

return 0
return exitCode
}

// Err returns any error encountered during New's initialization. See the
Expand Down
5 changes: 2 additions & 3 deletions app_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ package fx
import (
"errors"
"fmt"
"os"
"sync"
"testing"

Expand All @@ -42,7 +41,7 @@ func TestAppRun(t *testing.T) {
app := New(
WithLogger(func() fxevent.Logger { return spy }),
)
done := make(chan os.Signal)
done := make(chan ShutdownSignal)

var wg sync.WaitGroup
wg.Add(1)
Expand All @@ -51,7 +50,7 @@ func TestAppRun(t *testing.T) {
app.run(done)
}()

done <- _sigINT
done <- ShutdownSignal{Signal: _sigINT}
wg.Wait()

assert.Equal(t, []string{
Expand Down
6 changes: 6 additions & 0 deletions internal/e2e/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
This directory holds end-to-end tests for Fx.
Each subdirectory holds a complete Fx application
and a test for it.

This is marked as a separate Go module to prevent this code from being bundled
with the Fx library and allow for dependencies that don't leak into Fx.
21 changes: 21 additions & 0 deletions internal/e2e/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
module go.uber.org/fx/internal/e2e

go 1.20

require (
github.com/stretchr/testify v1.8.2
go.uber.org/fx v1.19.2
)

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
go.uber.org/atomic v1.7.0 // indirect
go.uber.org/dig v1.16.1 // indirect
go.uber.org/multierr v1.6.0 // indirect
go.uber.org/zap v1.23.0 // indirect
golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace go.uber.org/fx => ../..
31 changes: 31 additions & 0 deletions internal/e2e/go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=
github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw=
go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
go.uber.org/dig v1.16.1 h1:+alNIBsl0qfY0j6epRubp/9obgtrObRAc5aD+6jbWY8=
go.uber.org/dig v1.16.1/go.mod h1:557JTAUZT5bUK0SvCwikmLPPtdQhfvLYtO5tJgQSbnk=
go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI=
go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4=
go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY=
go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY=
golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0=
golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
34 changes: 34 additions & 0 deletions internal/e2e/shutdowner_run_exitcode/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright (c) 2023 Uber Technologies, Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package main

import (
"go.uber.org/fx"
)

func main() {
fx.New(
fx.Invoke(func(shutdowner fx.Shutdowner) error {
shutdowner.Shutdown(fx.ExitCode(20))
return nil
}),
).Run()
}
72 changes: 72 additions & 0 deletions internal/e2e/shutdowner_run_exitcode/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright (c) 2023 Uber Technologies, Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package main

import (
"os"
"os/exec"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/fx/internal/testutil"
)

// Hijacks the test binary so that the test can run main() as a subprocess
// instead of trying to compile the program and run it directly.
func TestMain(m *testing.M) {
// If the test binary is named "app", then we're running as a subprocess.
// Otherwise, run the tests.
switch filepath.Base(os.Args[0]) {
case "app":
main()
os.Exit(0)
default:
os.Exit(m.Run())
}
}

// Verifies that an Fx program running with Run
// exits with the exit code passed to Shutdowner.
//
// Regression test for https:/uber-go/fx/issues/1074.
func TestShutdownExitCode(t *testing.T) {
exe, err := os.Executable()
require.NoError(t, err)

out := testutil.WriteSyncer{T: t}

// Run the test binary with the name 'app' so that it runs main().
cmd := exec.Command(exe)
cmd.Args[0] = "app"
cmd.Stdout = &out
cmd.Stderr = &out

// The program should exit with code 20.
err = cmd.Run()
require.Error(t, err)

var exitErr *exec.ExitError
require.ErrorAs(t, err, &exitErr)

assert.Equal(t, 20, exitErr.ExitCode())
}

0 comments on commit 8f5597f

Please sign in to comment.