fx.Decorate not working w/ value groups

[aplr]

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

1. clone the reproduction repo: https://github.com/aplr/fx_decorate_value_group_issue
2. run go run main.go
3. 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.

Dependency graph

[JacobOaks]

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.

[JacobOaks]

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!

[aplr]

Sure thing, thanks @JacobOaks for fixing that so quickly!