Skip to content

Commit 95cbe83

Browse files
authored
Provide Fx types before user types (#1191)
Consider the following example: ```go func opts() fx.Option { return fx.Options( fx.WithLogger(func(fx.Lifecycle) fxevent.Logger { return &fxevent.ConsoleLogger{ W: os.Stdout } }), fx.Provide(func() string { return "" }), fx.Provide(func() string { return "" }), ) } func main() { fx.New(opts()).Run() } ``` The relevant issue to surface to the user is that they are double providing the same type. However, the actual error message is: ``` [Fx] ERROR Failed to start: the following errors occurred: - fx.Provide(main.opts.func3()) from: main.opts /home/user/go/src/scratch/fx_provide_order/main.go:17 main.main /home/user/go/src/scratch/fx_provide_order/main.go:22 runtime.main /opt/go/root/src/runtime/proc.go:271 Failed: cannot provide function "main".opts.func3 (/home/user/go/src/scratch/fx_provide_order/main.go:17): cannot provide string from [0]: already provided by "main".opts.func2 (/home/user/go/src/scratch/fx_provide_order/main.go:16) - could not build arguments for function "go.uber.org/fx".(*module).constructCustomLogger.func2 /home/user/go-repos/pkg/mod/go.uber.org/[email protected]/module.go:292: failed to build fxevent.Logger: missing dependencies for function "main".opts.func1 /home/user/go/src/scratch/fx_provide_order/main.go:11: missing type: - fx.Lifecycle (did you mean to Provide it?) ``` Which contains an additional error related to how the custom logger could not be built. This is because Fx will try to continue to build the custom logger in the face of DI failure, theoretically to report issues through the right channels. But after an error occurs when providing anything, [Fx refuses to provide any more types](https://github.com/uber-go/fx/blob/master/module.go#L184) - leading to a subsequent error when trying to build this custom logger that depends on the `fx.Lifecycle` type. This is a common issue that can be misleading for new engineers debugging their fx apps. I couldn't find any particular reason why user-provided provides are registered before these Fx types, so this PR switches this ordering so that custom loggers can still be built if they rely on the Fx types, which cleans up the error message: ``` [Fx] ERROR Failed to start: fx.Provide(main.opts.func3()) from: main.opts /home/user/go/src/scratch/fx_provide_order/main.go:17 main.main /home/user/go/src/scratch/fx_provide_order/main.go:22 runtime.main /opt/go/root/src/runtime/proc.go:271 Failed: cannot provide function "main".opts.func3 (/home/user/go/src/scratch/fx_provide_order/main.go:17): cannot provide string from [0]: already provided by "main".opts.func2 (/home/user/go/src/scratch/fx_provide_order/main.go:16) ```
1 parent 9814dd3 commit 95cbe83

File tree

3 files changed

+24
-14
lines changed

3 files changed

+24
-14
lines changed

app.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -480,10 +480,9 @@ func New(opts ...Option) *App {
480480
m.build(app, app.container)
481481
}
482482

483-
for _, m := range app.modules {
484-
m.provideAll()
485-
}
486-
483+
// Provide Fx types first to increase the chance a custom logger
484+
// can be successfully built in the face of unrelated DI failure.
485+
// E.g., for a custom logger that relies on the Lifecycle type.
487486
frames := fxreflect.CallerStack(0, 0) // include New in the stack for default Provides
488487
app.root.provide(provide{
489488
Target: func() Lifecycle { return app.lifecycle },
@@ -492,6 +491,10 @@ func New(opts ...Option) *App {
492491
app.root.provide(provide{Target: app.shutdowner, Stack: frames})
493492
app.root.provide(provide{Target: app.dotGraph, Stack: frames})
494493

494+
for _, m := range app.modules {
495+
m.provideAll()
496+
}
497+
495498
// Run decorators before executing any Invokes -- including the one
496499
// inside constructCustomLogger.
497500
app.err = multierr.Append(app.err, app.root.decorateAll())

app_test.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,13 @@ func TestNewApp(t *testing.T) {
113113
[]string{"Provided", "Provided", "Provided", "Provided", "LoggerInitialized", "Started"},
114114
spy.EventTypes())
115115

116-
assert.Contains(t, spy.Events()[0].(*fxevent.Provided).OutputTypeNames, "struct {}")
116+
// Fx types get provided first to increase chance of
117+
// successful custom logger build.
118+
assert.Contains(t, spy.Events()[0].(*fxevent.Provided).OutputTypeNames, "fx.Lifecycle")
119+
assert.Contains(t, spy.Events()[1].(*fxevent.Provided).OutputTypeNames, "fx.Shutdowner")
120+
assert.Contains(t, spy.Events()[2].(*fxevent.Provided).OutputTypeNames, "fx.DotGraph")
121+
// Our type should be index 3.
122+
assert.Contains(t, spy.Events()[3].(*fxevent.Provided).OutputTypeNames, "struct {}")
117123
})
118124

119125
t.Run("CircularGraphReturnsError", func(t *testing.T) {
@@ -575,7 +581,7 @@ func TestWithLogger(t *testing.T) {
575581
)
576582

577583
assert.Equal(t, []string{
578-
"Supplied", "Provided", "Provided", "Provided", "Run", "LoggerInitialized",
584+
"Provided", "Provided", "Provided", "Supplied", "Run", "LoggerInitialized",
579585
}, spy.EventTypes())
580586

581587
spy.Reset()
@@ -605,7 +611,7 @@ func TestWithLogger(t *testing.T) {
605611
"must provide constructor function, got (type *bytes.Buffer)",
606612
)
607613

608-
assert.Equal(t, []string{"Supplied", "Provided", "Run", "LoggerInitialized"}, spy.EventTypes())
614+
assert.Equal(t, []string{"Provided", "Provided", "Provided", "Supplied", "Provided", "Run", "LoggerInitialized"}, spy.EventTypes())
609615
})
610616

611617
t.Run("logger failed to build", func(t *testing.T) {
@@ -1166,8 +1172,9 @@ func TestOptions(t *testing.T) {
11661172
Provide(&bytes.Buffer{}), // error, not a constructor
11671173
WithLogger(func() fxevent.Logger { return spy }),
11681174
)
1169-
require.Equal(t, []string{"Provided", "LoggerInitialized"}, spy.EventTypes())
1170-
assert.Contains(t, spy.Events()[0].(*fxevent.Provided).Err.Error(), "must provide constructor function")
1175+
require.Equal(t, []string{"Provided", "Provided", "Provided", "Provided", "LoggerInitialized"}, spy.EventTypes())
1176+
// First 3 provides are Fx types (Lifecycle, Shutdowner, DotGraph).
1177+
assert.Contains(t, spy.Events()[3].(*fxevent.Provided).Err.Error(), "must provide constructor function")
11711178
})
11721179
}
11731180

module_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -261,15 +261,15 @@ func TestModuleSuccess(t *testing.T) {
261261
desc: "custom logger for module",
262262
giveWithLogger: fx.NopLogger,
263263
wantEvents: []string{
264-
"Supplied", "Provided", "Provided", "Provided",
264+
"Provided", "Provided", "Provided", "Supplied",
265265
"Run", "LoggerInitialized", "Invoking", "Invoked",
266266
},
267267
},
268268
{
269269
desc: "Not using a custom logger for module defaults to app logger",
270270
giveWithLogger: fx.Options(),
271271
wantEvents: []string{
272-
"Supplied", "Provided", "Provided", "Provided", "Provided", "Run",
272+
"Provided", "Provided", "Provided", "Supplied", "Provided", "Run",
273273
"LoggerInitialized", "Invoking", "Run", "Invoked", "Invoking", "Invoked",
274274
},
275275
},
@@ -660,7 +660,7 @@ func TestModuleFailures(t *testing.T) {
660660
giveAppOpts: spyAsLogger,
661661
wantErrContains: []string{"error building logger"},
662662
wantEvents: []string{
663-
"Supplied", "Provided", "Provided", "Provided", "Run",
663+
"Provided", "Provided", "Provided", "Supplied", "Run",
664664
"LoggerInitialized", "Provided", "LoggerInitialized",
665665
},
666666
},
@@ -678,7 +678,7 @@ func TestModuleFailures(t *testing.T) {
678678
giveAppOpts: spyAsLogger,
679679
wantErrContains: []string{"error building logger dependency"},
680680
wantEvents: []string{
681-
"Supplied", "Provided", "Provided", "Provided", "Run",
681+
"Provided", "Provided", "Provided", "Supplied", "Run",
682682
"LoggerInitialized", "Provided", "Provided", "Run", "LoggerInitialized",
683683
},
684684
},
@@ -690,7 +690,7 @@ func TestModuleFailures(t *testing.T) {
690690
"fx.WithLogger", "from:", "Failed",
691691
},
692692
wantEvents: []string{
693-
"Supplied", "Provided", "Provided", "Provided", "Run",
693+
"Provided", "Provided", "Provided", "Supplied", "Run",
694694
"LoggerInitialized", "Provided", "LoggerInitialized",
695695
},
696696
},

0 commit comments

Comments
 (0)