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

[BUG] Panic when following getting started tutorial #238

Closed
i-fix-retarded-bugs opened this issue Dec 13, 2023 · 2 comments
Closed

[BUG] Panic when following getting started tutorial #238

i-fix-retarded-bugs opened this issue Dec 13, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@i-fix-retarded-bugs
Copy link

Observed behavior

Panic from inside the library.

Expected Behavior

No panic.

Steps to reproduce

This code panics:

package main

import (
	"context"
	"fmt"
	flagd "github.com/open-feature/go-sdk-contrib/providers/flagd/pkg"
	"github.com/open-feature/go-sdk/openfeature"
)

func main() {
	provider := flagd.NewProvider(flagd.WithHost("localhost"), flagd.WithPort(8013))
        // Super duper asynchronous provider initialization magic is starting to happen.

	err := openfeature.SetProvider(provider)
        // We start using the provider variable, but the super duper magic is still happening and the provider is not ready for usage yet!!!
	if err != nil {
		fmt.Println(err.Error())
		return
	}

	client := openfeature.NewClient("app")

	ctx := context.Background()
	evalCtx := openfeature.EvaluationContext{}
        // Is the super duper magic over yet? Of course not. Panic.
	v2Enabled, err := client.BooleanValue(ctx, "v2_enabled", true, evalCtx) // This line of code panics.
	if err != nil {
		fmt.Println(err.Error())
		return
	}

	if v2Enabled {
		fmt.Println("v2 is enabled")
	}
}

This code does not. The only difference is the added time.Sleep.

package main

import (
	"context"
	"fmt"
	flagd "github.com/open-feature/go-sdk-contrib/providers/flagd/pkg"
	"github.com/open-feature/go-sdk/openfeature"
)

func main() {
	provider := flagd.NewProvider(flagd.WithHost("localhost"), flagd.WithPort(8013))
        // Super duper asynchronous provider initialization magic is starting to happen.

       time.Sleep(time.Second*3)
       // Actually wait for the super duper asynchronous initialization to finish, as if it was actually synchronous like it's supposed to be.

	err := openfeature.SetProvider(provider)
        // We start using the provider variable, and the super duper magic is actually over and nothing bad is going to happen.
	if err != nil {
		fmt.Println(err.Error())
		return
	}

	client := openfeature.NewClient("app")

	ctx := context.Background()
	evalCtx := openfeature.EvaluationContext{}
        // Is the super duper magic over yet? Of course! We used time.Sleep to circumvent the library's buggy behaviour.
	v2Enabled, err := client.BooleanValue(ctx, "v2_enabled", true, evalCtx) // This line of code does not panic.
	if err != nil {
		fmt.Println(err.Error())
		return
	}

	if v2Enabled {
		fmt.Println("v2 is enabled") // Output: "v2 is enabled"
	}
}

The issue is github.com/open-feature/go-sdk@v1.8.0/pkg/openfeature/api.go:174.

For some reason, library/provider initialization is asynchronous, meaning one can use the library/provider without any guarantee that it is ready for usage, which goes against all common sense. The fix is to not do async initializaiton. I will link the pull request shortly.

@i-fix-retarded-bugs i-fix-retarded-bugs added bug Something isn't working Needs Triage labels Dec 13, 2023
@toddbaert
Copy link
Member

Hi @i-fix-retarded-bugs . I'm sorry for the bad experience and thanks for the detailed report!

There's a couple things happening here:

  • Firstly, our doc is not helpful in this case. Some providers are ready immediately (those that use local rulesets, for example). Others require some initialization. In the second case, we'd hope users use the READY event handlers to await the provider's readiness before evaluating flags. However, our getting started doc misses that, and our README also doesn't mention it early enough. We also could add a new method that behaves like the one in your PR, which doesn't run async. However, we can't remove this old one since people expect it to be async. Other OpenFeature SDKs have this solution (one sync one async function).

  • Even if you use a provider before it's ready, it shouldn't panic - instead the default flag value you pass should be returned. There could be a bug here with either the flagd provider, OR the SDK itself. We'll look into that.

We will investigate and correct both these things if we confirm the issue.

Lastly, though I appreciate the detailed report, and though I understand your frustration, I believe your username to be in violation of the CNCF code of conduct since it contains a term many consider a slur. Your username shows up in issues, release notes, PRs, etc, and we can't let it become part of the record. We request you change your username and remove it from any commits etc. Otherwise we will re-open a similar issue in this one's place, close this one, and block this user.

@toddbaert
Copy link
Member

Closing. See #238 (comment) and linked issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants