Skip to content

Commit

Permalink
do not panic (#184)
Browse files Browse the repository at this point in the history
return error on Load instead
  • Loading branch information
ktong authored Feb 29, 2024
1 parent b3ceed8 commit 3893083
Show file tree
Hide file tree
Showing 20 changed files with 83 additions and 212 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
### Removed
- Remove konf.Default() to disallow loading configuration into the default Config (#180).
- Remove ExplainOption from Config.Explain for always blurring sensitive information (#180).
- Remove LoadOption from Config.Load (#184).

## [0.6.3] - 2024-02-23

Expand Down
25 changes: 6 additions & 19 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
package konf

import (
"context"
"errors"
"fmt"
"log/slog"
"slices"
Expand Down Expand Up @@ -79,33 +79,20 @@ func New(opts ...Option) Config {
return Config(*option)
}

var errNilLoader = errors.New("cannot load config from nil loader")

// Load loads configuration from the given loader.
// Each loader takes precedence over the loaders before it.
//
// This method can be called multiple times but it is not concurrency-safe.
func (c Config) Load(loader Loader, opts ...LoadOption) error {
func (c Config) Load(loader Loader) error {
if loader == nil {
c.logger.Warn("cannot load config from nil loader")

return nil
}

loadOption := &loadOptions{}
for _, opt := range opts {
opt(loadOption)
return errNilLoader
}

values, err := loader.Load()
if err != nil {
if !loadOption.continueOnError {
return fmt.Errorf("load configuration: %w", err)
}
c.logger.LogAttrs(
context.Background(), slog.LevelWarn,
"failed to load configuration",
slog.Any("loader", loader),
slog.Any("error", err),
)
return fmt.Errorf("load configuration: %w", err)
}
maps.Merge(c.values.values, values)

Expand Down
9 changes: 2 additions & 7 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,16 @@ func TestConfig_Load(t *testing.T) {
testcases := []struct {
description string
loader konf.Loader
opts []konf.LoadOption
err string
}{
{
description: "error",
loader: &errorLoader{},
err: "load configuration: load error",
},
{
description: "continue on error",
loader: &errorLoader{},
opts: []konf.LoadOption{konf.ContinueOnError()},
},
{
description: "nil loader",
err: "cannot load config from nil loader",
},
}

Expand All @@ -45,7 +40,7 @@ func TestConfig_Load(t *testing.T) {
t.Parallel()

config := konf.New()
err := config.Load(testcase.loader, testcase.opts...)
err := config.Load(testcase.loader)
if testcase.err == "" {
assert.NoError(t, err)
} else {
Expand Down
3 changes: 2 additions & 1 deletion internal/credential/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package credential
import (
"fmt"
"regexp"
"unsafe"
)

func Blur(name string, value any) string {
Expand All @@ -18,7 +19,7 @@ func Blur(name string, value any) string {
case string:
formatted = v
case []byte:
formatted = string(v)
formatted = unsafe.String(unsafe.SliceData(v), len(v))
default:
formatted = fmt.Sprint(value)
}
Expand Down
15 changes: 0 additions & 15 deletions option.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,3 @@ type (
Option func(*options)
options Config
)

// ContinueOnError allows watcher continues watching configuration even Config.Load fails to load the loader.
func ContinueOnError() LoadOption {
return func(options *loadOptions) {
options.continueOnError = true
}
}

type (
// LoadOption configures Config.Load with specific options.
LoadOption func(*loadOptions)
loadOptions struct {
continueOnError bool
}
)
10 changes: 0 additions & 10 deletions provider/appconfig/appconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,6 @@ type AppConfig struct {

// New creates an AppConfig with the given application, environment, profile and Option(s).
func New(application, environment, profile string, opts ...Option) AppConfig {
if application == "" {
panic("cannot create AppConfig with empty application")
}
if environment == "" {
panic("cannot create AppConfig with empty environment")
}
if profile == "" {
panic("cannot create AppConfig with empty profile")
}

option := &options{
client: &clientProxy{
application: application,
Expand Down
47 changes: 0 additions & 47 deletions provider/appconfig/appconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,53 +23,6 @@ import (
"github.com/nil-go/konf/provider/appconfig/internal/assert"
)

func TestAppConfig_New_panic(t *testing.T) {
t.Parallel()

testcases := []struct {
description string
call func()
err string
}{
{
description: "application",
call: func() {
appconfig.New("", "env", "profile")
},
err: "cannot create AppConfig with empty application",
},
{
description: "environment",
call: func() {
appconfig.New("app", "", "profile")
},
err: "cannot create AppConfig with empty environment",
},
{
description: "profile",
call: func() {
appconfig.New("app", "env", "")
},
err: "cannot create AppConfig with empty profile",
},
}

for _, testcase := range testcases {
testcase := testcase

t.Run(testcase.description, func(t *testing.T) {
t.Parallel()

defer func() {
assert.Equal(t, testcase.err, recover().(string))
}()

testcase.call()
t.Fail()
})
}
}

func TestAppConfig_Load(t *testing.T) {
t.Parallel()

Expand Down
4 changes: 0 additions & 4 deletions provider/azappconfig/appconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ type AppConfig struct {

// New creates an AppConfig with the given endpoint and Option(s).
func New(endpoint string, opts ...Option) AppConfig {
if endpoint == "" {
panic("cannot create Azure AppConfig with empty endpoint")
}

option := &options{
client: &clientProxy{
// Place holder for the default credential.
Expand Down
12 changes: 5 additions & 7 deletions provider/azappconfig/appconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,13 @@ import (
"github.com/nil-go/konf/provider/azappconfig/internal/assert"
)

func TestFile_New_panic(t *testing.T) {
func TestAppConfig(t *testing.T) {
t.Parallel()

defer func() {
assert.Equal(t, "cannot create Azure AppConfig with empty endpoint", recover().(string))
}()

azappconfig.New("")
t.Fail()
loader := azappconfig.New("")
values, err := loader.Load()
assert.Equal(t, nil, values)
assert.EqualError(t, err, "next page of list settings: no Host in request URL")
}

func TestAppConfig_Load(t *testing.T) {
Expand Down
4 changes: 0 additions & 4 deletions provider/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ type File struct {
//
// It panics if the path is empty.
func New(path string, opts ...Option) File {
if path == "" {
panic("cannot create File with empty path")
}

option := &options{
path: path,
}
Expand Down
15 changes: 4 additions & 11 deletions provider/file/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,6 @@ import (
"github.com/nil-go/konf/provider/file/internal/assert"
)

func TestFile_New_panic(t *testing.T) {
t.Parallel()

defer func() {
assert.Equal(t, "cannot create File with empty path", recover().(string))
}()

file.New("")
t.Fail()
}

func TestFile_Load(t *testing.T) {
t.Parallel()

Expand All @@ -32,6 +21,10 @@ func TestFile_Load(t *testing.T) {
expected map[string]any
err string
}{
{
description: "empty path",
err: "read file: open : no such file or directory",
},
{
description: "file",
path: "testdata/config.json",
Expand Down
4 changes: 0 additions & 4 deletions provider/flag/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ type konf interface {
//
// It panics if the konf is nil.
func New(konf konf, opts ...Option) Flag {
if konf == nil {
panic("cannot create Flag with nil konf")
}

option := &options{
konf: konf,
}
Expand Down
32 changes: 18 additions & 14 deletions provider/flag/flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,36 @@ import (
kflag "github.com/nil-go/konf/provider/flag"
)

func TestFlag_New_panic(t *testing.T) {
t.Parallel()

defer func() {
assert.Equal(t, "cannot create Flag with nil konf", recover().(string))
}()

kflag.New(nil)
t.Fail()
}

func TestFlag_Load(t *testing.T) {
t.Parallel()

testcases := []struct {
description string
exists bool
konf konf
opts []kflag.Option
expected map[string]any
}{
{
description: "nil konf",
opts: []kflag.Option{kflag.WithPrefix("p.")},
expected: map[string]any{
"p": map[string]any{
"k": "v",
"d": ".",
},
},
},
{
description: "with flag set",
konf: konf{exists: false},
opts: []kflag.Option{kflag.WithFlagSet(set)},
expected: map[string]any{
"k": "v",
},
},
{
description: "with delimiter",
konf: konf{exists: false},
opts: []kflag.Option{
kflag.WithPrefix("p_"),
kflag.WithNameSplitter(func(s string) []string { return strings.Split(s, "_") }),
Expand All @@ -54,6 +55,7 @@ func TestFlag_Load(t *testing.T) {
},
{
description: "with nil splitter",
konf: konf{exists: false},
opts: []kflag.Option{
kflag.WithPrefix("p_"),
kflag.WithNameSplitter(func(string) []string { return nil }),
Expand All @@ -62,6 +64,7 @@ func TestFlag_Load(t *testing.T) {
},
{
description: "with empty splitter",
konf: konf{exists: false},
opts: []kflag.Option{
kflag.WithPrefix("p_"),
kflag.WithNameSplitter(func(string) []string { return []string{""} }),
Expand All @@ -70,6 +73,7 @@ func TestFlag_Load(t *testing.T) {
},
{
description: "with prefix",
konf: konf{exists: false},
opts: []kflag.Option{kflag.WithPrefix("p.")},
expected: map[string]any{
"p": map[string]any{
Expand All @@ -80,7 +84,7 @@ func TestFlag_Load(t *testing.T) {
},
{
description: "with exists",
exists: true,
konf: konf{exists: true},
opts: []kflag.Option{kflag.WithPrefix("p.")},
expected: map[string]any{
"p": map[string]any{
Expand All @@ -97,7 +101,7 @@ func TestFlag_Load(t *testing.T) {
t.Run(testcase.description, func(t *testing.T) {
t.Parallel()

values, err := kflag.New(konf{exists: testcase.exists}, testcase.opts...).Load()
values, err := kflag.New(testcase.konf, testcase.opts...).Load()
assert.NoError(t, err)
assert.Equal(t, testcase.expected, values)
})
Expand Down
1 change: 1 addition & 0 deletions provider/fs/config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"p":{"k":"v"}}
Loading

0 comments on commit 3893083

Please sign in to comment.