From 210d73188d9b5fbfff12112dc8db80d05cab5b83 Mon Sep 17 00:00:00 2001 From: Sarah Zinger Date: Mon, 13 Jan 2025 14:58:13 -0500 Subject: [PATCH] Return a 400 instead of a 500 for empty query (#98841) --- pkg/tsdb/graphite/graphite.go | 9 ++++++++- pkg/tsdb/graphite/graphite_test.go | 9 +++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/pkg/tsdb/graphite/graphite.go b/pkg/tsdb/graphite/graphite.go index fa8b0166ded..e227e480d7e 100644 --- a/pkg/tsdb/graphite/graphite.go +++ b/pkg/tsdb/graphite/graphite.go @@ -124,7 +124,14 @@ func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest) logger.Warn("Found query models without targets", "models without targets", strings.Join(emptyQueries, "\n")) // If no queries had a valid target, return an error; otherwise, attempt with the targets we have if len(emptyQueries) == len(req.Queries) { - return &result, errors.New("no query target found for the alert rule") + if result.Responses == nil { + result.Responses = make(map[string]backend.DataResponse) + } + // marking this downstream error as it is a user error, but arguably this is a plugin error + // since the plugin should have frontend validation that prevents us from getting into this state + missingQueryResponse := backend.ErrDataResponseWithSource(400, backend.ErrorSourceDownstream, "no query target found for the alert rule") + result.Responses["A"] = missingQueryResponse + return &result, nil } } formData["target"] = targetList diff --git a/pkg/tsdb/graphite/graphite_test.go b/pkg/tsdb/graphite/graphite_test.go index edbc73b71b6..2bb94f71dea 100644 --- a/pkg/tsdb/graphite/graphite_test.go +++ b/pkg/tsdb/graphite/graphite_test.go @@ -125,7 +125,7 @@ func TestProcessQueries(t *testing.T) { assert.Equal(t, expectedInvalid, invalids[0]) }) - t.Run("QueryData with no valid queries returns an error", func(t *testing.T) { + t.Run("QueryData with no valid queries returns bad request response", func(t *testing.T) { queries := []backend.DataQuery{ { RefID: "A", @@ -142,11 +142,12 @@ func TestProcessQueries(t *testing.T) { } service.im = fakeInstanceManager{} - _, err := service.QueryData(context.Background(), &backend.QueryDataRequest{ + rsp, err := service.QueryData(context.Background(), &backend.QueryDataRequest{ Queries: queries, }) - assert.Error(t, err) - assert.Equal(t, err.Error(), "no query target found for the alert rule") + assert.NoError(t, err) + expectedResponse := backend.ErrDataResponseWithSource(400, backend.ErrorSourceDownstream, "no query target found for the alert rule") + assert.Equal(t, expectedResponse, rsp.Responses["A"]) }) }