[BUG] Panic when following getting started tutorial

i-fix-retarded-bugs commented 9 months ago

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.

toddbaert commented 9 months ago

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 commented 9 months ago

Closing. See https://github.com/open-feature/go-sdk/issues/238#issuecomment-1854409704 and linked issues.

open-feature / go-sdk