From 0569421585efb7b014c8c8f0137388692240b4b3 Mon Sep 17 00:00:00 2001
From: ktong <kuisong.tong@gmail.com>
Date: Mon, 29 Jan 2024 09:03:55 -0800
Subject: [PATCH] add doc for panic

---
 config.go             | 41 ++++++++++++++++++++++++++++++++---------
 default.go            | 10 ++++++++--
 provider/file/file.go |  4 +++-
 provider/fs/fs.go     |  6 ++++--
 4 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/config.go b/config.go
index c83c287b..9bb3c533 100644
--- a/config.go
+++ b/config.go
@@ -10,6 +10,7 @@ import (
 	"log/slog"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/go-viper/mapstructure/v2"
 
@@ -67,10 +68,11 @@ func New(opts ...Option) *Config {
 // Each loader takes precedence over the loaders before it.
 //
 // This method can be called multiple times but it is not concurrency-safe.
+// It panics if any loader is nil.
 func (c *Config) Load(loaders ...Loader) error {
 	for i, loader := range loaders {
 		if loader == nil {
-			panic(fmt.Sprintf("nil loader at loaders[%d]", i))
+			panic(fmt.Sprintf("cannot load config from nil loader at loaders[%d]", i))
 		}
 
 		values, err := loader.Load()
@@ -101,9 +103,10 @@ func (c *Config) Load(loaders ...Loader) error {
 // WARNING: All loaders passed in Load after calling Watch do not get watched.
 //
 // It only can be called once. Call after first has no effects.
+// It panics if ctx is nil.
 func (c *Config) Watch(ctx context.Context) error { //nolint:cyclop,funlen,gocognit
 	if ctx == nil {
-		panic("nil context")
+		panic("cannot watch change with nil context")
 	}
 
 	watched := true
@@ -136,12 +139,30 @@ func (c *Config) Watch(ctx context.Context) error { //nolint:cyclop,funlen,gocog
 				c.values = values
 				slog.Info("Configuration has been updated with change.")
 
-				// TODO: detect blocking onChange and return error.
-				for _, onChange := range onChanges {
-					onChange(c)
-				}
 				if len(onChanges) > 0 {
-					slog.Info("Configuration has been applied to onChanges.")
+					func() {
+						ctx, cancel = context.WithTimeout(context.Background(), time.Minute)
+						defer cancel()
+
+						done := make(chan struct{})
+						go func() {
+							defer close(done)
+
+							for _, onChange := range onChanges {
+								onChange(c)
+							}
+						}()
+
+						select {
+						case <-done:
+							slog.InfoContext(ctx, "Configuration has been applied to onChanges.")
+						case <-ctx.Done():
+							if errors.Is(ctx.Err(), context.DeadlineExceeded) {
+								slog.WarnContext(ctx, "Configuration has not been fully applied to onChanges due to timeout."+
+									" Please check if the onChanges is blocking or takes too long to complete.")
+							}
+						}
+					}()
 				}
 
 			case <-ctx.Done():
@@ -235,12 +256,14 @@ func sub(values map[string]any, path string, delimiter string) any {
 // It requires Config.Watch has been called first.
 // The paths are case-insensitive.
 //
-// TODO: add requirement for onchange function, like non-blocking,
+// The onChange function must be non-blocking and usually completes instantly.
+// If it requires a long time to complete, it should be executed in a separate goroutine.
 //
 // This method is concurrency-safe.
+// It panics if onChange is nil.
 func (c *Config) OnChange(onChange func(*Config), paths ...string) {
 	if onChange == nil {
-		panic("nil onChange")
+		panic("cannot register nil onChange")
 	}
 
 	c.onChangesMutex.Lock()
diff --git a/default.go b/default.go
index 47301f92..ba1955fa 100644
--- a/default.go
+++ b/default.go
@@ -38,8 +38,12 @@ func Unmarshal(path string, target any) error {
 // OnChange registers a callback function that is executed
 // when the value of any given path in the default Config changes.
 // The paths are case-insensitive.
-// TODO: add requirement for onchange function, like non-blocking,
+//
+// The onChange function must be non-blocking and usually completes instantly.
+// If it requires a long time to complete, it should be executed in a separate goroutine.
+//
 // This method is concurrency-safe.
+// It panics if onChange is nil.
 func OnChange(onChange func(), paths ...string) {
 	defaultConfig.Load().OnChange(func(*Config) { onChange() }, paths...)
 }
@@ -47,9 +51,11 @@ func OnChange(onChange func(), paths ...string) {
 // SetDefault sets the given Config as the default Config.
 // After this call, the konf package's top functions (e.g. konf.Get)
 // will interact with the given Config.
+//
+// It panics if config is nil.
 func SetDefault(config *Config) {
 	if config == nil {
-		panic("nil config")
+		panic("cannot set default with nil config")
 	}
 
 	defaultConfig.Store(config)
diff --git a/provider/file/file.go b/provider/file/file.go
index bfa1f8f0..046aef22 100644
--- a/provider/file/file.go
+++ b/provider/file/file.go
@@ -30,9 +30,11 @@ type File struct {
 }
 
 // New creates a File with the given path and Option(s).
+//
+// It panics if the path is empty.
 func New(path string, opts ...Option) File {
 	if path == "" {
-		panic("empty path")
+		panic("cannot create File with empty path")
 	}
 
 	option := &options{
diff --git a/provider/fs/fs.go b/provider/fs/fs.go
index 17b5702b..5a8b5379 100644
--- a/provider/fs/fs.go
+++ b/provider/fs/fs.go
@@ -32,12 +32,14 @@ type FS struct {
 }
 
 // New creates a FS with the given fs.FS, path and Option(s).
+//
+// It panics if the fs is nil or the path is empty.
 func New(fs fs.FS, path string, opts ...Option) FS { //nolint:varnamelen
 	if fs == nil {
-		panic("nil fs")
+		panic("cannot create FS with nil fs")
 	}
 	if path == "" {
-		panic("empty path")
+		panic("cannot create FS with empty path")
 	}
 
 	option := &options{