search

uber-go / fx

A dependency injection based application framework for Go.
https://uber-go.github.io/fx/
MIT License
5.65k stars 287 forks source link

Race in timeout failure report generation #815

Open sywhang opened 2 years ago

sywhang commented 2 years ago

When there is a timeout, we try to generate a timeout report by printing what timed out. There is a race in this logic which sometimes results in CI failures like this:

    --- FAIL: TestAppStart/Timeout (0.01s)
9 app_test.go:905: 
10 Error Trace: app_test.go:905
11 Error:       "OnStart hook added by  failed: context deadline exceeded" does not contain "OnStart hook added by go.uber.org/fx_test.TestAppStart.func1.1 failed: context deadline exceeded"
12 Test:        TestAppStart/Timeout
13 FAIL

We can actually repro this pretty easily by changing the timeout in TestAppStart/Timeout to 1 nanosecond:

context.WithTimeout(context.Background(), time.Nanosecond)
fx ➤ make test                                                                                                                                                                                git:master*
--- FAIL: TestAppStart (0.00s)
    --- FAIL: TestAppStart/Timeout (0.00s)
        app_test.go:905:
                Error Trace:    app_test.go:905
                Error:          "OnStart hook added by  failed: context deadline exceeded" does not contain "OnStart hook added by go.uber.org/fx_test.TestAppStart.func1.1 failed: context deadline exceeded"
                Test:           TestAppStart/Timeout
sywhang commented 2 years ago

Looks like https://github.com/uber-go/fx/pull/813 should also have addressed this issue. Closing as done.

sywhang commented 2 years ago

Actually, very short-lived processes can still run into this issue... Reopening.