fx.OnStart, fx.OnStop doesn't work with named dependencies (using fx.ResultTags)

[advdv]

Describe the bug Given the examples of fx.OnStart/fx.OnStop I would expect the following code to work:

package main

import (
	"database/sql"
	"fmt"

	"github.com/jackc/pgx/v4"
	"github.com/jackc/pgx/v4/stdlib"
	"go.uber.org/fx"
	"golang.org/x/net/context"
)

func connect() *sql.DB {
	return stdlib.OpenDB(pgx.ConnConfig{ /* usual connection params */ })
}

func main() {
	fx.New(
		fx.Provide(
			fx.Annotate(connect,
				fx.OnStart(func(ctx context.Context, ro *sql.DB) error { return ro.PingContext(ctx) }),
				fx.ResultTags(`name:"ro"`)),
			fx.Annotate(connect,
				fx.OnStart(func(ctx context.Context, rw *sql.DB) error { return rw.PingContext(ctx) }),
				fx.ResultTags(`name:"rw"`)),
		),

		fx.Invoke(func(db struct {
			fx.In
			RO *sql.DB `name:"ro"`
			RW *sql.DB `name:"rw"`
		}) {
			fmt.Printf("invoke: %p %p\n", db.RO, db.RW)
		}),
	).Run()
}

will result in the following error:

[Fx] PROVIDE	*sql.DB[name = "ro"] <= fx.Annotate(main.connect(), fx.ResultTags(["name:\"ro\""])
[Fx] PROVIDE	*sql.DB[name = "rw"] <= fx.Annotate(main.connect(), fx.ResultTags(["name:\"rw\""])
[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] INVOKE		main.main.func3()
[Fx] ERROR		fx.Invoke(main.main.func3()) called from:
main.main
	/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:28
runtime.main
	/usr/local/go/src/runtime/proc.go:250
Failed: could not build arguments for function "main".main.func3
	/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:28:
failed to build *sql.DB[name="ro"]:
missing dependencies for function "main".connect
	/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:13:
missing type:
	- *sql.DB (did you mean to Provide it?)
[Fx] ERROR		Failed to start: could not build arguments for function "main".main.func3
	/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:28:
failed to build *sql.DB[name="ro"]:
missing dependencies for function "main".connect
	/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:13:
missing type:
	- *sql.DB (did you mean to Provide it?)
exit status 1

This makes me think that should provide the fx.OnStart param with more information about which *sql.DB the hooks applies to. But the following also doesn't work:

package main

import (
	"database/sql"
	"fmt"

	"github.com/jackc/pgx/v4"
	"github.com/jackc/pgx/v4/stdlib"
	"go.uber.org/fx"
	"golang.org/x/net/context"
)

func connect() *sql.DB {
	return stdlib.OpenDB(pgx.ConnConfig{ /* usual connection params */ })
}

func main() {
	fx.New(
		fx.Provide(
			fx.Annotate(connect,
				fx.OnStart(func(ctx context.Context, db struct {
					fx.In
					RO *sql.DB `name:"ro"`
				}) error {
					return db.RO.PingContext(ctx)
				}),
				fx.ResultTags(`name:"ro"`)),
			fx.Annotate(connect,
				fx.OnStart(func(ctx context.Context, db struct {
					fx.In
					RW *sql.DB `name:"rw"`
				}) error {
					return db.RW.PingContext(ctx)
				}),
				fx.ResultTags(`name:"rw"`)),
		),

		fx.Invoke(func(db struct {
			fx.In
			RO *sql.DB `name:"ro"`
			RW *sql.DB `name:"rw"`
		}) {
			fmt.Printf("invoke: %p %p\n", db.RO, db.RW)
		}),
	).Run()
}

But this doesn't work either, it complains about cycle in the dependency graph:

[Fx] PROVIDE	*sql.DB[name = "ro"] <= fx.Annotate(main.connect(), fx.ResultTags(["name:\"ro\""])
[Fx] PROVIDE	*sql.DB[name = "rw"] <= fx.Annotate(main.connect(), fx.ResultTags(["name:\"rw\""])
[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] INVOKE		main.main.func3()
[Fx] ERROR		fx.Invoke(main.main.func3()) called from:
main.main
	/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:38
runtime.main
	/usr/local/go/src/runtime/proc.go:250
Failed: cycle detected in dependency graph:
func(struct { In dig.In; Hook0 struct { In dig.In; Lifecycle fx.Lifecycle; Field1 struct { dig.In; RO *sql.DB "name:\"ro\"" } } }) struct { Out dig.Out; Field0 *sql.DB "name:\"ro\"" } provided by "main".connect (/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:13)
	depends on func(struct { In dig.In; Hook0 struct { In dig.In; Lifecycle fx.Lifecycle; Field1 struct { dig.In; RO *sql.DB "name:\"ro\"" } } }) struct { Out dig.Out; Field0 *sql.DB "name:\"ro\"" } provided by "main".connect (/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:13)
[Fx] ERROR		Failed to start: cycle detected in dependency graph:
func(struct { In dig.In; Hook0 struct { In dig.In; Lifecycle fx.Lifecycle; Field1 struct { dig.In; RO *sql.DB "name:\"ro\"" } } }) struct { Out dig.Out; Field0 *sql.DB "name:\"ro\"" } provided by "main".connect (/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:13)
	depends on func(struct { In dig.In; Hook0 struct { In dig.In; Lifecycle fx.Lifecycle; Field1 struct { dig.In; RO *sql.DB "name:\"ro\"" } } }) struct { Out dig.Out; Field0 *sql.DB "name:\"ro\"" } provided by "main".connect (/Users/adam/Documents/Projects/github.com/crewlinker/core/fxbug/main.go:13)
exit status 1

To Reproduce See the code above

Expected behavior I expect that named dependencies can also have lifecycle annotations

Additional context

[sywhang]

Thanks for reporting this issue. We'll look into this issue shortly.

Internal Ref: GO-1661.

cc @tchung1118

[advdv]

Thank you for the fix and releasing this in the latest version. I finally got around to testing it today. I'm not sure it is intended but the obvious less-verbose method described in the first code snippet does not work. But the second approach in my second snippet does work. So this can be closed as a blocker to use the annotations. But maybe another issue is appropriate to make it less verbose?

[tchung1118]

Hey, sorry I missed this. Yeah, I think making it less verbose could be a valid improvement on this (as a syntactic sugar), but meanwhile we can probably close this because those annotations do work with other annotations now.