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 1a810d4
Show file tree
Hide file tree
Showing 2 changed files with 27 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
22 changes: 22 additions & 0 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,28 @@ 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 {
// this will timeout
time.Sleep(time.Millisecond)
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 1a810d4

Please sign in to comment.