Skip to content

Commit

Permalink
Fix timeout report generation race
Browse files Browse the repository at this point in the history
When fx is run with a very short timeout, it's possible for the
timeout error message to be in a bad format "OnStart hook added by  failed"
where the name of the OnStart hook's caller hasn't been recorded
before the context times out.

This fixes the error message generation to not rely on the OnStart hook's
caller always being known to fix this race.

Fixes uber-go#815
  • Loading branch information
sywhang committed Nov 20, 2021
1 parent 88cdb34 commit 16da6a1
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
5 changes: 5 additions & 0 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,11 @@ func withTimeout(ctx context.Context, param *withTimeoutParams) error {
caller,
err,
r)
} else if caller == "" {
return fmt.Errorf("%v hook failed: %w",
param.hook,
err)

}
return fmt.Errorf("%v hook added by %v failed: %w",
param.hook,
Expand Down
18 changes: 18 additions & 0 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,24 @@ func TestAppRunTimeout(t *testing.T) {
}
}

func TestVeryShortTimeout(t *testing.T) {
type A struct{}
spy := new(fxlog.Spy)
app := New(
WithLogger(func() fxevent.Logger { return spy }),
Provide(func() *A { return &A{} }),
Invoke(func(*A) {}),
)

ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
err := app.Start(ctx)
require.Error(t, err)
// The error message should never be in the format "added by" followed by an empty string.
assert.NotContains(t, err.Error(), "OnStart hook added by failed")
assert.Contains(t, err.Error(), "context deadline exceeded")
cancel()
}

func TestAppStart(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 16da6a1

Please sign in to comment.