Skip to content
This repository was archived by the owner on Aug 23, 2023. It is now read-only.

Commit df69121

Browse files
DieterbeDieterbe
authored
Merge pull request #1520 from grafana/expr-bad-request
Expr package: return proper errors upon bad request from user
2 parents 6254433 + a2c1af0 commit df69121

19 files changed

+109
-55
lines changed

‎api/graphite.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1358,7 +1358,7 @@ func (s *Server) metaTagRecordUpsert(ctx *middleware.Context, upsertRequest mode
13581358

13591359
record, err := tagquery.ParseMetaTagRecord(upsertRequest.MetaTags, upsertRequest.Expressions)
13601360
if err != nil {
1361-
response.Write(ctx, response.WrapError(err))
1361+
response.Write(ctx, response.NewError(http.StatusBadRequest, err.Error()))
13621362
return
13631363
}
13641364

@@ -1382,7 +1382,7 @@ func (s *Server) metaTagRecordSwap(ctx *middleware.Context, swapRequest models.M
13821382
for i, rawRecord := range swapRequest.Records {
13831383
metaTagRecords[i], err = tagquery.ParseMetaTagRecord(rawRecord.MetaTags, rawRecord.Expressions)
13841384
if err != nil {
1385-
response.Write(ctx, response.WrapError(fmt.Errorf("Error when parsing record %d: %s", i, err)))
1385+
response.Write(ctx, response.Errorf(http.StatusBadRequest, "Error when parsing record %d: %s", i, err))
13861386
return
13871387
}
13881388
}

‎api/response/error.go

+8
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package response
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"net/http"
67
"runtime/debug"
78

@@ -69,6 +70,13 @@ func NewError(code int, err string) *ErrorResp {
6970
}
7071
}
7172

73+
func Errorf(code int, format string, a ...interface{}) *ErrorResp {
74+
return &ErrorResp{
75+
code: code,
76+
err: fmt.Sprintf(format, a...),
77+
}
78+
}
79+
7280
func (r *ErrorResp) Error() string {
7381
return r.err
7482
}

‎errors/errors.go

+12-1
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
package errors
22

3-
import "net/http"
3+
import (
4+
"fmt"
5+
"net/http"
6+
)
47

58
type Internal string
69

710
func NewInternal(err string) Internal {
811
return Internal(err)
912
}
1013

14+
func NewInternalf(format string, a ...interface{}) Internal {
15+
return Internal(fmt.Sprintf(format, a...))
16+
}
17+
1118
func (i Internal) Code() int {
1219
return http.StatusInternalServerError
1320
}
@@ -22,6 +29,10 @@ func NewBadRequest(err string) BadRequest {
2229
return BadRequest(err)
2330
}
2431

32+
func NewBadRequestf(format string, a ...interface{}) BadRequest {
33+
return BadRequest(fmt.Sprintf(format, a...))
34+
}
35+
2536
func (b BadRequest) Code() int {
2637
return http.StatusBadRequest
2738
}

‎expr/expr.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"fmt"
55
"regexp"
66
"strings"
7+
8+
"github.com/grafana/metrictank/errors"
79
)
810

911
//go:generate stringer -type=exprType
@@ -203,7 +205,7 @@ func (e expr) consumeBasicArg(pos int, exp Arg) (int, error) {
203205
}
204206
*v.val = got.str
205207
default:
206-
return 0, fmt.Errorf("unsupported type %T for consumeBasicArg", exp)
208+
return 0, errors.NewBadRequestf("unsupported type %T for consumeBasicArg", exp)
207209
}
208210
pos++
209211
return pos, nil
@@ -213,7 +215,7 @@ func generateValidatorError(key string, err error) error {
213215
if len(key) == 0 {
214216
return err
215217
}
216-
return fmt.Errorf("%s: %s", key, err.Error())
218+
return errors.NewBadRequestf("%s: %s", key, err.Error())
217219
}
218220

219221
// consumeSeriesArg verifies that the argument at given pos matches the expected arg
@@ -292,7 +294,7 @@ Switch:
292294
*v.val = append(*v.val, fn)
293295
}
294296
default:
295-
return 0, nil, fmt.Errorf("unsupported type %T for consumeSeriesArg", exp)
297+
return 0, nil, errors.NewBadRequestf("unsupported type %T for consumeSeriesArg", exp)
296298
}
297299
pos++
298300
return pos, reqs, nil
@@ -372,7 +374,7 @@ func (e expr) consumeKwarg(key string, optArgs []Arg) error {
372374
}
373375
*v.val = got.str
374376
default:
375-
return fmt.Errorf("unsupported type %T for consumeKwarg", exp)
377+
return errors.NewBadRequestf("unsupported type %T for consumeKwarg", exp)
376378
}
377379
return nil
378380
}

‎expr/func_aspercent.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package expr
22

33
import (
4-
"errors"
54
"fmt"
65
"math"
76
"sort"
87
"strings"
98

109
"github.com/grafana/metrictank/api/models"
10+
"github.com/grafana/metrictank/errors"
1111
"github.com/grafana/metrictank/schema"
1212
)
1313

