From e523809216086b4fa4cb9f0c5a058b273bc5dbbf Mon Sep 17 00:00:00 2001
From: Danny Kopping <danny.kopping@grafana.com>
Date: Fri, 24 Nov 2023 16:20:27 +0200
Subject: [PATCH] bug(volume): return 400 error for invalid volume request
 (#11313)

**What this PR does / why we need it**:
Making a request to the `/volume{,_range}` endpoints results in a 500 is
a LogQL expression is given instead of the expected label matchers. The
reason for this is that the error is not wrapped in such a way as to be
interpreted by `ClientHTTPStatusAndError` correctly.

5xx responses from the querier are automatically retried, so any invalid
request currently results in 5 retries which are unnecessary.

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
---
 clients/pkg/logentry/logql/parser.go | 5 +++--
 pkg/logql/syntax/parser.go           | 2 +-
 pkg/logqlmodel/error.go              | 1 +
 pkg/util/server/error.go             | 2 +-
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/clients/pkg/logentry/logql/parser.go b/clients/pkg/logentry/logql/parser.go
index e5ee61227a9f..d567f6fce4c8 100644
--- a/clients/pkg/logentry/logql/parser.go
+++ b/clients/pkg/logentry/logql/parser.go
@@ -1,13 +1,14 @@
 package logql
 
 import (
-	"errors"
 	"fmt"
 	"strconv"
 	"strings"
 	"text/scanner"
 
 	"github.com/prometheus/prometheus/model/labels"
+
+	"github.com/grafana/loki/pkg/logqlmodel"
 )
 
 func init() {
@@ -44,7 +45,7 @@ func ParseMatchers(input string) ([]*labels.Matcher, error) {
 	}
 	matcherExpr, ok := expr.(*matchersExpr)
 	if !ok {
-		return nil, errors.New("only label matchers is supported")
+		return nil, logqlmodel.ErrParseMatchers
 	}
 	return matcherExpr.matchers, nil
 }
diff --git a/pkg/logql/syntax/parser.go b/pkg/logql/syntax/parser.go
index 81874ba6d6c4..710bf7132c4c 100644
--- a/pkg/logql/syntax/parser.go
+++ b/pkg/logql/syntax/parser.go
@@ -146,7 +146,7 @@ func ParseMatchers(input string, validate bool) ([]*labels.Matcher, error) {
 	}
 	matcherExpr, ok := expr.(*MatchersExpr)
 	if !ok {
-		return nil, errors.New("only label matchers is supported")
+		return nil, logqlmodel.ErrParseMatchers
 	}
 	return matcherExpr.Mts, nil
 }
diff --git a/pkg/logqlmodel/error.go b/pkg/logqlmodel/error.go
index 9491a8f3342c..68ddf72cc2f2 100644
--- a/pkg/logqlmodel/error.go
+++ b/pkg/logqlmodel/error.go
@@ -15,6 +15,7 @@ var (
 	ErrLimit           = errors.New("limit reached while evaluating the query")
 	ErrIntervalLimit   = errors.New("[interval] value exceeds limit")
 	ErrBlocked         = errors.New("query blocked by policy")
+	ErrParseMatchers   = errors.New("only label matchers are supported")
 	ErrorLabel         = "__error__"
 	PreserveErrorLabel = "__preserve_error__"
 	ErrorDetailsLabel  = "__error_details__"
diff --git a/pkg/util/server/error.go b/pkg/util/server/error.go
index ef4dedec9309..df2beaea2b0b 100644
--- a/pkg/util/server/error.go
+++ b/pkg/util/server/error.go
@@ -56,7 +56,7 @@ func ClientHTTPStatusAndError(err error) (int, error) {
 		return http.StatusGatewayTimeout, errors.New(ErrDeadlineExceeded)
 	case errors.As(err, &queryErr):
 		return http.StatusBadRequest, err
-	case errors.Is(err, logqlmodel.ErrLimit) || errors.Is(err, logqlmodel.ErrParse) || errors.Is(err, logqlmodel.ErrPipeline) || errors.Is(err, logqlmodel.ErrBlocked):
+	case errors.Is(err, logqlmodel.ErrLimit) || errors.Is(err, logqlmodel.ErrParse) || errors.Is(err, logqlmodel.ErrPipeline) || errors.Is(err, logqlmodel.ErrBlocked) || errors.Is(err, logqlmodel.ErrParseMatchers):
 		return http.StatusBadRequest, err
 	case errors.Is(err, user.ErrNoOrgID):
 		return http.StatusBadRequest, err