From 53568ef75f4ba46c511c818eae4612c99c52aef5 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Mon, 16 Aug 2021 13:08:41 +0200 Subject: [PATCH] CLI: display error details for ErrorDetail errors in eval output Before, errors displayed through the presentation package's prettyError() method would never show their details. The expectation in the code was that any error that has details would include them in its own Error() string method. When error types return their details using an interface, such as type ErrorDetail interface { Lines() []string } and their Error() method only return a short error message, then that detail would never make it to the user in any of the non-JSON output variants. Before: $ opa eval -t wasm '1+1' { "errors": [ { "message": "engine not found", "details": "WebAssembly runtime not supported in this build.\n----------------------------------------------------------------------------------\nPlease download an OPA binary with Wasm enabled from\nhttps://www.openpolicyagent.org/docs/latest/#running-opa\nor build it yourself (with Wasm enabled).\n----------------------------------------------------------------------------------\n" } ] } $ opa -fpretty -t wasm '1+1' 1 error occurred: engine not found Now: $ opa eval -t wasm '1+1' { "errors": [ { "message": "engine not found", "details": "WebAssembly runtime not supported in this build.\n----------------------------------------------------------------------------------\nPlease download an OPA binary with Wasm enabled from\nhttps://www.openpolicyagent.org/docs/latest/#running-opa\nor build it yourself (with Wasm enabled).\n----------------------------------------------------------------------------------" } ] } $ opa eval -fpretty -t wasm '1+1' 1 error occurred: engine not found WebAssembly runtime not supported in this build. ---------------------------------------------------------------------------------- Please download an OPA binary with Wasm enabled from https://www.openpolicyagent.org/docs/latest/#running-opa or build it yourself (with Wasm enabled). ---------------------------------------------------------------------------------- This also surfaces the error details in REPL sessions: $ opa run OPA 0.32.0-dev (commit 3da95f9c-dirty, built at 2021-08-16T11:24:46Z) Run 'help' to see a list of commands and check for updates. > target wasm > true 1 error occurred: engine not found WebAssembly runtime not supported in this build. ---------------------------------------------------------------------------------- Please download an OPA binary with Wasm enabled from https://www.openpolicyagent.org/docs/latest/#running-opa or build it yourself (with Wasm enabled). ---------------------------------------------------------------------------------- > Signed-off-by: Stephan Renatus --- internal/presentation/presentation.go | 15 +++++++++++---- internal/presentation/presentation_test.go | 10 +++------- internal/rego/opa/engine.go | 17 +++++++++-------- rego/errors.go | 6 +++--- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/internal/presentation/presentation.go b/internal/presentation/presentation.go index 2f3536917f..fbe1149a01 100644 --- a/internal/presentation/presentation.go +++ b/internal/presentation/presentation.go @@ -198,8 +198,9 @@ func NewOutputErrors(err error) []OutputError { Message: err.Error(), err: typedErr, }} - if d, ok := err.(rego.ErrorWithDetails); ok { - errs[0].Details = d.Details() + if d, ok := err.(rego.ErrorDetails); ok { + details := strings.Join(d.Lines(), "\n") + errs[0].Details = details } } } @@ -215,16 +216,22 @@ func (e OutputErrors) Error() string { return "no error(s)" } + var prefix string if len(e) == 1 { - return fmt.Sprintf("1 error occurred: %v", e[0].Error()) + prefix = "1 error occurred: " + } else { + prefix = fmt.Sprintf("%d errors occurred:\n", len(e)) } var s []string for _, err := range e { s = append(s, err.Error()) + if l, ok := err.Details.(string); ok { + s = append(s, l) + } } - return fmt.Sprintf("%d errors occurred:\n%s", len(e), strings.Join(s, "\n")) + return prefix + strings.Join(s, "\n") } // OutputError provides a common structure for all OPA diff --git a/internal/presentation/presentation_test.go b/internal/presentation/presentation_test.go index 686a59a66c..614376d31b 100644 --- a/internal/presentation/presentation_test.go +++ b/internal/presentation/presentation_test.go @@ -38,12 +38,8 @@ func (t *testErrorWithMarshaller) MarshalJSON() ([]byte, error) { type testErrorWithDetails struct{} -func (*testErrorWithDetails) Error() string { return "something went wrong" } -func (*testErrorWithDetails) Details() string { - return `oh -so -wrong` -} +func (*testErrorWithDetails) Error() string { return "something went wrong" } +func (*testErrorWithDetails) Lines() []string { return []string{"oh", "so", "wrong"} } func validateJSONOutput(t *testing.T, testErr error, expected string) { t.Helper() @@ -487,7 +483,7 @@ func TestRaw(t *testing.T) { output: Output{ Errors: NewOutputErrors(&testErrorWithDetails{}), }, - want: "1 error occurred: something went wrong\n", + want: "1 error occurred: something went wrong\noh\nso\nwrong\n", }, } diff --git a/internal/rego/opa/engine.go b/internal/rego/opa/engine.go index 872f6e1793..36ee844504 100644 --- a/internal/rego/opa/engine.go +++ b/internal/rego/opa/engine.go @@ -15,14 +15,15 @@ var ErrEngineNotFound error = &errEngineNotFound{} type errEngineNotFound struct{} func (*errEngineNotFound) Error() string { return "engine not found" } -func (*errEngineNotFound) Details() string { - return `WebAssembly runtime not supported in this build. ----------------------------------------------------------------------------------- -Please download an OPA binary with Wasm enabled from -https://www.openpolicyagent.org/docs/latest/#running-opa -or build it yourself (with Wasm enabled). ----------------------------------------------------------------------------------- -` +func (*errEngineNotFound) Lines() []string { + return []string{ + `WebAssembly runtime not supported in this build.`, + `----------------------------------------------------------------------------------`, + `Please download an OPA binary with Wasm enabled from`, + `https://www.openpolicyagent.org/docs/latest/#running-opa`, + `or build it yourself (with Wasm enabled).`, + `----------------------------------------------------------------------------------`, + } } // Engine repesents a factory for instances of EvalEngine implementations diff --git a/rego/errors.go b/rego/errors.go index 8e83cb5706..dcc5e2679d 100644 --- a/rego/errors.go +++ b/rego/errors.go @@ -17,8 +17,8 @@ func NewHaltError(err error) error { return &HaltError{err: err} } -// ErrorWithDetails interface is satisfied by an error that provides further +// ErrorDetails interface is satisfied by an error that provides further // details. -type ErrorWithDetails interface { - Details() string +type ErrorDetails interface { + Lines() []string }