`fx.Decorate` not working w/ value groups
aplr opened this issue · 3 comments
Describe the bug
When using fx.Decorate
with value groups, the decorated value (here: zap.Logger
) is not supplied to the constructors (here: NewDummyHandler
) which feed the values.
However, the decorator is fed to the other constructor in the group, NewMyService
, which has no dependents.
This issue becomes evident in the logs below. As both the NewMyService
as well as the NewDummyHandler
constructors are provided within the same module, I expect them to both be fed with the same, decorated logger.
I expect the logs to look sth like the following. I reduced the output only to the relevant fields.
{"logger":"my_service.service","msg":"name should be `my_service.service`"}
{"logger":"my_service.dummy","msg":"name should be `my_service.dummy`"}
However, this is the actual, faulty output, again only the relevant fields:
{"logger":"my_service.service","msg":"name should be `my_service.service`"}
{"logger":"dummy","msg":"name should be `my_service.dummy`"}
In the second line, I expect the logger
field to be my_service.dummy
, however, the decorated logger seems to not be fed.
I don't know what's the cause, probably it's a race (don't know if registring happens in parallel), or if the dependency resolution is faulty, or if it's even expected.
Edit: Probably a dig issue, don't know if I should cross-post the issue there.
Full logs
>
[Fx] SUPPLY *zap.Logger
[Fx] PROVIDE *main.ServiceHandler[group = "handlers"] <= main.NewDummyHandler() from module "my_service"
[Fx] PROVIDE *main.MyService <= main.NewMyService() from module "my_service"
[Fx] PROVIDE fx.Lifecycle <= go.uber.org/fx.New.func1()
[Fx] PROVIDE fx.Shutdowner <= go.uber.org/fx.(*App).shutdowner-fm()
[Fx] PROVIDE fx.DotGraph <= go.uber.org/fx.(*App).dotGraph-fm()
[Fx] DECORATE *zap.Logger <= main.main.func1() from module "my_service"
[Fx] INVOKE main.main.func2() from module "my_service"
[Fx] RUN supply: stub(*zap.Logger)
[Fx] RUN decorate: main.main.func1() from module "my_service"
[Fx] RUN provide: main.NewDummyHandler() from module "my_service"
[Fx] RUN provide: main.NewMyService() from module "my_service"
{"logger":"my_service.service","msg":"name should be `my_service.service`"}
{"logger":"dummy","msg":"name should be `my_service.dummy`"}
[Fx] RUNNING
To Reproduce
- clone the reproduction repo: https://github.com/aplr/fx_decorate_value_group_issue
- run
go run main.go
- observe the output
main.go
package main
import (
"go.uber.org/fx"
"go.uber.org/zap"
)
// MARK: -- APP
func main() {
log := zap.Must(zap.NewProduction())
app := fx.New(
fx.Supply(log),
fx.Module("my_service",
fx.Decorate(func(log *zap.Logger) *zap.Logger {
return log.Named("my_service")
}),
fx.Provide(NewDummyHandler),
fx.Provide(NewMyService),
fx.Invoke(func(srv *MyService) {
srv.Run()
}),
),
)
app.Run()
}
// MARK: -- HANDLER
type ServiceHandler struct {
Name string
Handler func() error
}
type DummyHandlerParams struct {
fx.In
Log *zap.Logger
}
type HandlerResult struct {
fx.Out
Handler *ServiceHandler `group:"handlers"`
}
func NewDummyHandler(params DummyHandlerParams) HandlerResult {
return HandlerResult{
Handler: &ServiceHandler{
Name: "dummy",
Handler: func() error {
params.Log.Named("dummy").Info("name should be `my_service.dummy`")
return nil
},
},
}
}
// MARK: -- SERVICE
type MyService struct {
log *zap.Logger
handlers []*ServiceHandler
}
type ServiceParams struct {
fx.In
Log *zap.Logger
Handlers []*ServiceHandler `group:"handlers"`
}
func NewMyService(params ServiceParams) *MyService {
return &MyService{
log: params.Log.Named("service"),
handlers: params.Handlers,
}
}
func (s *MyService) Run() error {
s.log.Info("name should be `my_service.service`")
for _, handler := range s.handlers {
if err := handler.Handler(); err != nil {
return err
}
}
return nil
}
Additional context
Using the newest versions of both dependencies.
Hey, thanks for reporting this. I've made a slightly simpler repro on the playground: https://go.dev/play/p/GtqaH27ek0N
This seems to be some weird edge case issue with the combination of value groups + decorators + modules/scoping. I'm going to continue investigating, but for now I wanted to report that, other than not using value groups or modules, one workaround I found is to add the fx.Private
option to the calls to fx.Provide
.
Will keep you updated.
I verified this change in Dig fixes your repro: uber-go/dig#393.
So next release should contain this fix, or you can pin Dig to master for now. Thanks again for reporting this!
Sure thing, thanks @JacobOaks for fixing that so quickly!