Skip to content

Commit

Permalink
bug(volume): return 400 error for invalid volume request (#11313)
Browse files Browse the repository at this point in the history
**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>
  • Loading branch information
Danny Kopping committed Nov 24, 2023
1 parent 75cfe59 commit e523809
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 4 deletions.
5 changes: 3 additions & 2 deletions 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() {
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/logql/syntax/parser.go
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions pkg/logqlmodel/error.go
Expand Up @@ -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__"
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/server/error.go
Expand Up @@ -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
Expand Down

0 comments on commit e523809

Please sign in to comment.