Skip to content

Commit

Permalink
Update Dig error types & reporting, more easily distinguish Dig error…
Browse files Browse the repository at this point in the history
…s vs. non-Dig errors (#360)

This update:
- Introduces the dig.Error interface, which is implemented by all errors returned by Dig, allowing users to use errors.Is and errors.As with Dig errors. 
- Replaces the concept of errors causing other errors (i.e. the causer interface) with Go's error wrapping conventions, so errors caused by other errors can now be unwrapped to their causing error with errors.Unwrap. 
- Updates the behavior of dig.RootCause to return the first non-dig.Error in the chain of errors passed to it, if there is one. Otherwise, it will return the first error that does not wrap another error (the bottom of the chain).
- Adds tests testing the new behavior of dig.RootCause in error_test.go
  • Loading branch information
JacobOaks authored Nov 4, 2022
1 parent 9324516 commit cbba855
Show file tree
Hide file tree
Showing 15 changed files with 389 additions and 438 deletions.
15 changes: 13 additions & 2 deletions cycle_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ package dig

import (
"bytes"
"errors"
"fmt"
"io"

"go.uber.org/dig/internal/digreflect"
)
Expand All @@ -37,6 +39,8 @@ type errCycleDetected struct {
scope *Scope
}

var _ digError = errCycleDetected{}

func (e errCycleDetected) Error() string {
// We get something like,
//
Expand All @@ -60,9 +64,16 @@ func (e errCycleDetected) Error() string {
return b.String()
}

func (e errCycleDetected) writeMessage(w io.Writer, v string) {
fmt.Fprint(w, e.Error())
}

func (e errCycleDetected) Format(w fmt.State, c rune) {
formatError(e, w, c)
}

// IsCycleDetected returns a boolean as to whether the provided error indicates
// a cycle was detected in the container graph.
func IsCycleDetected(err error) bool {
_, ok := RootCause(err).(errCycleDetected)
return ok
return errors.As(err, &errCycleDetected{})
}
9 changes: 3 additions & 6 deletions decorate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package dig

import (
"errors"
"fmt"
"reflect"

Expand Down Expand Up @@ -224,10 +223,8 @@ func (s *Scope) Decorate(decorator interface{}, opts ...DecorateOption) error {
}
for _, k := range keys {
if _, ok := s.decorators[k]; ok {
return fmt.Errorf("cannot decorate using function %v: %s already decorated",
dn.dtype,
k,
)
return newErrInvalidInput(
fmt.Sprintf("cannot decorate using function %v: %s already decorated", dn.dtype, k), nil)
}
s.decorators[k] = dn
}
Expand Down Expand Up @@ -275,7 +272,7 @@ func findResultKeys(r resultList) ([]key, error) {
keys = append(keys, key{t: innerResult.Type, name: innerResult.Name})
case resultGrouped:
if innerResult.Type.Kind() != reflect.Slice {
return nil, errors.New("decorating a value group requires decorating the entire value group, not a single value")
return nil, newErrInvalidInput("decorating a value group requires decorating the entire value group, not a single value", nil)
}
keys = append(keys, key{t: innerResult.Type.Elem(), group: innerResult.Group})
case resultObject:
Expand Down
44 changes: 23 additions & 21 deletions dig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,7 @@ func TestProvideConstructorErrors(t *testing.T) {
{
desc: "dig.Out",
constructor: func(out) io.Writer { return nil },
msg: "dig_test.out embeds a dig.Out",
msg: `dig_test.out embeds a dig.Out`,
},
{
desc: "*dig.Out",
Expand All @@ -1547,8 +1547,7 @@ func TestProvideConstructorErrors(t *testing.T) {
`cannot provide function "go.uber.org/dig_test".TestProvideConstructorErrors\S+`,
`dig_test.go:\d+`, // file:line
`bad argument 1:`,
`cannot depend on result objects:`,
tt.msg)
`cannot depend on result objects: `+tt.msg)
})
}
})
Expand All @@ -1572,8 +1571,7 @@ func TestProvideConstructorErrors(t *testing.T) {
`cannot provide function "go.uber.org/dig_test".TestProvideConstructorErrors\S+`,
`dig_test.go:\d+`, // file:line
`bad result 1:`,
"cannot specify a name for result objects:",
"dig_test.out embeds dig.Out",
"cannot specify a name for result objects: dig_test.out embeds dig.Out",
)
})

