SSE: Use errutil to show better error messages in prod (#71658)

- include public message
- propagate data source query errors so they are shown as well to which fixes #70026

pkg/expr/errors.go

@@ -0,0 +1,58 @@
+package expr
+
+import (
+	"errors"
+
+	"github.com/grafana/grafana/pkg/util/errutil"
+)
+
+var ConversionError = errutil.NewBase(
+	errutil.StatusBadRequest,
+	"sse.readDataError",
+).MustTemplate(
+	"[{{ .Public.refId }}] got error: {{ .Error }}",
+	errutil.WithPublic(
+		"failed to read data from from query {{ .Public.refId }}: {{ .Public.error }}",
+	),
+)
+
+func MakeConversionError(refID string, err error) error {
+	data := errutil.TemplateData{
+		// Conversion errors should only have meta information in errors
+		Public: map[string]interface{}{
+			"refId": refID,
+			"error": err.Error(),
+		},
+		Error: err,
+	}
+	return ConversionError.Build(data)
+}
+
+var QueryError = errutil.NewBase(
+	errutil.StatusBadRequest, "sse.dataQueryError").MustTemplate(
+	"failed to execute query [{{ .Public.refId }}]: {{ .Error }}",
+	errutil.WithPublic(
+		"failed to execute query [{{ .Public.refId }}]: {{ .Public.error }}",
+	))
+
+func MakeQueryError(refID, datasourceUID string, err error) error {
+	var pErr error
+	var utilErr errutil.Error
+	// See if this is grafana error, if so, grab public message
+	if errors.As(err, &utilErr) {
+		pErr = utilErr.Public()
+	} else {
+		pErr = err
+	}
+
+	data := errutil.TemplateData{
+		Public: map[string]interface{}{
+			"refId":         refID,
+			"datasourceUID": datasourceUID,
+			"error":         pErr.Error(),
+		},
+		Error: err,
+	}
+
+	return QueryError.Build(data)
+}

pkg/expr/errors_test.go

@@ -0,0 +1,20 @@
+package expr_test
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/expr"
+	"github.com/grafana/grafana/pkg/util/errutil"
+	"github.com/stretchr/testify/require"
+)
+
+func TestQueryErrorType(t *testing.T) {
+	qet := expr.QueryError
+	utilError := errutil.Error{}
+	qe := expr.MakeQueryError("A", "", fmt.Errorf("not work"))
+
+	require.True(t, errors.Is(qe, qet))
+	require.True(t, errors.As(qe, &utilError))
+}

pkg/expr/ml.go

@@ -111,10 +111,7 @@ func (m *MLNode) Execute(ctx context.Context, now time.Time, _ mathexp.Vars, s *
 	})
 
 	if err != nil {
-		return result, QueryError{
-			RefID: m.refID,
-			Err:   err,
-		}
+		return result, MakeQueryError(m.refID, "ml", err)
 	}
 
 	// data is not guaranteed to be specified. In this case simulate NoData scenario
@@ -124,11 +121,7 @@ func (m *MLNode) Execute(ctx context.Context, now time.Time, _ mathexp.Vars, s *
 
 	dataFrames, err := getResponseFrame(data, m.refID)
 	if err != nil {
-		return mathexp.Results{}, QueryError{
-			RefID:         m.refID,
-			DatasourceUID: mlPluginID,
-			Err:           err,
-		}
+		return mathexp.Results{}, MakeQueryError(m.refID, "ml", err)
 	}
 
 	// process the response the same way DSNode does. Use plugin ID as data source type. Semantically, they are the same.

pkg/expr/ml_test.go

@@ -213,7 +213,7 @@ func TestMLNodeExecute(t *testing.T) {
 		}
 
 		_, err := node.Execute(context.Background(), timeNow, nil, s)
-		require.IsType(t, err, QueryError{})
+		require.IsType(t, err, MakeQueryError("A", "", expectedError{}))
 		require.ErrorIs(t, err, cmd.Error)
 	})
 }

pkg/expr/nodes.go

@@ -27,20 +27,6 @@ var (
 	logger = log.New("expr")
 )
 
