helpers/mapstructure: turn panic while decoding to an error message

While there is more helpful information in a panic, this is confusing
to the user. With the amount of code using reflection, it seems better
to have clearer messages to help the user find the faulty section if
any.

common/helpers/mapstructure.go

@@ -4,6 +4,8 @@
 package helpers
 
 import (
+	"fmt"
+	"reflect"
 	"strings"
 
 	"github.com/mitchellh/mapstructure"
@@ -26,16 +28,31 @@ func GetMapStructureDecoderConfig(config interface{}, hooks ...mapstructure.Deco
 		ErrorUnused:      true,
 		WeaklyTypedInput: true,
 		MatchName:        MapStructureMatchName,
-		DecodeHook: mapstructure.ComposeDecodeHookFunc(
-			mapstructure.ComposeDecodeHookFunc(hooks...),
-			mapstructure.ComposeDecodeHookFunc(mapstructureUnmarshallerHookFuncs...),
-			mapstructure.TextUnmarshallerHookFunc(),
-			mapstructure.StringToTimeDurationHookFunc(),
-			mapstructure.StringToSliceHookFunc(","),
+		DecodeHook: ProtectedDecodeHookFunc(
+			mapstructure.ComposeDecodeHookFunc(
+				mapstructure.ComposeDecodeHookFunc(hooks...),
+				mapstructure.ComposeDecodeHookFunc(mapstructureUnmarshallerHookFuncs...),
+				mapstructure.TextUnmarshallerHookFunc(),
+				mapstructure.StringToTimeDurationHookFunc(),
+				mapstructure.StringToSliceHookFunc(","),
+			),
 		),
 	}
 }
 
+// ProtectedDecodeHookFunc wraps a DecodeHookFunc to recover and returns an error on panic.
+func ProtectedDecodeHookFunc(hook mapstructure.DecodeHookFunc) mapstructure.DecodeHookFunc {
+	return func(from, to reflect.Value) (v interface{}, err error) {
+		defer func() {
+			if r := recover(); r != nil {
+				v = nil
+				err = fmt.Errorf("internal error while parsing: %s", r)
+			}
+		}()
+		return mapstructure.DecodeHookExec(hook, from, to)
+	}
+}
+
 // MapStructureMatchName tells if map key and field names are equal.
 func MapStructureMatchName(mapKey, fieldName string) bool {
 	key := strings.ToLower(strings.ReplaceAll(mapKey, "-", ""))

common/helpers/mapstructure_test.go

@@ -3,7 +3,15 @@
 
 package helpers
 
-import "testing"
+import (
+	"errors"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+	"github.com/mitchellh/mapstructure"
+)
 
 func TestMapStructureMatchName(t *testing.T) {
 	cases := []struct {
@@ -27,3 +35,35 @@ func TestMapStructureMatchName(t *testing.T) {
 		}
 	}
 }
+
+func TestProtectedDecodeHook(t *testing.T) {
+	var configuration struct {
+		A string
+		B string
+	}
+	panicHook := func(from, to reflect.Type, data interface{}) (interface{}, error) {
+		if from.Kind() == reflect.String {
+			panic(errors.New("noooo"))
+		}
+		return data, nil
+	}
+	decoder, err := mapstructure.NewDecoder(GetMapStructureDecoderConfig(&configuration, panicHook))
+	if err != nil {
+		t.Fatalf("NewDecoder() error:\n%+v", err)
+	}
+	err = decoder.Decode(gin.H{"A": "hello", "B": "bye"})
+	if err == nil {
+		t.Fatal("Decode() did not error")
+	} else {
+		got := strings.Split(err.Error(), "\n")
+		expected := []string{
+			`2 error(s) decoding:`,
+			``,
+			`* error decoding 'A': internal error while parsing: noooo`,
+			`* error decoding 'B': internal error while parsing: noooo`,
+		}
+		if diff := Diff(got, expected); diff != "" {
+			t.Fatalf("Decode() error:\n%s", diff)
+		}
+	}
+}

console/data/docs/99-changelog.md

@@ -15,6 +15,7 @@ identified with a specific icon:
 
 - 🩹 *orchestrator*: validate configuration of other services on start
 - 🩹 *inlet*: correctly parse `inlet.snmp.communities` when it is just a string
+- 🌱 *cmd*: print a shorter message when an internal error happens when parsing configuration
 - 🌱 *inlet*: add `inlet.snmp.ports` to configure SNMP exporter ports
 
 ## 1.5.7 - 2022-08-23