@@ -58,12 +58,12 @@ func (s *FuncAsPercent) Exec(cache map[Req][]models.Series) ([]models.Series, er
5858

5959
if s.nodes != nil {
6060
if !math.IsNaN(s.totalFloat) {
61-
return nil, errors.New("total must be None or a seriesList")
61+
return nil, errors.NewBadRequest("total must be None or a seriesList")
6262
}
6363
outSeries, err = s.execWithNodes(series, totals, cache)
6464
} else {
6565
if totals != nil && len(totals) != 1 && len(totals) != len(series) {
66-
return nil, errors.New("asPercent second argument (total) must be missing, a single digit, reference exactly 1 series or reference the same number of series as the first argument")
66+
return nil, errors.NewBadRequest("asPercent second argument (total) must be missing, a single digit, reference exactly 1 series or reference the same number of series as the first argument")
6767
}
6868
outSeries, err = s.execWithoutNodes(series, totals, cache)
6969
}

‎expr/func_aspercent_test.go

+5
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package expr
33
import (
44
"fmt"
55
"math"
6+
"net/http"
67
"sort"
78
"strconv"
89
"testing"
@@ -21,6 +22,10 @@ func (e errAsPercentNumSeriesMismatch) Error() string {
2122
return fmt.Sprintf("asPercent got %d input series but %d total series (should be same amount or 1)", e.numIn, e.numTotal)
2223
}
2324

25+
func (e errAsPercentNumSeriesMismatch) Code() int {
26+
return http.StatusBadRequest
27+
}
28+
2429
var a1 = []schema.Point{
2530
{Val: 0, Ts: 10},
2631
{Val: 50.6, Ts: 20},

‎expr/func_divideseries.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package expr
22

33
import (
4-
"errors"
54
"fmt"
65
"math"
76

87
"github.com/grafana/metrictank/api/models"
8+
"github.com/grafana/metrictank/errors"
99
"github.com/grafana/metrictank/schema"
1010
)
1111

@@ -39,7 +39,7 @@ func (s *FuncDivideSeries) Exec(cache map[Req][]models.Series) ([]models.Series,
3939
return nil, err
4040
}
4141
if len(divisors) != 1 {
42-
return nil, errors.New(fmt.Sprintf("need 1 divisor series, not %d", len(divisors)))
42+
return nil, errors.NewBadRequestf("need 1 divisor series, not %d", len(divisors))
4343
}
4444
divisor := divisors[0]
4545

‎expr/func_divideserieslists.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package expr
22

33
import (
4-
"errors"
54
"fmt"
65
"math"
76

87
"github.com/grafana/metrictank/api/models"
8+
"github.com/grafana/metrictank/errors"
99
"github.com/grafana/metrictank/schema"
1010
)
1111

@@ -39,7 +39,7 @@ func (s *FuncDivideSeriesLists) Exec(cache map[Req][]models.Series) ([]models.Se
3939
return nil, err
4040
}
4141
if len(divisors) != len(dividends) {
42-
return nil, errors.New("dividendSeriesList and divisorSeriesList argument must have equal length")
42+
return nil, errors.NewBadRequest("dividendSeriesList and divisorSeriesList argument must have equal length")
4343
}
4444

4545
var series []models.Series

‎expr/func_filterseries.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package expr
22

33
import (
4-
"errors"
54
"math"
65

76
"github.com/grafana/metrictank/api/models"
87
"github.com/grafana/metrictank/consolidation"
8+
"github.com/grafana/metrictank/errors"
99
)
1010

1111
type FuncFilterSeries struct {
@@ -75,7 +75,7 @@ func getOperatorFunc(operator string) (func(float64, float64) bool, error) {
7575
return math.IsNaN(val) || val <= threshold
7676
}, nil
7777
}
78-
return func(v1, v2 float64) bool { return false }, errors.New("Unsupported operator: " + operator)
78+
return func(v1, v2 float64) bool { return false }, errors.NewBadRequest("Unsupported operator: " + operator)
7979
}
8080

8181
func (s *FuncFilterSeries) Exec(cache map[Req][]models.Series) ([]models.Series, error) {

‎expr/func_groupbytags.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ package expr
22

33
import (
44
"bytes"
5-
"errors"
65
"sort"
76

87
"github.com/grafana/metrictank/api/models"
8+
"github.com/grafana/metrictank/errors"
99
"github.com/grafana/metrictank/schema"
1010
)
1111

@@ -42,7 +42,7 @@ func (s *FuncGroupByTags) Exec(cache map[Req][]models.Series) ([]models.Series,
4242
}
4343

4444
if len(s.tags) == 0 {
45-
return nil, errors.New("No tags specified")
45+
return nil, errors.NewBadRequest("No tags specified")
4646
}
4747

4848
type Group struct {

‎expr/func_groupbytags_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package expr
22

33
import (
4-
"errors"
54
"math"
65
"sort"
76
"strconv"
87
"testing"
98

109
"github.com/grafana/metrictank/api/models"
10+
"github.com/grafana/metrictank/errors"
1111
"github.com/grafana/metrictank/schema"
1212
"github.com/grafana/metrictank/test"
1313
)
@@ -27,7 +27,7 @@ func TestNoTags(t *testing.T) {
2727
in := []models.Series{
2828
getModel("name1;tag1=val1", a),
2929
}
30-
expected := errors.New("No tags specified")
30+
expected := errors.NewBadRequest("No tags specified")
3131

3232
testGroupByTags("ErrNoTags", in, nil, "sum", []string{}, expected, t)
3333
}

‎expr/parse.go

+36-11
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,28 @@
11
package expr
22

33
import (
4-
"errors"
54
"fmt"
5+
"net/http"
66
"reflect"
77
"strconv"
88
"strings"
99

1010
"github.com/grafana/metrictank/api/models"
11+
"github.com/grafana/metrictank/errors"
1112
"github.com/grafana/metrictank/util"
1213
log "github.com/sirupsen/logrus"
1314
)
1415

1516
var (
16-
ErrMissingArg = errors.New("argument missing")
17-
ErrTooManyArg = errors.New("too many arguments")
18-
ErrMissingTimeseries = errors.New("missing time series argument")
19-
ErrWildcardNotAllowed = errors.New("found wildcard where series expected")
20-
ErrMissingExpr = errors.New("missing expression")
21-
ErrMissingComma = errors.New("missing comma")
22-
ErrMissingQuote = errors.New("missing quote")
23-
ErrUnexpectedCharacter = errors.New("unexpected character")
24-
ErrIllegalCharacter = errors.New("illegal character for function name")
17+
ErrMissingArg = errors.NewBadRequest("argument missing")
18+
ErrTooManyArg = errors.NewBadRequest("too many arguments")
19+
ErrMissingTimeseries = errors.NewBadRequest("missing time series argument")
20+
ErrWildcardNotAllowed = errors.NewBadRequest("found wildcard where series expected")
21+
ErrMissingExpr = errors.NewBadRequest("missing expression")
22+
ErrMissingComma = errors.NewBadRequest("missing comma")
23+
ErrMissingQuote = errors.NewBadRequest("missing quote")
24+
ErrUnexpectedCharacter = errors.NewBadRequest("unexpected character")
25+
ErrIllegalCharacter = errors.NewBadRequest("illegal character for function name")
2526
)
2627

2728
type ErrBadArgument struct {
@@ -33,6 +34,10 @@ func (e ErrBadArgument) Error() string {
3334
return fmt.Sprintf("argument bad type. expected %s - got %s", e.exp, e.got)
3435
}
3536

37+
func (e ErrBadArgument) Code() int {
38+
return http.StatusBadRequest
39+
}
40+
3641
type ErrBadArgumentStr struct {
3742
exp string
3843
got string
@@ -42,12 +47,20 @@ func (e ErrBadArgumentStr) Error() string {
4247
return fmt.Sprintf("argument bad type. expected %s - got %s", e.exp, e.got)
4348
}
4449

50+
func (e ErrBadArgumentStr) Code() int {
51+
return http.StatusBadRequest
52+
}
53+
4554
type ErrUnknownFunction string
4655

4756
func (e ErrUnknownFunction) Error() string {
4857
return fmt.Sprintf("unknown function %q", string(e))
4958
}
5059

60+
func (e ErrUnknownFunction) Code() int {
61+
return http.StatusBadRequest
62+
}
63+
5164
type ErrUnknownKwarg struct {
5265
key string
5366
}
@@ -56,6 +69,10 @@ func (e ErrUnknownKwarg) Error() string {
5669
return fmt.Sprintf("unknown keyword argument %q", e.key)
5770
}
5871

72+
func (e ErrUnknownKwarg) Code() int {
73+
return http.StatusBadRequest
74+
}
75+
5976
type ErrBadKwarg struct {
6077
key string
6178
exp Arg
@@ -66,6 +83,10 @@ func (e ErrBadKwarg) Error() string {
6683
return fmt.Sprintf("keyword argument %q bad type. expected %T - got %s", e.key, e.exp, e.got)
6784
}
6885

86+
func (e ErrBadKwarg) Code() int {
87+
return http.StatusBadRequest
88+
}
89+
6990
type ErrKwargSpecifiedTwice struct {
7091
key string
7192
}
@@ -74,6 +95,10 @@ func (e ErrKwargSpecifiedTwice) Error() string {
7495
return fmt.Sprintf("keyword argument %q specified twice", e.key)
7596
}
7697

98+
func (e ErrKwargSpecifiedTwice) Code() int {
99+
return http.StatusBadRequest
100+
}
101+
77102
type MetricRequest struct {
78103
Metric string
79104
From int32
@@ -90,7 +115,7 @@ func ParseMany(targets []string) ([]*expr, error) {
90115
return nil, err
91116
}
92117
if leftover != "" {
93-
return nil, fmt.Errorf("failed to parse %q fully. got leftover %q", target, leftover)
118+
return nil, errors.NewBadRequestf("failed to parse %q fully. got leftover %q", target, leftover)
94119
}
95120
out = append(out, e)
96121
}

0 commit comments

Comments
 (0)