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

@@ -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

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,

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
 }