diff --git a/pkg/util/fxutil/provide_comp.go b/pkg/util/fxutil/provide_comp.go index 7fd95295a337d..83416ef8815e4 100644 --- a/pkg/util/fxutil/provide_comp.go +++ b/pkg/util/fxutil/provide_comp.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "reflect" + "runtime" "slices" "unicode" "unicode/utf8" @@ -63,16 +64,26 @@ func ProvideComponentConstructor(compCtorFunc interface{}) fx.Option { // type-check the input argument to the constructor ctorFuncType := reflect.TypeOf(compCtorFunc) if ctorFuncType.Kind() != reflect.Func || ctorFuncType.NumIn() > 1 || ctorFuncType.NumOut() == 0 || ctorFuncType.NumOut() > 2 { - return fx.Error(errors.New("argument must be a function with 0 or 1 arguments, and 1 or 2 return values")) + // Caller(1) is the caller of *this* function, which should be a fx.go source file. + // This info lets us show better error messages to developers + _, file, line, _ := runtime.Caller(1) + errtext := fmt.Sprintf("%s:%d: argument must be a function with 0 or 1 arguments, and 1 or 2 return values", file, line) + return fx.Error(errors.New(errtext)) } if ctorFuncType.NumIn() > 0 && ctorFuncType.In(0).Kind() != reflect.Struct { - return fx.Error(errors.New(`constructor must either take 0 arguments, or 1 "requires" struct`)) + // Once we know the Kind == reflect.Func, we can get extra info like the function's name + funcname := runtime.FuncForPC(reflect.ValueOf(compCtorFunc).Pointer()).Name() + _, file, line, _ := runtime.Caller(1) + errmsg := fmt.Sprintf(`constructor %s must either take 0 arguments, or 1 "requires" struct`, funcname) + errtext := fmt.Sprintf("%s:%d: %s", file, line, errmsg) + return fx.Error(errors.New(errtext)) } hasZeroArg := ctorFuncType.NumIn() == 0 ctorTypes, err := getConstructorTypes(ctorFuncType) if err != nil { - return fx.Error(err) + _, file, line, _ := runtime.Caller(1) + return fx.Error(fmt.Errorf("%s:%d: %s", file, line, err)) } // build reflect.Type of the constructor function that will be provided to `fx.Provide` diff --git a/pkg/util/fxutil/provide_comp_test.go b/pkg/util/fxutil/provide_comp_test.go index 32055ce007a1b..3a0fe914124c4 100644 --- a/pkg/util/fxutil/provide_comp_test.go +++ b/pkg/util/fxutil/provide_comp_test.go @@ -10,6 +10,7 @@ import ( "encoding/json" "fmt" "reflect" + "regexp" "sort" "strings" "testing" @@ -43,19 +44,19 @@ func TestValidArgumentAndReturnValue(t *testing.T) { func TestInvalidArgumentOrReturnValue(t *testing.T) { errOpt := ProvideComponentConstructor(1) - assertIsSingleError(t, errOpt, "argument must be a function with 0 or 1 arguments, and 1 or 2 return values") + assertErrorWithSourceInfo(t, errOpt, "argument must be a function with 0 or 1 arguments, and 1 or 2 return values") errOpt = ProvideComponentConstructor(func() {}) - assertIsSingleError(t, errOpt, "argument must be a function with 0 or 1 arguments, and 1 or 2 return values") + assertErrorWithSourceInfo(t, errOpt, "argument must be a function with 0 or 1 arguments, and 1 or 2 return values") errOpt = ProvideComponentConstructor(func(FirstComp) SecondComp { return &secondImpl{} }) - assertIsSingleError(t, errOpt, `constructor must either take 0 arguments, or 1 "requires" struct`) + assertErrorWithSourceInfo(t, errOpt, `must either take 0 arguments, or 1 "requires" struct`) errOpt = ProvideComponentConstructor(func() (FirstComp, SecondComp) { return &firstImpl{}, &secondImpl{} }) - assertIsSingleError(t, errOpt, "second return value must be error, got fxutil.SecondComp") + assertErrorWithSourceInfo(t, errOpt, "second return value must be error, got fxutil.SecondComp") errOpt = ProvideComponentConstructor(func(requires1, requires2) FirstComp { return &firstImpl{} }) - assertIsSingleError(t, errOpt, "argument must be a function with 0 or 1 arguments, and 1 or 2 return values") + assertErrorWithSourceInfo(t, errOpt, "argument must be a function with 0 or 1 arguments, and 1 or 2 return values") } func TestGetConstructorTypes(t *testing.T) { @@ -553,14 +554,20 @@ func assertNoCtorError(t *testing.T, arg fx.Option) { } } -func assertIsSingleError(t *testing.T, arg fx.Option, errMsg string) { +func assertErrorWithSourceInfo(t *testing.T, arg fx.Option, errMsgContained string) { t.Helper() app := fx.New(arg) err := app.Err() if err == nil { t.Fatalf("expected an error, instead got %v of type %T", arg, arg) - } else if err.Error() != errMsg { - t.Fatalf("errror mismatch, expected %v, got %v", errMsg, err.Error()) + } else if !strings.Contains(err.Error(), errMsgContained) { + t.Fatalf(`error mismatch, expected to contain "%v", got "%v"`, errMsgContained, err.Error()) + } + // Assert that the callsite shows up in the error, with source file and line number + re := regexp.MustCompile(`pkg/util/fxutil/provide_comp_test.go:\d+:`) + match := re.FindString(err.Error()) + if match == "" { + t.Fatalf(`error expected to contain source file and line number, got "%v"`, err.Error()) } }