From 1a810d4365fdcf73227c5a813f25035957318adf Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Fri, 19 Nov 2021 21:39:11 -0800 Subject: [PATCH] Fix timeout report generation race 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 #815 --- app.go | 5 +++++ app_test.go | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/app.go b/app.go index da3ca2307..2868ab7ff 100644 --- a/app.go +++ b/app.go @@ -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, diff --git a/app_test.go b/app_test.go index 53f814d37..fe01a150b 100644 --- a/app_test.go +++ b/app_test.go @@ -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()