From 4eadb0fec873d74b2f2322331e39fb900aad16ae Mon Sep 17 00:00:00 2001 From: Adam Simpson Date: Fri, 16 May 2025 14:41:32 -0400 Subject: [PATCH] ds-querier: handle execute errors better (#105496) * ds-querier: handle execute errors better * fix: change how GetResponseCode works to return 418 if rsp is nil 418 is a bit of an easter egg which in this case works since we don't have an rsp but we do know something went wrong, so a 200 won't work. Also changed this to return the code in the frame, not sure why we weren't. * tests: fix GetResponseCode tests * log no rsp case * bring back og error log --- pkg/apis/query/v0alpha1/query.go | 8 ++++++-- pkg/apis/query/v0alpha1/query_test.go | 8 ++++---- pkg/registry/apis/query/query.go | 26 ++++++++++++++------------ 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/pkg/apis/query/v0alpha1/query.go b/pkg/apis/query/v0alpha1/query.go index 4e56065d68d..094e96fcacc 100644 --- a/pkg/apis/query/v0alpha1/query.go +++ b/pkg/apis/query/v0alpha1/query.go @@ -31,11 +31,15 @@ type QueryDataResponse struct { // GetResponseCode return the right status code for the response by checking the responses. func GetResponseCode(rsp *backend.QueryDataResponse) int { if rsp == nil { - return http.StatusBadRequest + return http.StatusTeapot // rsp is nil, so we return a teapot } for _, res := range rsp.Responses { + if res.Error != nil && res.Status != 0 { + return int(res.Status) + } + if res.Error != nil { - return http.StatusBadRequest + return http.StatusTeapot // Status is nil but we have an error, so we return a teapot } } return http.StatusOK diff --git a/pkg/apis/query/v0alpha1/query_test.go b/pkg/apis/query/v0alpha1/query_test.go index a2aadb8c1f1..164e071ec7c 100644 --- a/pkg/apis/query/v0alpha1/query_test.go +++ b/pkg/apis/query/v0alpha1/query_test.go @@ -93,8 +93,8 @@ func TestGetResponseCode(t *testing.T) { }, })) }) - t.Run("return 400 if there is an error in the responses", func(t *testing.T) { - assert.Equal(t, 400, query.GetResponseCode(&backend.QueryDataResponse{ + t.Run("return 418 if there is an error in the responses but no status code", func(t *testing.T) { + assert.Equal(t, 418, query.GetResponseCode(&backend.QueryDataResponse{ Responses: map[string]backend.DataResponse{ "A": { Error: fmt.Errorf("some wild error"), @@ -102,8 +102,8 @@ func TestGetResponseCode(t *testing.T) { }, })) }) - t.Run("return 400 if there is a partial error", func(t *testing.T) { - assert.Equal(t, 400, query.GetResponseCode(&backend.QueryDataResponse{ + t.Run("return 418 if there is a partial error but no status code", func(t *testing.T) { + assert.Equal(t, 418, query.GetResponseCode(&backend.QueryDataResponse{ Responses: map[string]backend.DataResponse{ "A": { Error: nil, diff --git a/pkg/registry/apis/query/query.go b/pkg/registry/apis/query/query.go index 112e8bbe9cd..9c01c9266a7 100644 --- a/pkg/registry/apis/query/query.go +++ b/pkg/registry/apis/query/query.go @@ -163,21 +163,18 @@ func (r *queryREST) Connect(connectCtx context.Context, name string, _ runtime.O // Actually run the query rsp, err := b.execute(ctx, req) if err != nil { - // log unexpected errors - var k8sErr *errorsK8s.StatusError - if errors.As(err, &k8sErr) { - // we do not need to log 4xx errors as they are expected - if k8sErr.ErrStatus.Code >= 500 { - b.log.Error("hit unexpected k8s error while executing query", "err", err, "status", k8sErr.Status()) - } - b.log.Debug("sending a known k8s error to the client", "err", err, "status", k8sErr.Status()) + b.log.Error("execute error", "http code", query.GetResponseCode(rsp), "err", err) + if rsp != nil { // if we have a response, we assume the err is set in the response + responder.Object(query.GetResponseCode(rsp), &query.QueryDataResponse{ + QueryDataResponse: *rsp, + }) + return } else { + // return the error to the client, will send all non k8s errors as a k8 unexpected error b.log.Error("hit unexpected error while executing query, this will show as an unhandled k8s status error", "err", err) + responder.Error(err) + return } - - // return the error to the client, will send all non k8s errors as a k8 unexpected error - responder.Error(err) - return } responder.Object(query.GetResponseCode(rsp), &query.QueryDataResponse{ @@ -289,6 +286,11 @@ func (b *QueryAPIBuilder) handleQuerySingleDatasource(ctx context.Context, req d } } + if err != nil { + b.log.Debug("error in single datasource query", "error", err) + rsp = buildErrorResponse(err, req) + } + return rsp, err }