fx.OnStart dependency missing, sometimes

[advdv]

Describe the bug
fx.OnStart is not able to resolve my *net.TCPListener dependency in the expected way. It does work in an unexpected way, see below for explanation.

To Reproduce
Let's consider the example below for a webserver module.

// Package webserver implements the http serving
package webserver

import (
	"context"
	"fmt"
	"net"
	"net/http"
	"net/netip"

	"go.uber.org/fx"
)

// NewListener provides a tcp connection listener for the webserver.
func NewListener() (*net.TCPListener, error) {
	ap, err := netip.ParseAddrPort("[::1]:8383")
	if err != nil {
		return nil, fmt.Errorf("failed to parse addr/port: %w", err)
	}

	ln, err := net.ListenTCP("tcp", net.TCPAddrFromAddrPort(ap))
	if err != nil {
		return nil, fmt.Errorf("failed to listen: %w", err)
	}

	return ln, nil
}

// New inits the http server.
func New(h http.Handler) *http.Server {
	return &http.Server{
		Handler: h,
	}
}

// moduleName standardizes the module name.
const moduleName = "webhandler"

// Prod dependencies.
func Prod() fx.Option {
	return fx.Module(moduleName,
		fx.Provide(fx.Annotate(NewListener)),
		fx.Provide(fx.Annotate(New,
			fx.OnStart(func(ctx context.Context, ln *net.TCPListener, s *http.Server) error {
				go s.Serve(ln) //nolint:errcheck

				return nil
			}),
			fx.OnStop(func(ctx context.Context, s *http.Server) error {
				if err := s.Shutdown(ctx); err != nil {
					return fmt.Errorf("failed to shut down: %w", err)
				}

				return nil
			}))),
	)
}

Given a test file like this:

package webserver

import (
	"context"
	"net/http"
	"testing"

	. "github.com/onsi/ginkgo/v2"
	. "github.com/onsi/gomega"
	"go.uber.org/fx"
)

func TestWebserver(t *testing.T) {
	t.Parallel()
	RegisterFailHandler(Fail)
	RunSpecs(t, "web/webserver")
}

var _ = Describe("failed to handle webserver", func() {
	BeforeEach(func(ctx context.Context) {
		app := fx.New(Prod(),
			fx.Invoke(func(s *http.Server) {}),
			fx.Supply(fx.Annotate(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
				fx.As(new(http.Handler)))))
		Expect(app.Start(ctx)).To(Succeed())
		DeferCleanup(app.Done)
	})

	It("should server http", func() {
              // ....
	})
})

Running go test will result in an error like this:

error invoking hook installer: missing dependencies for function \"reflect\".makeFuncStub (/usr/local/go/src/reflect/asm_arm64.s:29): missing type: *net.TCPListener",

But, if I change the signature of the New function, like this:

// Package webserver implements the http serving
package webserver

import (
	"context"
	"fmt"
	"net"
	"net/http"
	"net/netip"

	"go.uber.org/fx"
)

// NewListener provides a tcp connection listener for the webserver.
func NewListener() (*net.TCPListener, error) {
	ap, err := netip.ParseAddrPort("[::1]:8383")
	if err != nil {
		return nil, fmt.Errorf("failed to parse addr/port: %w", err)
	}

	ln, err := net.ListenTCP("tcp", net.TCPAddrFromAddrPort(ap))
	if err != nil {
		return nil, fmt.Errorf("failed to listen: %w", err)
	}

	return ln, nil
}

// New inits the http server. NOTE: I've added the *net.TCPListener as a dependency even though 
// we don't need it to make the *http.Server
func New(h http.Handler, _ *net.TCPListener) *http.Server {
	return &http.Server{
		Handler: h,
	}
}

// moduleName standardizes the module name.
const moduleName = "webhandler"

// Prod dependencies.
func Prod() fx.Option {
	return fx.Module(moduleName,
		fx.Provide(fx.Annotate(NewListener)),
		fx.Provide(fx.Annotate(New,
			fx.OnStart(func(ctx context.Context, ln *net.TCPListener, s *http.Server) error {
				go s.Serve(ln) //nolint:errcheck

				return nil
			}),
			fx.OnStop(func(ctx context.Context, s *http.Server) error {
				if err := s.Shutdown(ctx); err != nil {
					return fmt.Errorf("failed to shut down: %w", err)
				}

				return nil
			}))),
	)
}

It will work.

Expected behavior
I would expect the first setup above to work without missing dependency error.

Additional context
I'm pretty sure this used to work in the past since I THINK I've written this code before.

[sywhang]

Hey @advanderveer,

Hooks passed as OnStart hooks are not allowed to take dependencies outside of the provider's direct dependencies or result types.

See: https://pkg.go.dev/go.uber.org/fx#OnStart:
"The hook function passed into OnStart cannot take any arguments outside of the annotated constructor's existing dependencies or results, except a context.Context."

I'm pretty sure this used to work in the past since I THINK I've written this code before.

That was actually an unintentional effect of a bug that was fixed in #983.

[advdv]

That explains it then, thank you for the clear response. I guess a more clear error would be better (it could be detected that a requested dependency is invalid) but since it is already documented I guess this can also be closed.

[sywhang]

I guess a more clear error would be better (it could be detected that a requested dependency is invalid)

That's a valid ask. I've filed #1090 to track that.