-type QueryError struct {
-	RefID         string
-	DatasourceUID string
-	Err           error
-}
-
-func (e QueryError) Error() string {
-	return fmt.Sprintf("failed to execute query %s: %s", e.RefID, e.Err)
-}
-
-func (e QueryError) Unwrap() error {
-	return e.Err
-}
-
 // baseNode includes common properties used across DPNodes.
 type baseNode struct {
 	id    int64
@@ -254,24 +240,19 @@ func (dn *DSNode) Execute(ctx context.Context, now time.Time, _ mathexp.Vars, s
 
 	resp, err := s.dataService.QueryData(ctx, req)
 	if err != nil {
-		return mathexp.Results{}, QueryError{
-			RefID:         dn.refID,
-			DatasourceUID: dn.datasource.UID,
-			Err:           err,
-		}
+		return mathexp.Results{}, MakeQueryError(dn.refID, dn.datasource.UID, err)
 	}
 
 	dataFrames, err := getResponseFrame(resp, dn.refID)
 	if err != nil {
-		return mathexp.Results{}, QueryError{
-			RefID:         dn.refID,
-			DatasourceUID: dn.datasource.UID,
-			Err:           err,
-		}
+		return mathexp.Results{}, MakeQueryError(dn.refID, dn.datasource.UID, err)
 	}
 
 	var result mathexp.Results
 	responseType, result, err = convertDataFramesToResults(ctx, dataFrames, dn.datasource.Type, s, logger)
+	if err != nil {
+		err = MakeConversionError(dn.refID, err)
+	}
 	return result, err
 }
 
@@ -492,7 +473,7 @@ func extractNumberSet(frame *data.Frame) ([]mathexp.Number, error) {
 func WideToMany(frame *data.Frame, fixSeries func(series mathexp.Series, valueField *data.Field)) ([]mathexp.Series, error) {
 	tsSchema := frame.TimeSeriesSchema()
 	if tsSchema.Type != data.TimeSeriesTypeWide {
-		return nil, fmt.Errorf("input data must be a wide series but got type %s (input refid)", tsSchema.Type)
+		return nil, fmt.Errorf("input data must be a wide series but got type %s", tsSchema.Type)
 	}
 
 	if len(tsSchema.ValueIndices) == 1 {

pkg/expr/nodes_test.go

@@ -26,28 +26,19 @@ func (e expectedError) Error() string {
 }
 
 func TestQueryError_Error(t *testing.T) {
-	e := QueryError{
-		RefID: "A",
-		Err:   errors.New("this is an error message"),
-	}
-	assert.EqualError(t, e, "failed to execute query A: this is an error message")
+	e := MakeQueryError("A", "", errors.New("this is an error message"))
+	assert.EqualError(t, e, "[sse.dataQueryError] failed to execute query [A]: this is an error message")
 }
 
 func TestQueryError_Unwrap(t *testing.T) {
 	t.Run("errors.Is", func(t *testing.T) {
 		expectedIsErr := errors.New("expected")
-		e := QueryError{
-			RefID: "A",
-			Err:   expectedIsErr,
-		}
+		e := MakeQueryError("A", "", expectedIsErr)
 		assert.True(t, errors.Is(e, expectedIsErr))
 	})
 
 	t.Run("errors.As", func(t *testing.T) {
-		e := QueryError{
-			RefID: "A",
-			Err:   expectedError{},
-		}
+		e := MakeQueryError("A", "", expectedError{})
 		var expectedAsError expectedError
 		assert.True(t, errors.As(e, &expectedAsError))
 	})

pkg/services/ngalert/state/manager_test.go

@@ -1723,10 +1723,7 @@ func TestProcessEvalResults(t *testing.T) {
 				{
 					eval.Result{
 						Instance:           data.Labels{"instance_label": "test"},
-						Error: expr.QueryError{
-							RefID: "A",
-							Err:   errors.New("this is an error"),
-						},
+						Error:              expr.MakeQueryError("A", "", errors.New("this is an error")),
 						State:              eval.Error,
 						EvaluatedAt:        evaluationTime.Add(10 * time.Second),
 						EvaluationDuration: evaluationDuration,
@@ -1750,10 +1747,7 @@ func TestProcessEvalResults(t *testing.T) {
 					},
 					Values: make(map[string]float64),
 					State:  eval.Error,
-					Error: expr.QueryError{
-						RefID: "A",
-						Err:   errors.New("this is an error"),
-					},
+					Error:  expr.MakeQueryError("A", "", errors.New("this is an error")),
 					Results: []state.Evaluation{
 						{
 							EvaluationTime:  evaluationTime,
@@ -1770,7 +1764,7 @@ func TestProcessEvalResults(t *testing.T) {
 					EndsAt:             evaluationTime.Add(10 * time.Second).Add(state.ResendDelay * 3),
 					LastEvaluationTime: evaluationTime.Add(10 * time.Second),
 					EvaluationDuration: evaluationDuration,
-					Annotations:        map[string]string{"annotation": "test", "Error": "failed to execute query A: this is an error"},
+					Annotations:        map[string]string{"annotation": "test", "Error": "[sse.dataQueryError] failed to execute query [A]: this is an error"},
 				},
 			},
 		},
@@ -1803,10 +1797,7 @@ func TestProcessEvalResults(t *testing.T) {
 				{
 					eval.Result{
 						Instance:           data.Labels{"instance_label": "test"},
-						Error: expr.QueryError{
-							RefID: "A",
-							Err:   errors.New("this is an error"),
-						},
+						Error:              expr.MakeQueryError("A", "", errors.New("this is an error")),
 						State:              eval.Error,
 						EvaluatedAt:        evaluationTime.Add(10 * time.Second),
 						EvaluationDuration: evaluationDuration,
@@ -1879,10 +1870,7 @@ func TestProcessEvalResults(t *testing.T) {
 				{
 					eval.Result{
 						Instance:           data.Labels{"instance_label": "test"},
-						Error: expr.QueryError{
-							RefID: "A",
-							Err:   errors.New("this is an error"),
-						},
+						Error:              expr.MakeQueryError("A", "", errors.New("this is an error")),
 						State:              eval.Error,
 						EvaluatedAt:        evaluationTime.Add(10 * time.Second),
 						EvaluationDuration: evaluationDuration,

pkg/services/ngalert/state/state.go

@@ -17,6 +17,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/ngalert/eval"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/screenshot"
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 type State struct {
@@ -297,10 +298,11 @@ func resultError(state *State, rule *models.AlertRule, result eval.Result, logge
 				state.Annotations["Error"] = result.Error.Error()
 				// If the evaluation failed because a query returned an error then add the Ref ID and
 				// Datasource UID as labels
-				var queryError expr.QueryError
-				if errors.As(state.Error, &queryError) {
+				var utilError errutil.Error
+				if errors.As(state.Error, &utilError) &&
+					(errors.Is(state.Error, expr.QueryError) || errors.Is(state.Error, expr.ConversionError)) {
 					for _, next := range rule.Data {
-						if next.RefID == queryError.RefID {
+						if next.RefID == utilError.PublicPayload["refId"].(string) {
 							state.Labels["ref_id"] = next.RefID
 							state.Labels["datasource_uid"] = next.DatasourceUID
 							break