Expand Down Expand Up @@ -1601,8 +1599,7 @@ func TestProvideConstructorErrors(t *testing.T) {
`cannot provide function "go.uber.org/dig_test".TestProvideConstructorErrors\S+`,
`dig_test.go:\d+`, // file:line
`bad field "Result1" of dig_test.Result2:`,
"cannot specify a name for result objects:",
"dig_test.Result1 embeds dig.Out",
"cannot specify a name for result objects: dig_test.Result1 embeds dig.Out",
)
})
}
Expand Down Expand Up @@ -1909,8 +1906,7 @@ func TestCantProvideParameterObjects(t *testing.T) {
`cannot provide function "go.uber.org/dig_test".TestCantProvideParameterObjects\S+`,
`dig_test.go:\d+`, // file:line
"bad result 1:",
`cannot provide parameter objects:`,
`dig_test.Args embeds a dig.In`,
"cannot provide parameter objects: dig_test.Args embeds a dig.In",
)
})

Expand All @@ -1926,8 +1922,7 @@ func TestCantProvideParameterObjects(t *testing.T) {
`cannot provide function "go.uber.org/dig_test".TestCantProvideParameterObjects\S+`,
`dig_test.go:\d+`, // file:line
"bad result 1:",
`cannot provide parameter objects:`,
`\*dig_test.Args embeds a dig.In`,
`cannot provide parameter objects: \*dig_test.Args embeds a dig.In`,
)
})
}
Expand Down Expand Up @@ -2203,6 +2198,18 @@ func testProvideCycleFails(t *testing.T, dryRun bool) {
})
}

func TestProvideErrNonCycle(t *testing.T) {
c := digtest.New(t)
type A struct{}
type B struct{}
newA := func() *A { return &A{} }

c.RequireProvide(newA)
err := c.Invoke(func(*B) {})
require.Error(t, err)
assert.False(t, dig.IsCycleDetected(err))
}

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

Expand Down Expand Up @@ -2407,8 +2414,7 @@ func testProvideFailures(t *testing.T, dryRun bool) {
`cannot provide function "go.uber.org/dig_test".testProvideFailures\S+`,
`dig_test.go:\d+`, // file:line
"bad result 1:",
`cannot return a pointer to a result object, use a value instead:`,
`\*dig_test.out is a pointer to a struct that embeds dig.Out`,
`cannot return a pointer to a result object, use a value instead: \*dig_test.out is a pointer to a struct that embeds dig.Out`,
)
})

Expand All @@ -2427,8 +2433,7 @@ func testProvideFailures(t *testing.T, dryRun bool) {
`cannot provide function "go.uber.org/dig_test".testProvideFailures\S+`,
`dig_test.go:\d+`, // file:line
"bad result 1:",
`cannot build a result object by embedding \*dig.Out, embed dig.Out instead:`,
`dig_test.out embeds \*dig.Out`,
`cannot build a result object by embedding \*dig.Out, embed dig.Out instead: dig_test.out embeds \*dig.Out`,
)
})

Expand Down Expand Up @@ -2905,8 +2910,7 @@ func testInvokeFailures(t *testing.T, dryRun bool) {
require.Error(t, err)
dig.AssertErrorMatches(t, err,
"bad argument 1:",
"cannot depend on a pointer to a parameter object, use a value instead:",
`\*dig_test.in is a pointer to a struct that embeds dig.In`,
`cannot depend on a pointer to a parameter object, use a value instead: \*dig_test.in is a pointer to a struct that embeds dig.In`,
)
})

Expand All @@ -2925,8 +2929,7 @@ func testInvokeFailures(t *testing.T, dryRun bool) {
require.Error(t, err)
dig.AssertErrorMatches(t, err,
"bad argument 1:",
"cannot depend on result objects:",
"dig_test.in embeds a dig.Out",
"cannot depend on result objects: dig_test.in embeds a dig.Out",
)
})

Expand All @@ -2942,8 +2945,7 @@ func testInvokeFailures(t *testing.T, dryRun bool) {
require.Error(t, err)
dig.AssertErrorMatches(t, err,
"bad argument 1:",
`cannot build a parameter object by embedding \*dig.In, embed dig.In instead:`,
`dig_test.in embeds \*dig.In`,
`cannot build a parameter object by embedding \*dig.In, embed dig.In instead: dig_test.in embeds \*dig.In`,
)
})

Expand Down
Loading

0 comments on commit cbba855

Please sign in to comment.