Skip to content

Commit

Permalink
Add unwrap functionality to topdown.Error
Browse files Browse the repository at this point in the history
OPA currently doesn't support wrapping errors returned by builtin
functions. This change modifies topdown.Error to include a wrapped
error so that additional context about the builtin error is available.

Fixes open-policy-agent#5890

Signed-off-by: Ajith Subramanian <ajith_subramanian@trimble.com>
  • Loading branch information
ajith-sub authored and ashutosh-narkar committed Jun 8, 2023
1 parent 91f2aeb commit f09354e
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 7 deletions.
2 changes: 2 additions & 0 deletions rego/rego.go
Original file line number Diff line number Diff line change
Expand Up @@ -2647,12 +2647,14 @@ func finishFunction(name string, bctx topdown.BuiltinContext, result *ast.Term,
Code: topdown.BuiltinErr,
Message: fmt.Sprintf("%v: %v", name, e.Error()),
Location: bctx.Location,
Err: e,
}}
}
return &topdown.Error{
Code: topdown.BuiltinErr,
Message: fmt.Sprintf("%v: %v", name, err.Error()),
Location: bctx.Location,
Err: err,
}
}
if result == nil {
Expand Down
2 changes: 2 additions & 0 deletions topdown/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,14 @@ func handleBuiltinErr(name string, loc *ast.Location, err error) error {
Code: TypeErr,
Message: fmt.Sprintf("%v: %v", name, err.Error()),
Location: loc,
Err: err,
}
default:
return &Error{
Code: BuiltinErr,
Message: fmt.Sprintf("%v: %v", name, err.Error()),
Location: loc,
Err: err,
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions topdown/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type Error struct {
Code string `json:"code"`
Message string `json:"message"`
Location *ast.Location `json:"location,omitempty"`
Err error `json:"-"`
}

const (
Expand Down Expand Up @@ -90,6 +91,10 @@ func (e *Error) Error() string {
return msg
}

func (e *Error) Unwrap() error {
return e.Err
}

func functionConflictErr(loc *ast.Location) error {
return &Error{
Code: ConflictErr,
Expand Down
32 changes: 25 additions & 7 deletions topdown/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,17 @@ func TestErrorWrapping(t *testing.T) {
return errors.As(err, &topdown.Halt{})
}

builtinErr := errors.New("builtin error")
loc := location.Location{
File: "b.rego",
Col: 10,
Row: 12,
}

e0 := topdown.Error{Code: topdown.BuiltinErr,
Message: "builtin error",
Location: &location.Location{
File: "b.rego",
Col: 10,
Row: 12,
},
Message: "builtin error",
Location: &loc,
Err: builtinErr,
}

tests := []struct {
Expand Down Expand Up @@ -63,6 +67,13 @@ func TestErrorWrapping(t *testing.T) {
err: fmt.Errorf("meh: %w", &topdown.Error{Code: topdown.CancelErr}),
check: topdown.IsCancel,
},
{
note: "wrapped builtin error",
err: &e0,
check: func(err error) bool {
return errors.Is(err, builtinErr)
},
},
{
note: "matching errors, code",
err: &e0,
Expand All @@ -78,7 +89,14 @@ func TestErrorWrapping(t *testing.T) {
},
},
{
note: "matching errors, code and message and location",
note: "matching errors, code, message and location",
err: &e0,
check: func(err error) bool {
return errors.Is(err, &topdown.Error{Code: topdown.BuiltinErr, Message: "builtin error", Location: &loc})
},
},
{
note: "matching errors, code, message, location and builtin error",
err: &e0,
check: func(err error) bool {
return errors.Is(err, &e0)
Expand Down

0 comments on commit f09354e

Please sign in to comment.