From 3822650e1583284c4c5c17295b0bba99a2dd936b Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 18 Apr 2018 10:50:06 -0700 Subject: [PATCH] tfdiags: Diagnostics.ErrWithWarnings and .NonFatalErr There is some existing practice in the "terraform" package of returning a special error type ValidationError from EvalNode implementations in order to return warnings without halting the graph walk even though a non-nil error was returned. This is a diagnostics-flavored version of that approach, allowing us to avoid totally reworking the EvalNode concept around diagnostics and retaining the ability to return non-fatal errors. NonFatalErr is equivalent to the former terraform.ValidationError, while ErrWithWarnings is a helper that automatically treats any errors as fatal but returns NonFatalError if the diagnostics contains only warnings. --- tfdiags/diagnostics.go | 77 ++++++++++++++++++ tfdiags/diagnostics_test.go | 157 ++++++++++++++++++++++++++++++++++++ 2 files changed, 234 insertions(+) diff --git a/tfdiags/diagnostics.go b/tfdiags/diagnostics.go index 667ba8096..dc8a1b342 100644 --- a/tfdiags/diagnostics.go +++ b/tfdiags/diagnostics.go @@ -136,6 +136,42 @@ func (diags Diagnostics) Err() error { return diagnosticsAsError{diags} } +// ErrWithWarnings is similar to Err except that it will also return a non-nil +// error if the receiver contains only warnings. +// +// In the warnings-only situation, the result is guaranteed to be of dynamic +// type NonFatalError, allowing diagnostics-aware callers to type-assert +// and unwrap it, treating it as non-fatal. +// +// This should be used only in contexts where the caller is able to recognize +// and handle NonFatalError. For normal callers that expect a lack of errors +// to be signaled by nil, use just Diagnostics.Err. +func (diags Diagnostics) ErrWithWarnings() error { + if len(diags) == 0 { + return nil + } + if diags.HasErrors() { + return diags.Err() + } + return NonFatalError{diags} +} + +// NonFatalErr is similar to Err except that it always returns either nil +// (if there are no diagnostics at all) or NonFatalError. +// +// This allows diagnostics to be returned over an error return channel while +// being explicit that the diagnostics should not halt processing. +// +// This should be used only in contexts where the caller is able to recognize +// and handle NonFatalError. For normal callers that expect a lack of errors +// to be signaled by nil, use just Diagnostics.Err. +func (diags Diagnostics) NonFatalErr() error { + if len(diags) == 0 { + return nil + } + return NonFatalError{diags} +} + type diagnosticsAsError struct { Diagnostics } @@ -179,3 +215,44 @@ func (dae diagnosticsAsError) WrappedErrors() []error { } return errs } + +// NonFatalError is a special error type, returned by +// Diagnostics.ErrWithWarnings and Diagnostics.NonFatalErr, +// that indicates that the wrapped diagnostics should be treated as non-fatal. +// Callers can conditionally type-assert an error to this type in order to +// detect the non-fatal scenario and handle it in a different way. +type NonFatalError struct { + Diagnostics +} + +func (woe NonFatalError) Error() string { + diags := woe.Diagnostics + switch { + case len(diags) == 0: + // should never happen, since we don't create this wrapper if + // there are no diagnostics in the list. + return "no errors or warnings" + case len(diags) == 1: + desc := diags[0].Description() + if desc.Detail == "" { + return desc.Summary + } + return fmt.Sprintf("%s: %s", desc.Summary, desc.Detail) + default: + var ret bytes.Buffer + if diags.HasErrors() { + fmt.Fprintf(&ret, "%d problems:\n", len(diags)) + } else { + fmt.Fprintf(&ret, "%d warnings:\n", len(diags)) + } + for _, diag := range woe.Diagnostics { + desc := diag.Description() + if desc.Detail == "" { + fmt.Fprintf(&ret, "\n- %s", desc.Summary) + } else { + fmt.Fprintf(&ret, "\n- %s: %s", desc.Summary, desc.Detail) + } + } + return ret.String() + } +} diff --git a/tfdiags/diagnostics_test.go b/tfdiags/diagnostics_test.go index 709e8d507..2eab810bf 100644 --- a/tfdiags/diagnostics_test.go +++ b/tfdiags/diagnostics_test.go @@ -280,3 +280,160 @@ func TestDiagnosticsErr(t *testing.T) { } }) } + +func TestDiagnosticsErrWithWarnings(t *testing.T) { + t.Run("empty", func(t *testing.T) { + var diags Diagnostics + err := diags.ErrWithWarnings() + if err != nil { + t.Errorf("got non-nil error %#v; want nil", err) + } + }) + t.Run("warning only", func(t *testing.T) { + var diags Diagnostics + diags = diags.Append(SimpleWarning("bad")) + err := diags.ErrWithWarnings() + if err == nil { + t.Errorf("got nil error; want NonFatalError") + return + } + if _, ok := err.(NonFatalError); !ok { + t.Errorf("got %T; want NonFatalError", err) + } + }) + t.Run("one error", func(t *testing.T) { + var diags Diagnostics + diags = diags.Append(errors.New("didn't work")) + err := diags.ErrWithWarnings() + if err == nil { + t.Fatalf("got nil error %#v; want non-nil", err) + } + if got, want := err.Error(), "didn't work"; got != want { + t.Errorf("wrong error message\ngot: %s\nwant: %s", got, want) + } + }) + t.Run("two errors", func(t *testing.T) { + var diags Diagnostics + diags = diags.Append(errors.New("didn't work")) + diags = diags.Append(errors.New("didn't work either")) + err := diags.ErrWithWarnings() + if err == nil { + t.Fatalf("got nil error %#v; want non-nil", err) + } + want := strings.TrimSpace(` +2 problems: + +- didn't work +- didn't work either +`) + if got := err.Error(); got != want { + t.Errorf("wrong error message\ngot: %s\nwant: %s", got, want) + } + }) + t.Run("error and warning", func(t *testing.T) { + var diags Diagnostics + diags = diags.Append(errors.New("didn't work")) + diags = diags.Append(SimpleWarning("didn't work either")) + err := diags.ErrWithWarnings() + if err == nil { + t.Fatalf("got nil error %#v; want non-nil", err) + } + // Since this "as error" mode is just a fallback for + // non-diagnostics-aware situations like tests, we don't actually + // distinguish warnings and errors here since the point is to just + // get the messages rendered. User-facing code should be printing + // each diagnostic separately, so won't enter this codepath, + want := strings.TrimSpace(` +2 problems: + +- didn't work +- didn't work either +`) + if got := err.Error(); got != want { + t.Errorf("wrong error message\ngot: %s\nwant: %s", got, want) + } + }) +} + +func TestDiagnosticsNonFatalErr(t *testing.T) { + t.Run("empty", func(t *testing.T) { + var diags Diagnostics + err := diags.NonFatalErr() + if err != nil { + t.Errorf("got non-nil error %#v; want nil", err) + } + }) + t.Run("warning only", func(t *testing.T) { + var diags Diagnostics + diags = diags.Append(SimpleWarning("bad")) + err := diags.NonFatalErr() + if err == nil { + t.Errorf("got nil error; want NonFatalError") + return + } + if _, ok := err.(NonFatalError); !ok { + t.Errorf("got %T; want NonFatalError", err) + } + }) + t.Run("one error", func(t *testing.T) { + var diags Diagnostics + diags = diags.Append(errors.New("didn't work")) + err := diags.NonFatalErr() + if err == nil { + t.Fatalf("got nil error %#v; want non-nil", err) + } + if got, want := err.Error(), "didn't work"; got != want { + t.Errorf("wrong error message\ngot: %s\nwant: %s", got, want) + } + if _, ok := err.(NonFatalError); !ok { + t.Errorf("got %T; want NonFatalError", err) + } + }) + t.Run("two errors", func(t *testing.T) { + var diags Diagnostics + diags = diags.Append(errors.New("didn't work")) + diags = diags.Append(errors.New("didn't work either")) + err := diags.NonFatalErr() + if err == nil { + t.Fatalf("got nil error %#v; want non-nil", err) + } + want := strings.TrimSpace(` +2 problems: + +- didn't work +- didn't work either +`) + if got := err.Error(); got != want { + t.Errorf("wrong error message\ngot: %s\nwant: %s", got, want) + } + if _, ok := err.(NonFatalError); !ok { + t.Errorf("got %T; want NonFatalError", err) + } + }) + t.Run("error and warning", func(t *testing.T) { + var diags Diagnostics + diags = diags.Append(errors.New("didn't work")) + diags = diags.Append(SimpleWarning("didn't work either")) + err := diags.NonFatalErr() + if err == nil { + t.Fatalf("got nil error %#v; want non-nil", err) + } + // Since this "as error" mode is just a fallback for + // non-diagnostics-aware situations like tests, we don't actually + // distinguish warnings and errors here since the point is to just + // get the messages rendered. User-facing code should be printing + // each diagnostic separately, so won't enter this codepath, + want := strings.TrimSpace(` +2 problems: + +- didn't work +- didn't work either +`) + if got := err.Error(); got != want { + t.Errorf("wrong error message\ngot: %s\nwant: %s", got, want) + } + if _, ok := err.(NonFatalError); !ok { + t.Errorf("got %T; want NonFatalError", err) + } + }) +}