Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fx.ValidateApp fails on a valid graph when using fx.Decorate #1018

Closed
vladimir-gavrilenko opened this issue Jan 9, 2023 · 2 comments
Closed
Assignees

Comments

@vladimir-gavrilenko
Copy link

vladimir-gavrilenko commented Jan 9, 2023

Describe the bug
Functions inside fx.Decorate are invoked with nils as required parameters when calling fx.ValidateApp.

To Reproduce
If I take a snippet from the docs, and make a unit test, fx.ValidateApp will panic.

func TestDecoratorScope(t *testing.T) {
	opts := fx.Options(
		fx.Module("mymodule",
			fx.Decorate(func(log *zap.Logger) *zap.Logger {
				return log.Named("myapp")
			}),
			fx.Invoke(func(log *zap.Logger) {
				log.Info("decorated logger")
				// Output:
				// {"level": "info","logger":"myapp","msg":"decorated logger"}
			}),
		),
		fx.Invoke(func(log *zap.Logger) {
			log.Info("plain logger")
			// Output:
			// {"level": "info","msg":"plain logger"}
		}),
		fx.Provide(zap.NewProduction),
	)

	// uncomment the line below to panic
	// assert.NoError(t, fx.ValidateApp(opts))
	app := fxtest.New(t, opts)
	app.RequireStart().RequireStop()
}

Expected behavior
If an app can run, fx.ValidateApp should not cause a panic.

Additional context
n/a

@JacobOaks JacobOaks self-assigned this Jan 9, 2023
@JacobOaks
Copy link
Contributor

Thanks for reporting this. I've reproduced the panic and will look into it.

JacobOaks added a commit to JacobOaks/dig that referenced this issue Jan 9, 2023
Currently, decorators still run even when dry run is set
in the container (see Fx issue: uber-go/fx#1018)

This changes decorators to use the scope's invoker when decorating,
causing it to fill-in zero-valued results instead of actually running the function
when the dry run option is set.
JacobOaks added a commit to uber-go/dig that referenced this issue Jan 9, 2023
Currently, decorators still run even when dry run is set
in the container (see Fx issue: uber-go/fx#1018)

This changes decorators to use the scope's invoker when decorating,
causing it to fill-in zero-valued results instead of actually running the function
when the dry run option is set.
@sywhang
Copy link
Contributor

sywhang commented Jan 22, 2023

This issue was fixed in Dig. If you update your dependency to the latest version of Dig (v1.16.1), that should address the issue.

@sywhang sywhang closed this as completed Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants