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

Optional dependency becomes required if there is a provider available #1098

Open
errb opened this issue Jun 27, 2023 · 1 comment
Open

Optional dependency becomes required if there is a provider available #1098

errb opened this issue Jun 27, 2023 · 1 comment

Comments

@errb
Copy link

errb commented Jun 27, 2023

Describe the bug

In my application I have an optional dependency which I provide via fx.Provide. Due to the nature of the dependency, it can occasionally fail during initialization. In my case that's perfectly fine because the dependency is optional and the application is able to run without it.

I recently stumbled upon an issue where if I provide the aforementioned dependency, fx startup fails even though all uses of that dependency are tagged with optional:"true".

To Reproduce

package fxdeps

import (
	"context"
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
	"go.uber.org/fx"
)

type foo struct{}

type params struct {
	fx.In

	Foo *foo `optional:"true"`
}

// This test passes
func TestFxProvideOptionalGood(t *testing.T) {
	app := fx.New(
		fx.Invoke(func(p params) {
			require.Nil(t, p.Foo)
		}),
	)
	require.NoError(t, app.Start(context.Background()))
	require.NoError(t, app.Stop(context.Background()))
}

// This test fails -- note the provide function that returns an error.
func TestFxProvideOptionalBad(t *testing.T) {
	app := fx.New(
		fx.Provide(func() (*foo, error) {
			return nil, assert.AnError
		}),
		fx.Invoke(func(p params) {
			require.Nil(t, p.Foo)
		}),
	)
	require.NoError(t, app.Start(context.Background())) // fails with "could not build arguments for function"
	require.NoError(t, app.Stop(context.Background()))
}

The first test passes but the second one fails with could not build arguments for function.

Expected behavior

I would expect both of these tests to pass. The documentation claims that optional dependencies allow the application to function in a degraded state. If there is a soft dependency which can fail during initialization, it seems like it would be reasonable to support both use cases.

If there's a reason why it is implemented in this way, I think it would be useful to update the documentation and clarify this because it seems a bit counterintuitive.

Additional context

N/A

@sywhang
Copy link
Contributor

sywhang commented Jun 27, 2023

Hey @errb , thanks for raising this issue.

An optional dependency is optional from the receiver (the constructor who takes in that type as a parameter) does not error out when it does not see one; that doesn't mean that its constructors can error out.

We'll clarify this in the docs as you suggested though; I can see how it can be confusing.

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

2 participants