uber-go / fx

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

Provide Fx types before user types #1191

Closed JacobOaks closed 2 months ago

JacobOaks commented 2 months ago

Consider the following example:

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/fx@v1.21.0/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 - 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)
codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.73%. Comparing base (9814dd3) to head (4626474).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1191 +/- ## ======================================= Coverage 98.73% 98.73% ======================================= Files 31 31 Lines 2851 2851 ======================================= Hits 2815 2815 Misses 29 29 Partials 7 7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

JacobOaks commented 2 months ago

Did we validate this in internal CI? I don't see a reason it should affect anything but in case.

Yes, this has been validated in internal CI @sywhang

sywhang commented 2 months ago

@JacobOaks do you want to cut a release?