go-sdk icon indicating copy to clipboard operation
go-sdk copied to clipboard

fix: Prevent panic when setting non-comparable named providers

Open Arthur1 opened this issue 1 year ago • 1 comments

This PR

If we set non-comparable provider multiple times as a named provider, runtime error panic occurs.

reflect.DeepEqual() is generally used for provider-to-provider comparisons, but in one place a simple != is used for comparison between providers.

https://github.com/open-feature/go-sdk/blob/fb88c2434664cb7f4d8dd137661f1b5fd544e114/openfeature/event_executor.go#L347

This PR prevents panic by replacing the != comparison with DeepEqual.

Related Issues

  • https://github.com/open-feature/go-sdk/issues/285

How to test

reproduce the problem

go.mod:

module hoge

go 1.23.0

require github.com/open-feature/go-sdk v1.13.0

require (
	github.com/go-logr/logr v1.4.2 // indirect
	golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
)

main.go:

package main

import (
	"fmt"

	"github.com/open-feature/go-sdk/openfeature"
	"github.com/open-feature/go-sdk/openfeature/memprovider"
)

func main() {
	openfeature.SetNamedProviderAndWait("a", memprovider.NewInMemoryProvider(nil))
	openfeature.SetNamedProviderAndWait("b", memprovider.NewInMemoryProvider(nil))
	fmt.Println("succeeded.")
}

run:

$ go run ./main.go                                                                  ✘ 1
panic: runtime error: comparing uncomparable type memprovider.InMemoryProvider

goroutine 1 [running]:
github.com/open-feature/go-sdk/openfeature.(*eventExecutor).triggerEvent(0x14000130000, {{0x1028f7cb9, 0x10}, {0x1028f774f, 0xe}, {{0x1028fbf11, 0x22}, {0x0, 0x0, 0x0}, ...}}, ...)
	/Users/arthur-1/go/pkg/mod/github.com/open-feature/[email protected]/openfeature/event_executor.go:347 +0x1ec
github.com/open-feature/go-sdk/openfeature.(*evaluationAPI).initNewAndShutdownOld(0x14000130080, {0x10294dd50, 0x0}, {0x0, 0x0}, 0x54?)
	/Users/arthur-1/go/pkg/mod/github.com/open-feature/[email protected]/openfeature/openfeature_api.go:236 +0x1dc
github.com/open-feature/go-sdk/openfeature.(*evaluationAPI).SetNamedProvider(0x14000130080, {0x10291d700, 0x1}, {0x10294dd50, 0x0}, 0x0)
	/Users/arthur-1/go/pkg/mod/github.com/open-feature/[email protected]/openfeature/openfeature_api.go:76 +0x110
github.com/open-feature/go-sdk/openfeature.SetNamedProviderAndWait(...)
	/Users/arthur-1/go/pkg/mod/github.com/open-feature/[email protected]/openfeature/openfeature.go:59
main.main()
	/Users/arthur-1/Repositories/private/openfeature-sdk-panic/main.go:12 +0x88
exit status 2

Verify the fix

go.mod:

 module hoge
 
 go 1.23.0
 
 require github.com/open-feature/go-sdk v1.13.0
 
 require (
 	github.com/go-logr/logr v1.4.2 // indirect
 	golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
 )

+replace github.com/open-feature/go-sdk v1.13.0 => github.com/Arthur1/openfeature-go-sdk v0.0.0-20240924220603-27a4a711ba16

run:

$ go run ./main.go
succeeded.

Arthur1 avatar Sep 24 '24 22:09 Arthur1

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Please upload report for BASE (main@3551f3c). Learn more about missing BASE report. Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #286   +/-   ##
=======================================
  Coverage        ?   81.89%           
=======================================
  Files           ?       10           
  Lines           ?     1215           
  Branches        ?        0           
=======================================
  Hits            ?      995           
  Misses          ?      199           
  Partials        ?       21           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Sep 24 '24 22:09 codecov[bot]

Nice catch! Sorry for the delay!

toddbaert avatar Oct 18 '24 11:10 toddbaert